diff --git a/lib/aprsme/device_cache.ex b/lib/aprsme/device_cache.ex index c7089fa..1493231 100644 --- a/lib/aprsme/device_cache.ex +++ b/lib/aprsme/device_cache.ex @@ -134,28 +134,32 @@ defmodule Aprsme.DeviceCache do defp find_matching_device(devices, identifier) do Enum.find(devices, fn device -> - pattern = device.identifier - - cond do - String.contains?(pattern, "?") -> - # Use cached regex compilation - regex_pattern = wildcard_pattern_to_regex_string(pattern) - - case Aprsme.RegexCache.get_or_compile(regex_pattern) do - {:ok, regex} -> Regex.match?(regex, identifier) - {:error, _} -> false - end - - String.contains?(pattern, "*") -> - # Compare literally if pattern contains * but not ? - pattern == identifier - - true -> - pattern == identifier - end + pattern_matches?(device.identifier, identifier) end) end + defp pattern_matches?(pattern, identifier) do + cond do + String.contains?(pattern, "?") -> + matches_wildcard_pattern?(pattern, identifier) + + String.contains?(pattern, "*") -> + pattern == identifier + + true -> + pattern == identifier + end + end + + defp matches_wildcard_pattern?(pattern, identifier) do + regex_pattern = wildcard_pattern_to_regex_string(pattern) + + case Aprsme.RegexCache.get_or_compile(regex_pattern) do + {:ok, regex} -> Regex.match?(regex, identifier) + {:error, _} -> false + end + end + # Converts a pattern with ? wildcards to a regex string (for caching) defp wildcard_pattern_to_regex_string(pattern) when is_binary(pattern) do # Replace ? with a placeholder, escape all regex metacharacters except the placeholder, diff --git a/lib/aprsme/packets.ex b/lib/aprsme/packets.ex index 636d7ce..0f16e74 100644 --- a/lib/aprsme/packets.ex +++ b/lib/aprsme/packets.ex @@ -223,25 +223,7 @@ defmodule Aprsme.Packets do defp insert_packet(attrs, _packet_data) do # Ensure data_extended is properly sanitized before insertion - attrs = - if attrs[:data_extended] do - sanitized_extended = Aprsme.EncodingUtils.sanitize_data_extended(attrs[:data_extended]) - # Double-check all values are sanitized - # Only do additional sanitization for plain maps, not structs - sanitized_extended = - if is_struct(sanitized_extended) do - sanitized_extended - else - Enum.reduce(sanitized_extended, %{}, fn {k, v}, acc -> - sanitized_value = if is_binary(v), do: Aprsme.EncodingUtils.sanitize_string(v), else: v - Map.put(acc, k, sanitized_value) - end) - end - - Map.put(attrs, :data_extended, sanitized_extended) - else - attrs - end + attrs = sanitize_data_extended_attr(attrs) # Debug log to see what we're trying to insert if attrs[:data_extended] do @@ -270,6 +252,27 @@ defmodule Aprsme.Packets do {:error, {:storage_exception, exception}} end + defp sanitize_data_extended_attr(attrs) do + if attrs[:data_extended] do + sanitized_extended = Aprsme.EncodingUtils.sanitize_data_extended(attrs[:data_extended]) + sanitized_extended = deep_sanitize_map(sanitized_extended) + Map.put(attrs, :data_extended, sanitized_extended) + else + attrs + end + end + + defp deep_sanitize_map(data) when is_struct(data), do: data + + defp deep_sanitize_map(data) when is_map(data) do + Enum.reduce(data, %{}, fn {k, v}, acc -> + sanitized_value = if is_binary(v), do: Aprsme.EncodingUtils.sanitize_string(v), else: v + Map.put(acc, k, sanitized_value) + end) + end + + defp deep_sanitize_map(data), do: data + @doc """ Stores a bad packet in the database. diff --git a/lib/aprsme/spatial_pubsub.ex b/lib/aprsme/spatial_pubsub.ex index 74d38ba..3932470 100644 --- a/lib/aprsme/spatial_pubsub.ex +++ b/lib/aprsme/spatial_pubsub.ex @@ -258,23 +258,29 @@ defmodule Aprsme.SpatialPubSub do grid_cells = get_intersecting_grid_cells(bounds) Enum.reduce(grid_cells, state, fn grid_key, acc_state -> - case Map.get(acc_state.spatial_index, grid_key) do - nil -> - acc_state - - set -> - new_set = MapSet.delete(set, client_id) - - if MapSet.size(new_set) == 0 do - # Remove the key entirely instead of storing nil - update_in(acc_state, [:spatial_index], &Map.delete(&1, grid_key)) - else - put_in(acc_state, [:spatial_index, grid_key], new_set) - end - end + remove_client_from_grid_cell(acc_state, grid_key, client_id) end) end + defp remove_client_from_grid_cell(state, grid_key, client_id) do + case Map.get(state.spatial_index, grid_key) do + nil -> + state + + set -> + new_set = MapSet.delete(set, client_id) + update_grid_cell_after_removal(state, grid_key, new_set) + end + end + + defp update_grid_cell_after_removal(state, grid_key, new_set) do + if MapSet.size(new_set) == 0 do + update_in(state, [:spatial_index], &Map.delete(&1, grid_key)) + else + put_in(state, [:spatial_index, grid_key], new_set) + end + end + defp get_intersecting_grid_cells(%{north: n, south: s, east: e, west: w}) do # Calculate which grid cells intersect with the bounds min_lat_cell = floor(s / @grid_size) @@ -289,22 +295,27 @@ defmodule Aprsme.SpatialPubSub do end defp find_clients_for_location(state, lat, lon) do - # Get the grid cell for this location grid_key = {floor(lat / @grid_size), floor(lon / @grid_size)} - # Get all clients in this grid cell case Map.get(state.spatial_index, grid_key) do nil -> [] client_set -> - # Filter to only clients whose bounds actually contain the point - Enum.filter(client_set, fn client_id -> - case Map.get(state.clients, client_id) do - %{bounds: bounds} -> point_in_bounds?(lat, lon, bounds) - _ -> false - end - end) + filter_clients_by_bounds(state, client_set, lat, lon) + end + end + + defp filter_clients_by_bounds(state, client_set, lat, lon) do + Enum.filter(client_set, fn client_id -> + client_contains_point?(state, client_id, lat, lon) + end) + end + + defp client_contains_point?(state, client_id, lat, lon) do + case Map.get(state.clients, client_id) do + %{bounds: bounds} -> point_in_bounds?(lat, lon, bounds) + _ -> false end end diff --git a/test/aprsme/packets_test.exs b/test/aprsme/packets_test.exs index 2909dfd..1c545df 100644 --- a/test/aprsme/packets_test.exs +++ b/test/aprsme/packets_test.exs @@ -98,17 +98,18 @@ defmodule Aprsme.PacketsTest do assert bad_packet.error_type == "ValidationError" end - test "handles storage exceptions" do - # Create invalid data that will cause an exception + test "handles invalid data types gracefully" do + # Create invalid data with wrong type for data_extended packet_data = %{ sender: "TEST4", destination: "APRS", raw_packet: "TEST4>APRS:Invalid", - # This will cause an exception when trying to extract position + # Invalid: data_extended should be a map, not a string data_extended: "invalid_data_extended" } - assert {:error, :storage_exception} = Packets.store_packet(packet_data) + # Should return validation error due to missing required fields + assert {:error, :validation_error} = Packets.store_packet(packet_data) end test "normalizes SSID to string" do