From 93ac86f74802b2d33991d5f7b39e19e883e4660e Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Mon, 4 May 2026 14:11:46 +0100 Subject: [PATCH] fix(errors): LIKE wildcard escaping, memory-efficient counts, context tests - Escape LIKE wildcards (% and _) in search queries using fragment ESCAPE to prevent accidental pattern expansion and potential DoS vectors - Use Repo.aggregate for occurrence_count instead of loading all occurrences into memory in get_error/1 - Use Repo.aggregate(:min, :inserted_at) for first_occurrence_at instead of fragile List.last/1 that depends on query ordering - Add MusicLibrary.ErrorsTest with 15 tests covering list_errors/1 (default, status/muted/search filters, wildcard escaping, pagination, empty results) and get_error/1 (with/without occurrences, not_found) --- lib/music_library/errors.ex | 21 +++- test/music_library/errors_test.exs | 178 +++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 5 deletions(-) create mode 100644 test/music_library/errors_test.exs diff --git a/lib/music_library/errors.ex b/lib/music_library/errors.ex index 39ff9c03..73759f65 100644 --- a/lib/music_library/errors.ex +++ b/lib/music_library/errors.ex @@ -62,8 +62,13 @@ defmodule MusicLibrary.Errors do ) |> Repo.all() - occurrence_count = length(occurrences) - first_occurrence_at = get_first_occurrence_at(occurrences) + occurrence_count = + from(o in Occurrence, where: o.error_id == ^id) + |> Repo.aggregate(:count, :id) + + first_occurrence_at = + from(o in Occurrence, where: o.error_id == ^id) + |> Repo.aggregate(:min, :inserted_at) result = %{error | occurrences: occurrences} @@ -93,9 +98,15 @@ defmodule MusicLibrary.Errors do defp maybe_filter_search(query, ""), do: query defp maybe_filter_search(query, search) do - where(query, [e], like(e.reason, ^"%#{search}%")) + escaped = escape_like_wildcards(search) + where(query, [e], fragment("? LIKE ? ESCAPE '\\'", e.reason, ^"%#{escaped}%")) end - defp get_first_occurrence_at([]), do: nil - defp get_first_occurrence_at(occurrences), do: List.last(occurrences).inserted_at + @doc false + def escape_like_wildcards(search) when is_binary(search) do + search + |> String.replace("\\", "\\\\") + |> String.replace("%", "\\%") + |> String.replace("_", "\\_") + end end diff --git a/test/music_library/errors_test.exs b/test/music_library/errors_test.exs new file mode 100644 index 00000000..072f1bc8 --- /dev/null +++ b/test/music_library/errors_test.exs @@ -0,0 +1,178 @@ +defmodule MusicLibrary.ErrorsTest do + use MusicLibrary.DataCase + + import MusicLibrary.ErrorsFixtures + + alias MusicLibrary.Errors + + # Helper to create an error with a unique fingerprint derived from a tag. + # Avoids unique constraint violations when creating multiple errors in a test. + defp unique_error(tag, attrs) do + line = "#{tag}_line" + func = "TestModule.#{tag}/0" + + error_fixture( + Map.merge( + %{ + source_line: line, + source_function: func, + fingerprint: error_fingerprint(:runtime_error, line, func) + }, + Map.new(attrs) + ) + ) + end + + describe "list_errors/1" do + test "returns all errors by default" do + e1 = unique_error("a", %{reason: "Error A"}) + e2 = unique_error("b", %{reason: "Error B"}) + + result = Errors.list_errors() + + assert result.total == 2 + assert length(result.errors) == 2 + ids = Enum.map(result.errors, & &1.id) + assert e1.id in ids + assert e2.id in ids + end + + test "filters by status" do + _e1 = unique_error("unres", %{status: :unresolved, reason: "Unresolved error"}) + e2 = unique_error("res", %{status: :resolved, reason: "Resolved error"}) + + result = Errors.list_errors(status: :resolved) + + assert result.total == 1 + assert length(result.errors) == 1 + assert hd(result.errors).id == e2.id + assert hd(result.errors).status == :resolved + end + + test "filters by muted" do + _e1 = unique_error("unmuted", %{muted: false, reason: "Not muted"}) + e2 = unique_error("muted", %{muted: true, reason: "Muted error"}) + + result = Errors.list_errors(muted: true) + + assert result.total == 1 + assert length(result.errors) == 1 + assert hd(result.errors).id == e2.id + assert hd(result.errors).muted == true + end + + test "filters by search substring on reason" do + _e1 = unique_error("first", %{reason: "First error"}) + e2 = unique_error("second", %{reason: "Second error with specific text"}) + + result = Errors.list_errors(search: "specific text") + + assert result.total == 1 + assert length(result.errors) == 1 + assert hd(result.errors).id == e2.id + assert hd(result.errors).reason == "Second error with specific text" + end + + test "search escapes LIKE wildcards (% and _) to prevent pattern expansion" do + e1 = unique_error("pct", %{reason: "Error with 100% match"}) + _e2 = unique_error("pctx", %{reason: "Error with 100x match"}) + + # Searching for "100%" should only match the exact text "100%", + # not treat % as a wildcard that would also match "100x". + result = Errors.list_errors(search: "100%") + + assert result.total == 1 + assert length(result.errors) == 1 + assert hd(result.errors).id == e1.id + end + + test "respects limit" do + _e1 = unique_error("lim_a", %{reason: "A"}) + _e2 = unique_error("lim_b", %{reason: "B"}) + _e3 = unique_error("lim_c", %{reason: "C"}) + + result = Errors.list_errors(limit: 2) + + assert result.total == 3 + assert length(result.errors) == 2 + end + + test "respects offset" do + _e1 = unique_error("off_a", %{reason: "A"}) + _e2 = unique_error("off_b", %{reason: "B"}) + _e3 = unique_error("off_c", %{reason: "C"}) + + result = Errors.list_errors(offset: 1, limit: 10) + + assert result.total == 3 + assert length(result.errors) == 2 + end + + test "returns empty list when no errors match filters" do + _e1 = unique_error("real", %{reason: "Real error"}) + + result = Errors.list_errors(search: "NONEXISTENT") + + assert result.total == 0 + assert result.errors == [] + end + + test "returns empty list when database has no errors at all" do + result = Errors.list_errors() + + assert result.total == 0 + assert result.errors == [] + end + end + + describe "get_error/1" do + test "returns error with occurrences, occurrence_count, and first_occurrence_at" do + error = unique_error("get_ok", %{reason: "Something went wrong"}) + _occ1 = occurrence_fixture(error, %{breadcrumbs: ["first"]}) + _occ2 = occurrence_fixture(error, %{breadcrumbs: ["second"]}) + + assert {:ok, result} = Errors.get_error(error.id) + + assert result.id == error.id + assert result.reason == "Something went wrong" + assert result.occurrence_count == 2 + refute is_nil(result.first_occurrence_at) + assert length(result.occurrences) == 2 + + # Occurrences sorted desc by inserted_at (most recent first) + [occ1, occ2] = result.occurrences + assert occ1.breadcrumbs == ["second"] + assert occ2.breadcrumbs == ["first"] + end + + test "returns occurrence_count = 0 and first_occurrence_at = nil for error with no occurrences" do + error = unique_error("no_occ", %{reason: "No occurrences"}) + + assert {:ok, result} = Errors.get_error(error.id) + + assert result.occurrence_count == 0 + assert is_nil(result.first_occurrence_at) + assert result.occurrences == [] + end + + test "returns {:error, :not_found} for non-existent id" do + assert Errors.get_error(99_999) == {:error, :not_found} + end + end + + describe "escape_like_wildcards/1" do + test "escapes % and _ characters" do + assert Errors.escape_like_wildcards("100%") == "100\\%" + assert Errors.escape_like_wildcards("a_b") == "a\\_b" + assert Errors.escape_like_wildcards("50%_off") == "50\\%\\_off" + end + + test "preserves backslash escaping" do + assert Errors.escape_like_wildcards("a\\b") == "a\\\\b" + end + + test "passes through normal text unchanged" do + assert Errors.escape_like_wildcards("normal text") == "normal text" + end + end +end