From 1ddef07bcc6900f0dc01bc70d4bb33456c4caf4e Mon Sep 17 00:00:00 2001
From: Claudio Ortolina <cloud8421@gmail.com>
Date: Mon, 4 May 2026 14:17:45 +0100
Subject: [PATCH] fix(api): handle non-integer error ids, clamp negative
 pagination params

- show/2: use case instead of bare Integer.parse match so
  /api/v1/errors/not-an-id returns 404 instead of crashing
- index/2: clamp limit to >= 1 and offset to >= 0 to prevent
  SQLite from treating negative values as unlimited
- Add 3 tests: non-integer id 404, negative limit/offset clamping
---
 .../controllers/error_controller.ex           | 20 ++++++++-----
 .../controllers/error_controller_test.exs     | 28 +++++++++++++++++++
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/lib/music_library_web/controllers/error_controller.ex b/lib/music_library_web/controllers/error_controller.ex
index fdb996fe..54a5ae6f 100644
--- a/lib/music_library_web/controllers/error_controller.ex
+++ b/lib/music_library_web/controllers/error_controller.ex
@@ -7,8 +7,8 @@ defmodule MusicLibraryWeb.ErrorController do
     status = parse_status(params["status"])
     muted = parse_muted(params["muted"])
     search = params["search"]
-    limit = parse_int(params["limit"], 50)
-    offset = parse_int(params["offset"], 0)
+    limit = max(1, parse_int(params["limit"], 50))
+    offset = max(0, parse_int(params["offset"], 0))
 
     opts =
       []
@@ -24,13 +24,19 @@ defmodule MusicLibraryWeb.ErrorController do
   end
 
   def show(conn, %{"id" => id}) do
-    {id_int, ""} = Integer.parse(id)
+    case Integer.parse(id) do
+      {id_int, ""} when id_int > 0 ->
+        case Errors.get_error(id_int) do
+          {:ok, error} ->
+            render(conn, :show, error: error)
 
-    case Errors.get_error(id_int) do
-      {:ok, error} ->
-        render(conn, :show, error: error)
+          {:error, :not_found} ->
+            conn
+            |> put_status(:not_found)
+            |> json(%{error: "Not Found"})
+        end
 
-      {:error, :not_found} ->
+      _ ->
         conn
         |> put_status(:not_found)
         |> json(%{error: "Not Found"})
diff --git a/test/music_library_web/controllers/error_controller_test.exs b/test/music_library_web/controllers/error_controller_test.exs
index 47be3368..c7e42390 100644
--- a/test/music_library_web/controllers/error_controller_test.exs
+++ b/test/music_library_web/controllers/error_controller_test.exs
@@ -133,6 +133,25 @@ defmodule MusicLibraryWeb.ErrorControllerTest do
 
       assert %{"errors" => [], "total" => 0} = json_response(conn, 200)
     end
+
+    test "clamps negative limit to 1", %{conn: conn} do
+      conn =
+        conn
+        |> put_req_header("authorization", "Bearer #{api_token()}")
+        |> get(~p"/api/v1/errors?limit=-1")
+
+      assert %{"errors" => returned, "limit" => 1} = json_response(conn, 200)
+      assert returned != []
+    end
+
+    test "clamps negative offset to 0", %{conn: conn} do
+      conn =
+        conn
+        |> put_req_header("authorization", "Bearer #{api_token()}")
+        |> get(~p"/api/v1/errors?offset=-5")
+
+      assert %{"offset" => 0} = json_response(conn, 200)
+    end
   end
 
   describe "GET /api/v1/errors/:id" do
@@ -226,5 +245,14 @@ defmodule MusicLibraryWeb.ErrorControllerTest do
 
       assert json_response(conn, 404) == %{"error" => "Not Found"}
     end
+
+    test "returns 404 for non-integer id instead of crashing", %{conn: conn} do
+      conn =
+        conn
+        |> put_req_header("authorization", "Bearer #{api_token()}")
+        |> get(~p"/api/v1/errors/not-an-id")
+
+      assert json_response(conn, 404) == %{"error" => "Not Found"}
+    end
   end
 end