mirror of
https://github.com/simonw/datasette.git
synced 2026-06-12 12:06:57 +02:00
Fix _extra=query to report the params that were actually bound
QueryExtra re-derived named parameters from the SQL with a regex, which missed parameters declared in a stored query's params list, reported magic _-prefixed parameters with raw querystring values that were never bound, and echoed the entire querystring when no SQL was present. QueryView now passes its named_parameter_values dict - the parameters it actually bound - through QueryExtraContext. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
ab62ec96d1
commit
8f888515b6
3 changed files with 40 additions and 11 deletions
|
|
@ -717,7 +717,7 @@ class QueryView(View):
|
|||
rows=rows,
|
||||
columns=columns,
|
||||
sql=sql,
|
||||
params=params_for_query,
|
||||
params=named_parameter_values,
|
||||
query_name=stored_query.name if stored_query else None,
|
||||
stored_query=stored_query,
|
||||
stored_query_write=stored_query_write,
|
||||
|
|
|
|||
|
|
@ -8,7 +8,6 @@ from datasette.resources import TableResource
|
|||
from datasette.utils import (
|
||||
await_me_maybe,
|
||||
call_with_supported_arguments,
|
||||
named_parameters as derive_named_parameters,
|
||||
path_with_added_args,
|
||||
path_with_format,
|
||||
path_with_removed_args,
|
||||
|
|
@ -592,17 +591,9 @@ class QueryExtra(Extra):
|
|||
scopes = frozenset({ExtraScope.TABLE, ExtraScope.ROW, ExtraScope.QUERY})
|
||||
|
||||
async def resolve(self, context):
|
||||
params = context.params
|
||||
if context.scope == ExtraScope.QUERY and context.sql:
|
||||
parameter_names = set(derive_named_parameters(context.sql))
|
||||
params = {
|
||||
key: value
|
||||
for key, value in dict(context.params).items()
|
||||
if key in parameter_names
|
||||
}
|
||||
return {
|
||||
"sql": context.sql,
|
||||
"params": params,
|
||||
"params": context.params,
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -131,6 +131,44 @@ def test_query_extra_private_for_arbitrary_sql():
|
|||
assert anon.status == 403
|
||||
|
||||
|
||||
def test_query_extra_query_reports_bound_params():
|
||||
config = {
|
||||
"databases": {
|
||||
"fixtures": {
|
||||
"queries": {
|
||||
"declared_params": {
|
||||
"sql": "select 1 as one",
|
||||
"params": ["foo"],
|
||||
},
|
||||
"magic_host": {
|
||||
"sql": "select :_header_host as h",
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
with make_app_client(config=config) as client:
|
||||
# Declared parameters are reported even when the regex cannot find them
|
||||
response = client.get("/fixtures/declared_params.json?foo=bar&_extra=query")
|
||||
assert response.status == 200
|
||||
assert response.json["query"]["params"] == {"foo": "bar"}
|
||||
# Magic parameters are bound internally and should not be reported,
|
||||
# especially not as a value taken from the querystring
|
||||
response = client.get(
|
||||
"/fixtures/magic_host.json?_extra=query&_header_host=spoofed"
|
||||
)
|
||||
assert response.status == 200
|
||||
assert response.json["rows"] == [{"h": "localhost"}]
|
||||
assert response.json["query"]["params"] == {}
|
||||
|
||||
|
||||
def test_query_extra_query_does_not_echo_querystring_without_sql():
|
||||
with make_app_client() as client:
|
||||
response = client.get("/fixtures/-/query.json?_extra=query&foo=bar")
|
||||
assert response.status == 200
|
||||
assert response.json["query"]["params"] == {}
|
||||
|
||||
|
||||
def test_query_extra_private_false_when_sql_is_public():
|
||||
with make_app_client() as client:
|
||||
response = client.get(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue