mirror of
https://github.com/simonw/datasette.git
synced 2026-07-08 16:44:34 +02:00
Return 400 for unknown _extra names on data formats
Unknown ?_extra= names (including internal HTML-only extras such as display_rows) were silently ignored, so a typo returned the default payload with no signal. Table, row and query data formats now return 400 "Unknown _extra: <names>". HTML pages continue to ignore unknown names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GrHZSypDfMnym1tM5XJAFZ
This commit is contained in:
parent
9ee95cab3d
commit
e0ba8b3c6a
9 changed files with 69 additions and 7 deletions
|
|
@ -5,6 +5,8 @@ from typing import ClassVar
|
|||
|
||||
from asyncinject import Registry
|
||||
|
||||
from datasette.utils.asgi import BadRequest
|
||||
|
||||
|
||||
def extra_names_from_request(request):
|
||||
extra_bits = request.args.getlist("_extra")
|
||||
|
|
@ -113,6 +115,17 @@ class ExtraRegistry:
|
|||
self._allowed_names[key] = names
|
||||
return names
|
||||
|
||||
def validate_requested(self, requested, scope):
|
||||
"""
|
||||
Raise BadRequest if any requested extra name is not a public extra
|
||||
for this scope. Used by data formats such as .json - HTML pages
|
||||
silently ignore unknown names instead.
|
||||
"""
|
||||
allowed = self._allowed_names_for_scope(scope, include_internal=False)
|
||||
unknown = sorted(name for name in requested if name not in allowed)
|
||||
if unknown:
|
||||
raise BadRequest("Unknown _extra: {}".format(", ".join(unknown)))
|
||||
|
||||
async def resolve(self, requested, context, scope, include_internal=False):
|
||||
allowed_names = self._allowed_names_for_scope(scope, include_internal)
|
||||
requested_names = [name for name in requested if name in allowed_names]
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ import markupsafe
|
|||
import os
|
||||
import textwrap
|
||||
|
||||
from datasette.extras import extra_names_from_request
|
||||
from datasette.extras import extra_names_from_request, ExtraScope
|
||||
from datasette.database import QueryInterrupted
|
||||
from datasette.resources import DatabaseResource, QueryResource
|
||||
from datasette.stored_queries import StoredQuery, stored_query_to_dict
|
||||
|
|
@ -855,6 +855,7 @@ class QueryView(View):
|
|||
raise DatasetteError("?sql= is required", status=400)
|
||||
data = {"ok": True, "rows": rows, "columns": columns}
|
||||
extras = extra_names_from_request(request)
|
||||
table_extra_registry.validate_requested(extras, ExtraScope.QUERY)
|
||||
if extras:
|
||||
query_extra_context = QueryExtraContext(
|
||||
datasette=datasette,
|
||||
|
|
|
|||
|
|
@ -596,6 +596,9 @@ class RowView(BaseView):
|
|||
}
|
||||
|
||||
extras = extra_names_from_request(request)
|
||||
if request.url_vars.get("format"):
|
||||
# Data formats reject unknown extras; HTML ignores them
|
||||
table_extra_registry.validate_requested(extras, ExtraScope.ROW)
|
||||
|
||||
# Process extras
|
||||
row_extra_context = RowExtraContext(
|
||||
|
|
|
|||
|
|
@ -2254,6 +2254,10 @@ async def table_view_data(
|
|||
|
||||
# Resolve extras
|
||||
extras = extra_names_from_request(request)
|
||||
if not extra_extras:
|
||||
# Data formats reject unknown extras; the HTML path (which passes
|
||||
# extra_extras={"_html"}) resolves internal extras of its own
|
||||
table_extra_registry.validate_requested(extras, ExtraScope.TABLE)
|
||||
if any(k for k in request.args.keys() if k == "_facet" or k.startswith("_facet_")):
|
||||
extras.add("facet_results")
|
||||
if request.args.get("_shape") == "object":
|
||||
|
|
|
|||
|
|
@ -283,6 +283,8 @@ These can be repeated or comma-separated:
|
|||
|
||||
?_extra=columns&_extra=count,next_url
|
||||
|
||||
Requesting an ``_extra`` name that does not exist returns a ``400`` error in the :ref:`standard error format <json_api_errors>`, for example ``{"ok": false, "error": "Unknown _extra: nope", ...}``.
|
||||
|
||||
.. [[[cog
|
||||
from json_api_doc import table_extras
|
||||
table_extras(cog)
|
||||
|
|
|
|||
|
|
@ -176,9 +176,10 @@ build JSON directly):
|
|||
Table, row and query JSON responses support `?_extra=` (repeatable and/or
|
||||
comma-separated, extras.py:9-14) to add keys to the response. Extras are
|
||||
scope-registered (`ExtraScope.TABLE` / `ROW` / `QUERY`) and only **public**
|
||||
extras are available over JSON (extras.py:73-92). Unknown extra names are
|
||||
silently ignored. The available names per scope are listed with the relevant
|
||||
endpoints below.
|
||||
extras are available over JSON (extras.py:73-92). Unknown extra names (and
|
||||
internal HTML-only names) on data formats return 400
|
||||
`Unknown _extra: <names>`; HTML pages ignore them. The available names per
|
||||
scope are listed with the relevant endpoints below.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -166,7 +166,11 @@ Endpoints disagree about the success envelope:
|
|||
matches the read API).~~ ✅ **Done** — row update now returns
|
||||
`rows: [{...}]`.
|
||||
|
||||
### 2b. `_extra`/`_shape` support is uneven (P2)
|
||||
### 2b. `_extra`/`_shape` support is uneven (P2) — partially implemented
|
||||
|
||||
> **Status:** unknown `_extra` names on data formats now return 400
|
||||
> `Unknown _extra: <names>` (HTML pages still ignore them). Extending
|
||||
> extras/shaping to database/instance scope remains open.
|
||||
|
||||
The extras system (`?_extra=`, scope-registered) is the 1.0 mechanism for
|
||||
response shaping — but it only exists on table, row and query endpoints. The
|
||||
|
|
|
|||
|
|
@ -570,3 +570,37 @@ async def test_schema_endpoints_no_existence_oracle(tmp_path_factory):
|
|||
async def test_table_schema_unknown_database_is_404_not_500(ds_client):
|
||||
response = await ds_client.get("/no_such_db/some_table/-/schema.json")
|
||||
assert_canonical_error(response, 404)
|
||||
|
||||
|
||||
# Unknown _extra names are a 400, not silently ignored
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"path",
|
||||
(
|
||||
"/fixtures/facetable.json?_extra=nope",
|
||||
"/fixtures/facetable.json?_extra=count,nope",
|
||||
"/fixtures/simple_primary_key/1.json?_extra=nope",
|
||||
"/fixtures/-/query.json?sql=select+1&_extra=nope",
|
||||
),
|
||||
)
|
||||
async def test_unknown_extra_is_400(ds_client, path):
|
||||
response = await ds_client.get(path)
|
||||
data = assert_canonical_error(response, 400)
|
||||
assert data["errors"] == ["Unknown _extra: nope"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_html_only_extra_via_json_is_400(ds_client):
|
||||
# display_rows exists for the HTML view but is not part of the JSON API
|
||||
response = await ds_client.get("/fixtures/facetable.json?_extra=display_rows")
|
||||
data = assert_canonical_error(response, 400)
|
||||
assert data["errors"] == ["Unknown _extra: display_rows"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unknown_extra_ignored_on_html_pages(ds_client):
|
||||
response = await ds_client.get("/fixtures/facetable?_extra=nope")
|
||||
assert response.status_code == 200
|
||||
assert response.headers["content-type"].startswith("text/html")
|
||||
|
|
|
|||
|
|
@ -123,8 +123,8 @@ async def test_html_only_extras_are_not_available_via_json(ds_client, extra):
|
|||
# These extras exist for the HTML view; their values are not JSON
|
||||
# serializable so they are internal, not part of the JSON API
|
||||
response = await ds_client.get(f"/fixtures/facetable.json?_extra={extra}")
|
||||
assert response.status_code == 200
|
||||
assert extra not in response.json()
|
||||
assert response.status_code == 400
|
||||
assert response.json()["errors"] == [f"Unknown _extra: {extra}"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue