diff --git a/datasette/templates/debug_allowed.html b/datasette/templates/debug_allowed.html index 4f8106b8..83cc1ae6 100644 --- a/datasette/templates/debug_allowed.html +++ b/datasette/templates/debug_allowed.html @@ -49,7 +49,7 @@
- + Number of results per page (max 200)
@@ -88,7 +88,7 @@ const hasDebugPermission = {{ 'true' if has_debug_permission else 'false' }}; (function() { const params = populateFormFromURL(); const action = params.get('action'); - const page = params.get('page'); + const page = params.get('_page'); if (action) { fetchResults(page ? parseInt(page) : 1); } @@ -102,14 +102,14 @@ async function fetchResults(page = 1) { const params = new URLSearchParams(); for (const [key, value] of formData.entries()) { - if (value && key !== 'page_size') { + if (value && key !== '_size' && key !== '_page') { params.append(key, value); } } const pageSize = document.getElementById('page_size').value || '50'; - params.append('page', page.toString()); - params.append('page_size', pageSize); + params.append('_page', page.toString()); + params.append('_size', pageSize); try { const response = await fetch('{{ urls.path("-/allowed.json") }}?' + params.toString(), { diff --git a/datasette/templates/debug_rules.html b/datasette/templates/debug_rules.html index aafa755d..d00ba9cc 100644 --- a/datasette/templates/debug_rules.html +++ b/datasette/templates/debug_rules.html @@ -37,7 +37,7 @@
- + Number of results per page (max 200)
@@ -75,7 +75,7 @@ const submitBtn = document.getElementById('submit-btn'); (function() { const params = populateFormFromURL(); const action = params.get('action'); - const page = params.get('page'); + const page = params.get('_page'); if (action) { fetchResults(page ? parseInt(page) : 1); } @@ -89,14 +89,14 @@ async function fetchResults(page = 1) { const params = new URLSearchParams(); for (const [key, value] of formData.entries()) { - if (value && key !== 'page_size') { + if (value && key !== '_size' && key !== '_page') { params.append(key, value); } } const pageSize = document.getElementById('page_size').value || '50'; - params.append('page', page.toString()); - params.append('page_size', pageSize); + params.append('_page', page.toString()); + params.append('_size', pageSize); try { const response = await fetch('{{ urls.path("-/rules.json") }}?' + params.toString(), { diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index 1d921f02..19de31ff 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -1294,6 +1294,28 @@ async def derive_named_parameters(db: "Database", sql: str) -> List[str]: return named_parameters(sql) +def parse_size_limit(value, default, maximum, name="_size"): + """ + Parse a page-size parameter using the same semantics as the table + view's ?_size=: blank means default, "max" means maximum, integers + must be 0 or greater and no larger than maximum. Raises ValueError + with a message suitable for a 400 response. + """ + if value in (None, ""): + return default + if value == "max": + return maximum + try: + size = int(value) + if size < 0: + raise ValueError + except ValueError: + raise ValueError("{} must be a positive integer".format(name)) + if size > maximum: + raise ValueError("{} must be <= {}".format(name, maximum)) + return size + + UNSTABLE_API_MESSAGE = ( "This API is not part of Datasette's stable interface and may change at any time" ) diff --git a/datasette/views/query_helpers.py b/datasette/views/query_helpers.py index 026a999f..e9f85b6d 100644 --- a/datasette/views/query_helpers.py +++ b/datasette/views/query_helpers.py @@ -13,6 +13,7 @@ from datasette.write_sql import ( operation_is_write, ) from datasette.utils import ( + parse_size_limit, named_parameters as derive_named_parameters, escape_sqlite, path_from_row_pks, @@ -94,13 +95,11 @@ def _as_optional_bool(value, name): raise QueryValidationError("{} must be 0 or 1".format(name)) -def _query_list_limit(value, default=50): - if value in (None, ""): - return default +def _query_list_limit(value, default, maximum): try: - return min(max(1, int(value)), 1000) + return parse_size_limit(value, default, maximum) except ValueError as ex: - raise QueryValidationError("_size must be an integer") from ex + raise QueryValidationError(str(ex)) from ex def _derived_query_parameters(sql): diff --git a/datasette/views/special.py b/datasette/views/special.py index 28d4d6a1..9386440c 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -8,6 +8,7 @@ from datasette.utils.asgi import Response, Forbidden from datasette.utils import ( UNSTABLE_API_MESSAGE, actor_matches_allow, + parse_size_limit, add_cors_headers, await_me_maybe, error_body, @@ -373,17 +374,17 @@ class AllowedResourcesView(BaseView): ) try: - page = int(request.args.get("page", "1")) - page_size = int(request.args.get("page_size", "50")) + page = int(request.args.get("_page", "1")) + if page < 1: + raise ValueError except ValueError: - return error_body("page and page_size must be integers", 400), 400 - if page < 1: - return error_body("page must be >= 1", 400), 400 - if page_size < 1: - return error_body("page_size must be >= 1", 400), 400 - max_page_size = 200 - if page_size > max_page_size: - page_size = max_page_size + return error_body("_page must be a positive integer", 400), 400 + try: + page_size = parse_size_limit( + request.args.get("_size"), default=50, maximum=200 + ) + except ValueError as ex: + return error_body(str(ex), 400), 400 offset = (page - 1) * page_size # Use the simplified allowed_resources method @@ -448,12 +449,12 @@ class AllowedResourcesView(BaseView): def build_page_url(page_number): pairs = [] for key in request.args: - if key in {"page", "page_size"}: + if key in {"_page", "_size"}: continue for value in request.args.getlist(key): pairs.append((key, value)) - pairs.append(("page", str(page_number))) - pairs.append(("page_size", str(page_size))) + pairs.append(("_page", str(page_number))) + pairs.append(("_size", str(page_size))) query = urllib.parse.urlencode(pairs) return f"{request.path}?{query}" @@ -511,19 +512,19 @@ class PermissionRulesView(BaseView): actor = request.actor if isinstance(request.actor, dict) else None try: - page = int(request.args.get("page", "1")) - page_size = int(request.args.get("page_size", "50")) + page = int(request.args.get("_page", "1")) + if page < 1: + raise ValueError except ValueError: return Response.json( - error_body("page and page_size must be integers", 400), status=400 + error_body("_page must be a positive integer", 400), status=400 ) - if page < 1: - return Response.json(error_body("page must be >= 1", 400), status=400) - if page_size < 1: - return Response.json(error_body("page_size must be >= 1", 400), status=400) - max_page_size = 200 - if page_size > max_page_size: - page_size = max_page_size + try: + page_size = parse_size_limit( + request.args.get("_size"), default=50, maximum=200 + ) + except ValueError as ex: + return Response.json(error_body(str(ex), 400), status=400) offset = (page - 1) * page_size from datasette.utils.actions_sql import build_permission_rules_sql @@ -574,12 +575,12 @@ class PermissionRulesView(BaseView): def build_page_url(page_number): pairs = [] for key in request.args: - if key in {"page", "page_size"}: + if key in {"_page", "_size"}: continue for value in request.args.getlist(key): pairs.append((key, value)) - pairs.append(("page", str(page_number))) - pairs.append(("page_size", str(page_size))) + pairs.append(("_page", str(page_number))) + pairs.append(("_size", str(page_size))) query = urllib.parse.urlencode(pairs) return f"{request.path}?{query}" diff --git a/datasette/views/stored_queries.py b/datasette/views/stored_queries.py index ea49d983..bb23a6bd 100644 --- a/datasette/views/stored_queries.py +++ b/datasette/views/stored_queries.py @@ -90,6 +90,7 @@ class QueryListView(BaseView): limit = _query_list_limit( request.args.get("_size"), default=20 if format_ == "html" else 50, + maximum=self.ds.max_returned_rows, ) is_write = _as_optional_bool(request.args.get("is_write"), "is_write") is_private = _as_optional_bool(request.args.get("is_private"), "is_private") diff --git a/docs/authentication.rst b/docs/authentication.rst index 7d4d019d..0a476c1c 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -1162,7 +1162,7 @@ The ``/-/allowed`` endpoint displays resources that the current actor can access This endpoint provides an interactive HTML form interface. Add ``.json`` to the URL path (e.g. ``/-/allowed.json``) to get the raw JSON response instead. -Pass ``?action=view-table`` (or another action) to select the action. Optional ``parent=`` and ``child=`` query parameters can narrow the results to a specific database/table pair. +Pass ``?action=view-table`` (or another action) to select the action. Optional ``parent=`` and ``child=`` query parameters can narrow the results to a specific database/table pair. Results are paginated: ``?_size=`` sets the page size (default 50, maximum 200, ``max`` for the maximum) and ``?_page=`` selects a page. This endpoint is publicly accessible to help users understand their own permissions. The potentially sensitive ``reason`` field is only shown to users with the ``permissions-debug`` permission - it shows the plugins and explanatory reasons that were responsible for each decision. @@ -1175,7 +1175,7 @@ The ``/-/rules`` endpoint displays all permission rules (both allow and deny) fo This endpoint provides an interactive HTML form interface. Add ``.json`` to the URL path (e.g. ``/-/rules.json?action=view-table``) to get the raw JSON response instead. -Pass ``?action=`` as a query parameter to specify which action to check. +Pass ``?action=`` as a query parameter to specify which action to check. The ``?_size=`` and ``?_page=`` pagination parameters work the same as on ``/-/allowed``. This endpoint requires the ``permissions-debug`` permission. diff --git a/docs/pages.rst b/docs/pages.rst index 67fe390b..65c03a49 100644 --- a/docs/pages.rst +++ b/docs/pages.rst @@ -95,7 +95,7 @@ Use the :ref:`ExecuteWriteView` JSON API to execute writable SQL programmaticall Stored query browsers --------------------- -The ``/-/queries`` page lists stored queries across every database visible to the current actor. The ``/database-name/-/queries`` page lists stored queries for a single database. +The ``/-/queries`` page lists stored queries across every database visible to the current actor. The ``/database-name/-/queries`` page lists stored queries for a single database. The JSON versions accept ``?_size=`` (default 50, ``max`` for the :ref:`setting_max_returned_rows` limit) and a ``?_next=`` pagination token. These pages support search, pagination and filters for read-only or writable queries and private or public queries. Adding a ``.json`` extension to either URL returns the same list as JSON. diff --git a/existing-api.md b/existing-api.md index 2f9996dc..c0c8e442 100644 --- a/existing-api.md +++ b/existing-api.md @@ -377,8 +377,8 @@ path always renders the HTML form; `.json` returns JSON. resources. Items gain a `reason` field if the actor also holds `permissions-debug`. - **Parameters:** `action` (required; missing → 400 canonical error, unknown - → 404), `parent`, `child` (requires `parent`), `page` (default 1), - `page_size` (default 50, silently capped at 200). + → 404), `parent`, `child` (requires `parent`), `_page` (default 1), + `_size` (default 50, maximum 200, accepts `max`; out-of-range → 400). - **Response:** `{"action", "actor_id", "page", "page_size", "total", "items": [{"parent", "child", "resource"}]}` with optional `next_url` / `previous_url`. @@ -1031,8 +1031,9 @@ databases (`database`/`database_color` are null, `show_database` true). - **Permissions:** no single gate; results filtered per query by `view-query` (private queries appear only for their owner). -- **Parameters:** `_size` (default 20 HTML / **50 JSON**, clamped 1–1000; - non-integer → 400), `_next` (cursor), `q` (substring search over +- **Parameters:** `_size` (default 20 HTML / **50 JSON**; accepts `max`; + values over `max_returned_rows` or non-integers → 400, matching table + `_size` semantics), `_next` (cursor), `q` (substring search over name/title/description/sql), `is_write` / `is_private` (booleans; invalid → 400 `"is_write must be 0 or 1"`), `source`, `owner_id`. - **Response** — 200: diff --git a/stable-api-recommendations.md b/stable-api-recommendations.md index b86d8228..ba837204 100644 --- a/stable-api-recommendations.md +++ b/stable-api-recommendations.md @@ -203,9 +203,13 @@ number. > **Status:** `next_url` now accompanies `next` in the default table JSON > keys (previously it required `?_extra=next_url`), so every response with > a `next` token also carries the ready-to-follow URL. Pagination tokens -> are deliberately left undocumented as to their internal structure. The -> `_size`/`page_size` naming and `has_more`/`total` differences remain -> open. +> are deliberately left undocumented as to their internal structure. +> `_size` is now the single page-size parameter with uniform table-style +> semantics everywhere: query lists accept `max` and 400 on out-of-range +> values (previously silently clamped), and the `/-/allowed` and +> `/-/rules` debug endpoints renamed `page`/`page_size` to +> `_page`/`_size` with the same validation (400 instead of silent +> capping at 200). The `has_more`/`total` differences remain open. | Endpoint | Mechanism | Token | Extras | |---|---|---|---| diff --git a/tests/test_error_shape.py b/tests/test_error_shape.py index a34fbed7..2398940b 100644 --- a/tests/test_error_shape.py +++ b/tests/test_error_shape.py @@ -612,3 +612,53 @@ async def test_threads_requires_permissions_debug(ds_error_shape): allowed = await ds_error_shape.client.get("/-/threads.json", actor={"id": "root"}) assert allowed.status_code == 200 assert allowed.json()["ok"] is True + + +# _size is the one page-size parameter, with uniform validation + + +@pytest.mark.asyncio +async def test_query_list_size_supports_max_keyword(ds_client): + response = await ds_client.get("/fixtures/-/queries.json?_size=max") + assert response.status_code == 200 + # ds_client runs with max_returned_rows=100 + assert response.json()["limit"] == 100 + + +@pytest.mark.asyncio +async def test_query_list_size_rejects_out_of_range(ds_client): + response = await ds_client.get("/fixtures/-/queries.json?_size=5000") + data = assert_canonical_error(response, 400) + assert data["errors"] == ["_size must be <= 100"] + + +@pytest.mark.asyncio +async def test_query_list_size_rejects_non_integer(ds_client): + response = await ds_client.get("/fixtures/-/queries.json?_size=bananas") + data = assert_canonical_error(response, 400) + assert data["errors"] == ["_size must be a positive integer"] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("endpoint", ("allowed", "rules")) +async def test_debug_endpoints_use_size_and_page_parameters(ds_error_shape, endpoint): + base = "/-/{}.json?action=view-instance".format(endpoint) + ok = await ds_error_shape.client.get( + base + "&_size=1&_page=1", actor={"id": "root"} + ) + assert ok.status_code == 200 + assert ok.json()["page_size"] == 1 + + max_size = await ds_error_shape.client.get( + base + "&_size=max", actor={"id": "root"} + ) + assert max_size.status_code == 200 + assert max_size.json()["page_size"] == 200 + + too_big = await ds_error_shape.client.get(base + "&_size=500", actor={"id": "root"}) + data = assert_canonical_error(too_big, 400) + assert data["errors"] == ["_size must be <= 200"] + + bad_page = await ds_error_shape.client.get(base + "&_page=0", actor={"id": "root"}) + data = assert_canonical_error(bad_page, 400) + assert data["errors"] == ["_page must be a positive integer"] diff --git a/tests/test_html.py b/tests/test_html.py index 22943300..b4c47d80 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -1363,12 +1363,12 @@ async def test_permission_debug_tabs_with_query_string(ds_client): # Test /-/allowed with query string response = await ds_client.get( - "/-/allowed?action=view-table&page_size=50", actor=actor + "/-/allowed?action=view-table&_size=50", actor=actor ) assert response.status_code == 200 # Check that Rules and Check tabs have the query string - assert 'href="/-/rules?action=view-table&page_size=50"' in response.text - assert 'href="/-/check?action=view-table&page_size=50"' in response.text + assert 'href="/-/rules?action=view-table&_size=50"' in response.text + assert 'href="/-/check?action=view-table&_size=50"' in response.text # Playground and Actions should not have query string assert 'href="/-/permissions"' in response.text assert 'href="/-/actions"' in response.text diff --git a/tests/test_permission_endpoints.py b/tests/test_permission_endpoints.py index e25be23e..8726ab62 100644 --- a/tests/test_permission_endpoints.py +++ b/tests/test_permission_endpoints.py @@ -137,9 +137,7 @@ async def test_allowed_json_pagination(): await ds.refresh_schemas() # Test page 1 - response = await ds.client.get( - "/-/allowed.json?action=view-table&page_size=10&page=1" - ) + response = await ds.client.get("/-/allowed.json?action=view-table&_size=10&_page=1") assert response.status_code == 200 data = response.json() assert data["page"] == 1 @@ -147,9 +145,7 @@ async def test_allowed_json_pagination(): assert len(data["items"]) == 10 # Test page 2 - response = await ds.client.get( - "/-/allowed.json?action=view-table&page_size=10&page=2" - ) + response = await ds.client.get("/-/allowed.json?action=view-table&_size=10&_page=2") assert response.status_code == 200 data = response.json() assert data["page"] == 2 @@ -157,10 +153,10 @@ async def test_allowed_json_pagination(): # Verify items are different between pages response1 = await ds.client.get( - "/-/allowed.json?action=view-table&page_size=10&page=1" + "/-/allowed.json?action=view-table&_size=10&_page=1" ) response2 = await ds.client.get( - "/-/allowed.json?action=view-table&page_size=10&page=2" + "/-/allowed.json?action=view-table&_size=10&_page=2" ) items1 = {(item["parent"], item["child"]) for item in response1.json()["items"]} items2 = {(item["parent"], item["child"]) for item in response2.json()["items"]} @@ -323,7 +319,7 @@ async def test_rules_json_pagination(): # Test basic pagination structure - just verify it returns paginated results response = await ds.client.get( - "/-/rules.json?action=view-table&page_size=2&page=1", + "/-/rules.json?action=view-table&_size=2&_page=1", actor={"id": "root"}, ) assert response.status_code == 200