From fecbed82b43042a200f71bf73af2034157f84a30 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 13:52:44 +0000 Subject: [PATCH] Keep JSON parameter types for stored write queries and execute-write, refs #2946 Stored write queries converted every JSON value with str(), so null was stored as 'None', true as 'True' and lists as Python repr. execute-write kept JSON types but rejected arrays and objects and silently bound missing parameters as empty strings. Both endpoints now share one function for JSON parameters: - strings, numbers and null pass through, true/false become 1/0, and arrays and objects are stored as JSON text, matching the insert API - missing parameters return a 400 listing them - unknown parameters return a 400 Stored write queries keep their top-level body shape, with _json and csrftoken as the only reserved keys. Parameters declared in "params" are accepted, magic parameters are not required, and a body wrapped in {"params": ...} gets an error explaining the difference. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012CXYAsBaNidHVk4GipkDjs --- datasette/views/database.py | 42 ++++++-- datasette/views/execute_write.py | 2 +- datasette/views/query_helpers.py | 63 +++++++++-- docs/json_api.rst | 4 + docs/sql_queries.rst | 13 +++ tests/test_stored_queries.py | 173 ++++++++++++++++++++++++++++++- 6 files changed, 281 insertions(+), 16 deletions(-) diff --git a/datasette/views/database.py b/datasette/views/database.py index da207601..79020c12 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -41,8 +41,10 @@ from datasette.write_sql import QueryWriteRejected from . import Context from .base import DatasetteError, View, stream_csv from .query_helpers import ( + QueryValidationError, _block_framing, _ensure_stored_query_execution_permissions, + _stored_query_json_parameters, _table_columns, ) from .table_create_alter import _create_table_ui_context @@ -626,23 +628,49 @@ class QueryView(View): # Process the POST body = await request.post_body() body = body.decode("utf-8").strip() - if body.startswith("{") and body.endswith("}"): - params = json.loads(body) - # But we want key=value strings - for key, value in params.items(): - params[key] = str(value) + is_json = request.headers.get("content-type", "").startswith( + "application/json" + ) or (body.startswith("{") and body.endswith("}")) + validation_error = None + if is_json: + try: + params = json.loads(body or "{}") + except json.JSONDecodeError as ex: + params = {} + validation_error = f"Invalid JSON: {ex}" + if not isinstance(params, dict): + params = {} + validation_error = "JSON must be a dictionary" else: params = dict(parse_qsl(body, keep_blank_values=True)) - # Don't ever send csrftoken as a SQL parameter + # csrftoken and _json are reserved keys, never SQL parameters params.pop("csrftoken", None) + json_requested = params.pop("_json", None) # Should we return JSON? should_return_json = ( request.headers.get("accept") == "application/json" or request.args.get("_json") - or params.get("_json") + or json_requested ) + if is_json and validation_error is None: + try: + params = _stored_query_json_parameters(stored_query, params) + except QueryValidationError as ex: + validation_error = ex.message + if validation_error: + if should_return_json: + return Response.json( + dict( + error_body([validation_error], 400), + redirect=stored_query.on_error_redirect, + ), + status=400, + ) + datasette.add_message(request, validation_error, datasette.ERROR) + return Response.redirect(stored_query.on_error_redirect or request.path) + params_for_query = MagicParameters(stored_query.sql, params, request, datasette) await params_for_query.execute_params() ok = None diff --git a/datasette/views/execute_write.py b/datasette/views/execute_write.py index c4f5e3fe..527a1d4b 100644 --- a/datasette/views/execute_write.py +++ b/datasette/views/execute_write.py @@ -363,7 +363,7 @@ class ExecuteWriteView(BaseView): data, is_json = await _json_or_form_payload(request) sql, provided_params = _coerce_execute_write_payload(data, is_json) parameter_names, params, analysis = await _prepare_execute_write( - self.ds, db, sql, provided_params, request.actor + self.ds, db, sql, provided_params, request.actor, is_json=is_json ) except QueryValidationError as ex: if _wants_json(request, is_json, data): diff --git a/datasette/views/query_helpers.py b/datasette/views/query_helpers.py index 725d9cdb..aa406532 100644 --- a/datasette/views/query_helpers.py +++ b/datasette/views/query_helpers.py @@ -302,16 +302,65 @@ def _coerce_execute_write_payload(data, is_json): return data.get("sql"), params -async def _prepare_execute_write(datasette, db, sql, params, actor): +def _reject_unknown_parameters(params, allowed_names): + unknown = set(params) - set(allowed_names) + if unknown: + raise QueryValidationError( + "Unknown parameters: {}".format(", ".join(sorted(unknown))) + ) + + +def _json_parameter_value(value): + # Match the insert API: booleans become 1/0, arrays and objects JSON text + if isinstance(value, bool): + return int(value) + if isinstance(value, (list, dict)): + return json.dumps(value) + return value + + +def _json_sql_parameters(params, allowed_names, required_names): + """ + Validate a dictionary of SQL parameters decoded from a JSON request + body and convert its values to types SQLite can bind. + """ + if not isinstance(params, dict): + raise QueryValidationError("params must be a dictionary") + _reject_unknown_parameters(params, allowed_names) + missing = [name for name in required_names if name not in params] + if missing: + raise QueryValidationError("Missing parameters: {}".format(", ".join(missing))) + return {key: _json_parameter_value(value) for key, value in params.items()} + + +def _stored_query_json_parameters(query: StoredQuery, params): + sql_names = list(dict.fromkeys(derive_named_parameters(query.sql))) + # Parameters declared in "params" are form fields, so accept them too + allowed_names = sql_names + [ + name for name in query.parameters if name not in sql_names + ] + # Magic parameters such as :_actor_id are not supplied by the client + required_names = [name for name in sql_names if not name.startswith("_")] + try: + return _json_sql_parameters(params, allowed_names, required_names) + except QueryValidationError as ex: + if "params" not in allowed_names and isinstance(params.get("params"), dict): + raise QueryValidationError( + ex.message + ". Stored queries take parameters as top-level keys," + ' not inside "params"' + ) from ex + raise + + +async def _prepare_execute_write(datasette, db, sql, params, actor, *, is_json=False): if not sql or not isinstance(sql, str): raise QueryValidationError("SQL is required") parameter_names = _derived_query_parameters(sql) - extra_params = set(params) - set(parameter_names) - if extra_params: - raise QueryValidationError( - "Unknown parameters: {}".format(", ".join(sorted(extra_params))) - ) - params = {name: params.get(name, "") for name in parameter_names} + if is_json: + params = _json_sql_parameters(params, parameter_names, parameter_names) + else: + _reject_unknown_parameters(params, parameter_names) + params = {name: params.get(name, "") for name in parameter_names} try: analysis = await db.analyze_sql(sql, params) except sqlite3.DatabaseError as ex: diff --git a/docs/json_api.rst b/docs/json_api.rst index 73212e70..a573ba34 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -1705,6 +1705,10 @@ The request body must include a ``"sql"`` string. Named SQL parameters can be pr } } +Every named parameter in the SQL must have a value in ``"params"``, and ``"params"`` cannot contain keys that are not used by the SQL. Either mistake results in a ``400`` error such as ``"Missing parameters: name"`` or ``"Unknown parameters: breed"``. + +Parameter values keep their JSON types, using the same rules as the :ref:`insert API `: strings, numbers and ``null`` are passed through unchanged, ``true`` and ``false`` become ``1`` and ``0``, and arrays and objects are passed as JSON text. + The SQL must be writable. Read-only ``select`` queries should use the regular :ref:`custom SQL query JSON API ` instead. Datasette analyzes the SQL before executing it. The actor must have ``execute-write-sql`` permission for the database, and must also have any permissions required by the operations in the SQL. For example, inserts and updates against a table require ``insert-row``, ``update-row`` and ``delete-row`` permissions for that table. Reads performed as part of the write, such as ``insert into dogs select ... from other_table``, require ``view-table`` permission on the source table. Schema changes require ``create-table``, ``alter-table`` or ``drop-table`` permissions as appropriate. diff --git a/docs/sql_queries.rst b/docs/sql_queries.rst index 4c6e4426..f65dfcef 100644 --- a/docs/sql_queries.rst +++ b/docs/sql_queries.rst @@ -642,9 +642,22 @@ Writable queries can also be accessed using a JSON API. You can POST data to the To submit JSON to a writable query, encode key/value parameters as a JSON document:: POST /mydatabase/add_message + Content-Type: application/json {"message": "Message goes here"} +Each top-level key is the name of a parameter in the query's SQL, or one of the parameters listed in its ``"params"`` configuration. Parameters are not wrapped in a ``"params"`` object - that format is used by :ref:`execute-write `, which needs to send the SQL as well. + +The keys ``_json`` and ``csrftoken`` are reserved: they are never passed to the SQL query, so a query cannot use them as parameter names. + +JSON values are passed to SQLite with their types intact, using the same rules as the :ref:`insert API `: + +- Strings, numbers and ``null`` are passed through unchanged. +- ``true`` and ``false`` are passed as ``1`` and ``0``. +- Arrays and objects are passed as JSON text, so they can be used with SQLite functions such as ``json_each()``. + +Every parameter used by the SQL must be included, apart from :ref:`magic parameters `. Missing parameters or unknown keys result in a ``400`` error, for example ``"Missing parameters: message"``. + You can also continue to submit data using regular form encoding, like so:: POST /mydatabase/add_message diff --git a/tests/test_stored_queries.py b/tests/test_stored_queries.py index e0b0e6c6..1f2faefd 100644 --- a/tests/test_stored_queries.py +++ b/tests/test_stored_queries.py @@ -5,6 +5,8 @@ import pytest from asgiref.sync import async_to_sync from bs4 import BeautifulSoup as Soup +from datasette.app import Datasette + from .fixtures import make_app_client @@ -224,7 +226,7 @@ def test_json_post_body(stored_write_client): assert 302 == response.status assert "/data/add_name?success" == response.headers["Location"] rows = stored_write_client.get("/data/names.json?_shape=array").json - assert rows == [{"rowid": 1, "name": "['Hello', 'there']"}] + assert rows == [{"rowid": 1, "name": '["Hello", "there"]'}] @pytest.mark.parametrize( @@ -474,3 +476,172 @@ def test_stored_write_query_disabled_for_immutable_database( ) assert response.status == 403 assert "Database is immutable" in response.text + + +JSON_EVENT_SQL = ( + "insert into events (value, flag, extra, tags, meta) " + "values (:value, :flag, :extra, :tags, :meta)" +) +JSON_EVENT_PARAMS = { + "value": 5, + "flag": True, + "extra": None, + "tags": ["a", "b"], + "meta": {"k": 1}, +} + + +async def _json_params_datasette(): + ds = Datasette( + memory=True, + config={ + "databases": { + "data": { + "queries": { + "add_event": {"sql": JSON_EVENT_SQL, "write": True}, + "add_event_declared": { + "sql": "insert into events (value) values (:value)", + "params": ["value", "note"], + "write": True, + }, + "add_event_actor": { + "sql": "insert into events (value, extra) values (:value, :_actor_id)", + "write": True, + }, + } + } + } + }, + ) + ds.root_enabled = True + db = ds.add_memory_database("stored_query_json_params", name="data") + # Columns without a declared type store values exactly as they are bound + await db.execute_write( + "create table if not exists events (value, flag, extra, tags, meta)" + ) + await db.execute_write("delete from events") + await ds.invoke_startup() + return ds, db + + +async def _post_json_params(ds, endpoint, params): + if endpoint == "stored": + return await ds.client.post( + "/data/add_event", + actor={"id": "root"}, + json=params, + headers={"Accept": "application/json"}, + ) + return await ds.client.post( + "/data/-/execute-write", + actor={"id": "root"}, + json={"sql": JSON_EVENT_SQL, "params": params}, + ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("endpoint", ("stored", "execute-write")) +async def test_json_parameters_keep_types(endpoint): + # https://github.com/simonw/datasette/issues/2946 + ds, db = await _json_params_datasette() + response = await _post_json_params(ds, endpoint, JSON_EVENT_PARAMS) + assert response.status_code == 200 + assert response.json()["ok"] is True + rows = ( + await db.execute( + "select value, typeof(value) as value_type, flag, typeof(flag) as flag_type, " + "extra, typeof(extra) as extra_type, tags, meta from events" + ) + ).dicts() + assert rows == [ + { + "value": 5, + "value_type": "integer", + "flag": 1, + "flag_type": "integer", + "extra": None, + "extra_type": "null", + "tags": '["a", "b"]', + "meta": '{"k": 1}', + } + ] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("endpoint", ("stored", "execute-write")) +@pytest.mark.parametrize( + "params,expected_error", + ( + ({"value": 1}, "Missing parameters: flag, extra, tags, meta"), + ({**JSON_EVENT_PARAMS, "bogus": 1}, "Unknown parameters: bogus"), + ), +) +async def test_json_parameters_missing_or_unknown(endpoint, params, expected_error): + ds, db = await _json_params_datasette() + response = await _post_json_params(ds, endpoint, params) + assert response.status_code == 400 + assert response.json()["errors"] == [expected_error] + assert (await db.execute("select count(*) from events")).single_value() == 0 + + +@pytest.mark.asyncio +async def test_stored_query_json_params_wrapper_hint(): + ds, db = await _json_params_datasette() + response = await ds.client.post( + "/data/add_event", + actor={"id": "root"}, + json={"params": JSON_EVENT_PARAMS}, + headers={"Accept": "application/json"}, + ) + assert response.status_code == 400 + assert response.json()["errors"] == [ + ( + "Unknown parameters: params. Stored queries take parameters as " + 'top-level keys, not inside "params"' + ) + ] + assert (await db.execute("select count(*) from events")).single_value() == 0 + + +@pytest.mark.asyncio +async def test_stored_query_json_declared_and_magic_parameters(): + ds, db = await _json_params_datasette() + # Parameters declared in "params" are accepted even if the SQL does not use them + declared = await ds.client.post( + "/data/add_event_declared", + actor={"id": "root"}, + json={"value": 1, "note": "ignored"}, + headers={"Accept": "application/json"}, + ) + assert declared.status_code == 200 + # Magic parameters are filled in by Datasette, not required from the client + magic = await ds.client.post( + "/data/add_event_actor", + actor={"id": "root"}, + json={"value": 2}, + headers={"Accept": "application/json"}, + ) + assert magic.status_code == 200 + rows = (await db.execute("select value, extra from events order by value")).dicts() + assert rows == [{"value": 1, "extra": None}, {"value": 2, "extra": "root"}] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "body,expected_error", + ( + ("[1, 2]", "JSON must be a dictionary"), + ("{bad json}", "Invalid JSON: "), + ), +) +async def test_stored_query_invalid_json_body(body, expected_error): + ds, db = await _json_params_datasette() + response = await ds.client.post( + "/data/add_event", + actor={"id": "root"}, + content=body, + headers={"Content-Type": "application/json", "Accept": "application/json"}, + ) + assert response.status_code == 400 + assert response.json()["errors"][0].startswith(expected_error) + assert (await db.execute("select count(*) from events")).single_value() == 0