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
This commit is contained in:
@@ -7,8 +7,8 @@ defmodule MusicLibraryWeb.ErrorController do
|
|||||||
status = parse_status(params["status"])
|
status = parse_status(params["status"])
|
||||||
muted = parse_muted(params["muted"])
|
muted = parse_muted(params["muted"])
|
||||||
search = params["search"]
|
search = params["search"]
|
||||||
limit = parse_int(params["limit"], 50)
|
limit = max(1, parse_int(params["limit"], 50))
|
||||||
offset = parse_int(params["offset"], 0)
|
offset = max(0, parse_int(params["offset"], 0))
|
||||||
|
|
||||||
opts =
|
opts =
|
||||||
[]
|
[]
|
||||||
@@ -24,13 +24,19 @@ defmodule MusicLibraryWeb.ErrorController do
|
|||||||
end
|
end
|
||||||
|
|
||||||
def show(conn, %{"id" => id}) do
|
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
|
{:error, :not_found} ->
|
||||||
{:ok, error} ->
|
conn
|
||||||
render(conn, :show, error: error)
|
|> put_status(:not_found)
|
||||||
|
|> json(%{error: "Not Found"})
|
||||||
|
end
|
||||||
|
|
||||||
{:error, :not_found} ->
|
_ ->
|
||||||
conn
|
conn
|
||||||
|> put_status(:not_found)
|
|> put_status(:not_found)
|
||||||
|> json(%{error: "Not Found"})
|
|> json(%{error: "Not Found"})
|
||||||
|
|||||||
@@ -133,6 +133,25 @@ defmodule MusicLibraryWeb.ErrorControllerTest do
|
|||||||
|
|
||||||
assert %{"errors" => [], "total" => 0} = json_response(conn, 200)
|
assert %{"errors" => [], "total" => 0} = json_response(conn, 200)
|
||||||
end
|
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
|
end
|
||||||
|
|
||||||
describe "GET /api/v1/errors/:id" do
|
describe "GET /api/v1/errors/:id" do
|
||||||
@@ -226,5 +245,14 @@ defmodule MusicLibraryWeb.ErrorControllerTest do
|
|||||||
|
|
||||||
assert json_response(conn, 404) == %{"error" => "Not Found"}
|
assert json_response(conn, 404) == %{"error" => "Not Found"}
|
||||||
end
|
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
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user