mirror of
https://github.com/simonw/datasette.git
synced 2026-07-08 16:44:34 +02:00
Return 400 for query data formats when ?sql= is missing
GET /db/-/query.json with no (or blank) ?sql= previously returned 200 with empty rows, masking caller bugs, while the .csv format returned 400 "?sql= is required" for the same request. All data formats now return the 400; the HTML SQL editor page is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GrHZSypDfMnym1tM5XJAFZ
This commit is contained in:
parent
aaaffe45b8
commit
ea9c1b1524
5 changed files with 37 additions and 6 deletions
|
|
@ -851,6 +851,8 @@ class QueryView(View):
|
|||
|
||||
return await stream_csv(datasette, fetch_data_for_csv, request, db.name)
|
||||
elif format_ in datasette.renderers.keys():
|
||||
if not sql:
|
||||
raise DatasetteError("?sql= is required", status=400)
|
||||
data = {"ok": True, "rows": rows, "columns": columns}
|
||||
extras = extra_names_from_request(request)
|
||||
if extras:
|
||||
|
|
|
|||
|
|
@ -486,8 +486,9 @@ queries section).
|
|||
HTTP 400 `{"ok": false, "error": "<message>", "rows": [], "truncated": false}`.
|
||||
- Time limit → 400 titled `"SQL Interrupted"` (the `error` value contains
|
||||
an HTML fragment).
|
||||
- `?sql=` omitted → 200 `{"ok": true, "rows": [], "truncated": false}`
|
||||
(the CSV format instead errors 400 `"?sql= is required"`).
|
||||
- `?sql=` omitted or blank → 400 `"?sql= is required"` for all data
|
||||
formats (`.json`, `.csv`, plugin formats). The HTML page remains the
|
||||
SQL editor.
|
||||
- `.csv` streams CSV; unknown extensions → 404.
|
||||
|
||||
### GET /\<database\>/-/query/parameters
|
||||
|
|
|
|||
|
|
@ -338,9 +338,11 @@ Concerns:
|
|||
`is_trusted` — an actor with `delete-query` can delete a config-defined
|
||||
trusted query via the API (it will resync on restart, making the
|
||||
behavior confusing rather than catastrophic). Align delete with update.
|
||||
2. **GET `/db/-/query` with no `?sql=` returns 200 `{"ok": true, "rows":
|
||||
2. ~~**GET `/db/-/query` with no `?sql=` returns 200 `{"ok": true, "rows":
|
||||
[]}`** while `.csv` on the same request returns 400 `"?sql= is
|
||||
required"`. The JSON behavior masks caller bugs; return 400 on both.
|
||||
required"`. The JSON behavior masks caller bugs; return 400 on both.~~
|
||||
✅ **Done** — all data formats now return 400; the HTML SQL editor page
|
||||
is unchanged.
|
||||
3. **`_shape=object` HTTP 200 error** (§1b) — almost certainly unintended.
|
||||
4. ~~**Row delete 500** (§1c) — inconsistent with every sibling endpoint.~~
|
||||
✅ Done — now 400.
|
||||
|
|
|
|||
|
|
@ -483,3 +483,27 @@ async def test_token_when_signed_tokens_disabled_returns_401(tmp_path_factory):
|
|||
assert "not enabled" in data["error"]
|
||||
finally:
|
||||
ds.close()
|
||||
|
||||
|
||||
# GET /db/-/query without SQL: 400 for data formats, HTML editor stays 200
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"path",
|
||||
(
|
||||
"/fixtures/-/query.json",
|
||||
"/fixtures/-/query.json?sql=",
|
||||
),
|
||||
)
|
||||
async def test_query_json_without_sql_is_400(ds_client, path):
|
||||
response = await ds_client.get(path)
|
||||
data = assert_canonical_error(response, 400)
|
||||
assert data["errors"] == ["?sql= is required"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_query_html_without_sql_is_still_the_editor(ds_client):
|
||||
response = await ds_client.get("/fixtures/-/query")
|
||||
assert response.status_code == 200
|
||||
assert response.headers["content-type"].startswith("text/html")
|
||||
|
|
|
|||
|
|
@ -180,9 +180,11 @@ def test_query_extra_query_reports_bound_params():
|
|||
assert response.json["query"]["params"] == {}
|
||||
|
||||
|
||||
def test_query_extra_query_does_not_echo_querystring_without_sql():
|
||||
def test_query_extra_query_does_not_echo_querystring():
|
||||
with make_app_client() as client:
|
||||
response = client.get("/fixtures/-/query.json?_extra=query&foo=bar")
|
||||
response = client.get(
|
||||
"/fixtures/-/query.json?sql=select+1&_extra=query&foo=bar"
|
||||
)
|
||||
assert response.status == 200
|
||||
assert response.json["query"]["params"] == {}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue