fix: reject page[limit] above max_page_size instead of truncating silently

server/lib/orcasite/radio/detection.ex

@@ -128,6 +128,7 @@ defmodule Orcasite.Radio.Detection do
         offset? true
         countable true
         default_limit 100
+        max_page_size 1000
       end
 
       argument :feed_id, :string
@@ -141,6 +142,7 @@ defmodule Orcasite.Radio.Detection do
         offset? true
         countable true
         default_limit 100
+        max_page_size 1000
       end
 
       argument :category, Orcasite.Types.DetectionCategory do

server/lib/orcasite_web/json_api_router.ex

@@ -1,6 +1,17 @@
 defmodule OrcasiteWeb.JsonApiRouter do
-  use AshJsonApi.Router,
-    domains: [Orcasite.Notifications, Orcasite.Radio],
-    json_schema: "/json_schema",
-    open_api: "/open_api"
+  @moduledoc """
+  Entry point for the JSON:API.
+
+  Wraps `OrcasiteWeb.JsonApiRouter.AshRouter` so our own plugs can run — and
+  halt — before dispatch. `AshJsonApi.Router`'s `before_dispatch` hook is not
+  usable for rejecting a request: it calls the route's controller regardless of
+  `conn.halted`.
+  """
+
+  use Plug.Builder
+
+  plug OrcasiteWeb.Plugs.EnforceMaxPageSize,
+    domains: [Orcasite.Notifications, Orcasite.Radio]
+
+  plug OrcasiteWeb.JsonApiRouter.AshRouter
 end

server/lib/orcasite_web/json_api_router/ash_router.ex

@@ -0,0 +1,11 @@
+defmodule OrcasiteWeb.JsonApiRouter.AshRouter do
+  @moduledoc """
+  The generated JSON:API router. Reached through `OrcasiteWeb.JsonApiRouter`,
+  which runs our own plugs first.
+  """
+
+  use AshJsonApi.Router,
+    domains: [Orcasite.Notifications, Orcasite.Radio],
+    json_schema: "/json_schema",
+    open_api: "/open_api"
+end

server/lib/orcasite_web/plugs/enforce_max_page_size.ex

@@ -0,0 +1,119 @@
+defmodule OrcasiteWeb.Plugs.EnforceMaxPageSize do
+  @moduledoc """
+  Rejects JSON:API requests whose `page[limit]` exceeds the matched action's
+  `max_page_size`.
+
+  Without this, an over-large limit is silently truncated and the response
+  claims to be complete. Ash clamps the SQL `LIMIT` to `max_page_size`, but
+  keeps the *requested* limit for its "is there more?" bookkeeping
+  (`Ash.Actions.Read.to_page/7` splits on `page[:limit]`), so the page comes
+  back short with `more?: false`. AshJsonApi then sees `Enum.count(results) <
+  limit` and emits `links.next: null`. A client asking for 500 detections gets
+  251 and no reason to keep paginating.
+
+  We can't serve what was asked for, so we say so rather than under-serving it
+  quietly.
+
+  See https://github.com/orcasound/orcasite/issues/992.
+  """
+
+  @behaviour Plug
+
+  import Plug.Conn
+
+  @impl Plug
+  def init(opts), do: Keyword.validate!(opts, domains: [])
+
+  @impl Plug
+  def call(conn, opts) do
+    conn = fetch_query_params(conn)
+
+    with {:ok, limit} <- requested_limit(conn),
+         {:ok, resource, action_name} <- match_route(conn, Keyword.fetch!(opts, :domains)),
+         {:ok, max_page_size} <- max_page_size(resource, action_name),
+         true <- limit > max_page_size do
+      send_invalid_pagination(conn, limit, max_page_size)
+    else
+      _ -> conn
+    end
+  end
+
+  defp requested_limit(%{query_params: %{"page" => %{"limit" => limit}}}) when is_binary(limit) do
+    case Integer.parse(limit) do
+      {limit, ""} -> {:ok, limit}
+      _ -> :error
+    end
+  end
+
+  defp requested_limit(_conn), do: :error
+
+  # Mirrors the route resolution in `AshJsonApi.Controllers.Router`. This plug
+  # runs ahead of that router (which does not honor `halt/1`), so it has to find
+  # the route itself to know which action's limits apply.
+  defp match_route(conn, domains) do
+    Enum.find_value(domains, :error, fn domain ->
+      case match_domain_route(domain, conn) do
+        {:ok, resource, route} ->
+          {:ok, resource, route.action}
+
+        :error ->
+          domain
+          |> Ash.Domain.Info.resources()
+          |> Enum.filter(&(AshJsonApi.Resource in Spark.extensions(&1)))
+          |> Enum.find_value(fn resource ->
+            case resource.json_api_match_route(conn.method, conn.path_info) do
+              {:ok, route, _params} -> {:ok, resource, route.action}
+              :error -> nil
+            end
+          end)
+      end
+    end)
+  end
+
+  defp match_domain_route(domain, conn) do
+    if Code.ensure_loaded?(domain) and function_exported?(domain, :json_api_match_route, 2) do
+      case domain.json_api_match_route(conn.method, conn.path_info) do
+        {:ok, resource, route, _params} -> {:ok, resource, route}
+        :error -> :error
+      end
+    else
+      :error
+    end
+  end
+
+  # Only read actions carry `pagination`; anything else (creates, or a `related`
+  # route whose action lives on the destination resource) falls through and is
+  # left alone.
+  defp max_page_size(resource, action_name) do
+    case Ash.Resource.Info.action(resource, action_name) do
+      %{pagination: %{max_page_size: max_page_size}} when is_integer(max_page_size) ->
+        {:ok, max_page_size}
+
+      _ ->
+        :error
+    end
+  end
+
+  defp send_invalid_pagination(conn, limit, max_page_size) do
+    errors =
+      AshJsonApi.Serializer.serialize_errors(nil, [
+        %AshJsonApi.Error{
+          id: Ash.UUID.generate(),
+          status_code: 400,
+          code: "invalid_pagination",
+          title: "InvalidPagination",
+          detail:
+            "Invalid pagination: page[limit] of #{limit} exceeds the maximum page size of " <>
+              "#{max_page_size} for this endpoint. Request #{max_page_size} or fewer and " <>
+              "follow links.next to retrieve the rest.",
+          source_parameter: "page[limit]",
+          meta: %{}
+        }
+      ])
+
+    conn
+    |> put_resp_content_type("application/vnd.api+json")
+    |> send_resp(400, errors)
+    |> halt()
+  end
+end

server/test/orcasite_web/json_api/max_page_size_test.exs

@@ -0,0 +1,71 @@
+defmodule OrcasiteWeb.JsonApi.MaxPageSizeTest do
+  @moduledoc """
+  Regression tests for https://github.com/orcasound/orcasite/issues/992.
+
+  A `page[limit]` above the action's `max_page_size` used to be silently
+  clamped, returning a short page with `links.next: null` — which reads as
+  "that's everything". It must be an error instead.
+  """
+
+  use OrcasiteWeb.ConnCase, async: true
+
+  describe "page[limit] above max_page_size" do
+    test "is rejected on an action with an explicit max_page_size", %{conn: conn} do
+      assert %{
+               "errors" => [
+                 %{
+                   "code" => "invalid_pagination",
+                   "detail" => detail,
+                   "source" => %{"parameter" => "page[limit]"},
+                   "status" => "400",
+                   "title" => "InvalidPagination"
+                 }
+               ]
+             } =
+               conn
+               |> get("/api/json/detections?page[limit]=1001")
+               |> json_response(400)
+
+      assert detail =~ "exceeds the maximum page size of 1000"
+    end
+
+    test "is rejected on an action using Ash's default max_page_size", %{conn: conn} do
+      assert %{"errors" => [%{"code" => "invalid_pagination", "detail" => detail}]} =
+               conn
+               |> get("/api/json/feed_segments?page[limit]=251")
+               |> json_response(400)
+
+      assert detail =~ "exceeds the maximum page size of 250"
+    end
+  end
+
+  describe "page[limit] at or below max_page_size" do
+    test "is served for an action with an explicit max_page_size", %{conn: conn} do
+      assert %{"data" => _} =
+               conn
+               |> get("/api/json/detections?page[limit]=1000")
+               |> json_response(200)
+    end
+
+    test "is served for an action using Ash's default max_page_size", %{conn: conn} do
+      assert %{"data" => _} =
+               conn
+               |> get("/api/json/feed_segments?page[limit]=250")
+               |> json_response(200)
+    end
+
+    test "requests without page[limit] are untouched", %{conn: conn} do
+      assert %{"data" => _} =
+               conn
+               |> get("/api/json/detections")
+               |> json_response(200)
+    end
+  end
+
+  test "unmatched routes still 404 rather than being swallowed", %{conn: conn} do
+    assert %{"errors" => [%{"code" => "no_route_found"}]} =
+             conn
+             |> get("/api/json/not_a_real_resource?page[limit]=1000000")
+             |> json_response(404)
+  end
+end