diff --git a/datasette/views/database.py b/datasette/views/database.py index cf7a6db3..99e85d2b 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -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: diff --git a/existing-api.md b/existing-api.md index 8c6e1c4e..9947edeb 100644 --- a/existing-api.md +++ b/existing-api.md @@ -486,8 +486,9 @@ queries section). HTTP 400 `{"ok": false, "error": "", "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 /\/-/query/parameters diff --git a/stable-api-recommendations.md b/stable-api-recommendations.md index 273a272c..11f0a75d 100644 --- a/stable-api-recommendations.md +++ b/stable-api-recommendations.md @@ -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. diff --git a/tests/test_error_shape.py b/tests/test_error_shape.py index 49b2c314..9d4a566c 100644 --- a/tests/test_error_shape.py +++ b/tests/test_error_shape.py @@ -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") diff --git a/tests/test_table_api.py b/tests/test_table_api.py index c8ba31b7..41c89f39 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -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"] == {}