mirror of
https://github.com/simonw/datasette.git
synced 2026-10-18 07:17:00 +02:00
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012CXYAsBaNidHVk4GipkDjs
This commit is contained in:
parent
cf113a5985
commit
fecbed82b4
6 changed files with 281 additions and 16 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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 <TableInsertView>`: 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 <json_api_custom_sql>` 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.
|
||||
|
|
|
|||
|
|
@ -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 <ExecuteWriteView>`, 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 <TableInsertView>`:
|
||||
|
||||
- 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 <queries_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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue