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