Skip to content

Return MultiParams from request.post_vars() to handle multi-valued keys#2755

Open
aicayzer wants to merge 1 commit into
simonw:mainfrom
aicayzer:fix/post-vars-multiparams
Open

Return MultiParams from request.post_vars() to handle multi-valued keys#2755
aicayzer wants to merge 1 commit into
simonw:mainfrom
aicayzer:fix/post-vars-multiparams

Conversation

@aicayzer
Copy link
Copy Markdown

Closes #2425.

Problem

request.post_vars() is implemented as dict(parse_qsl(body, keep_blank_values=True)), which silently collapses any duplicate keys to the last value. Forms that submit multiple values for the same field — e.g. <select multiple>, multi-checkbox groups, ?_facet=...&_facet=... style POSTs — lose data with no error. request.args already handles this correctly via MultiParams.

Fix

Return a MultiParams built from parse_qs(...) instead of a dict, matching the shape of request.args. MultiParams was already imported in asgi.py, so no new imports.

Added an items() method to MultiParams that yields (key, first_value) pairs (matching __getitem__ semantics), so MultiParams can be passed directly into dict(), dict-comprehension patterns and similar.

The internal _json_or_form_payload helper feeds two callers — _coerce_execute_write_payload and the QueryStoreView / QueryUpdateView POST handlers — that both had isinstance(data, dict) guards intended to validate JSON shape. Those guards spuriously rejected the new MultiParams on the form path, so they have been gated on is_json. The fall-through logic that re-renders the HTML form on validation errors uses .get() on query_data and works against either dict or MultiParams.

Behavioural notes

  • post_vars[key] and post_vars.get(key) continue to work but now return the first submitted value rather than the last. This matches request.args semantics and is documented in the changelog.
  • post_vars.getlist(key) now returns the full list of submitted values — the new capability the issue asked for.
  • Plugins doing Response.json(await request.post_vars()) should wrap in dict(...) because MultiParams is not directly JSON-serialisable. The bundled my_plugin test fixture has been updated to demonstrate this.

Test plan

  • Updated test_request_post_vars to assert MultiParams behaviour against the previously-existing single-value payload.
  • New test_request_post_vars_multi mirrors test_request_args, covering [], get, getlist, in, keys, len and KeyError for missing keys.
  • Full test suite passes (pytest --ignore=tests/test_multipart.py -n auto: 1853 passed). tests/test_multipart.py has a pre-existing ModuleNotFoundError: multipart_form_data_conformance unrelated to this change.

Files touched

  • datasette/utils/asgi.pypost_vars() returns MultiParams.
  • datasette/utils/__init__.pyMultiParams.items().
  • datasette/views/query_helpers.py — gate isinstance(data, dict) on is_json in _coerce_execute_write_payload.
  • datasette/views/stored_queries.py — gate isinstance guards on is_json in QueryStoreView.post and QueryUpdateView.post; drop the now-redundant isinstance(query_data, dict) checks in the error fall-through branches.
  • tests/test_internals_request.py — updated and added tests.
  • tests/plugins/my_plugin.py — wrap post_vars() in dict() for JSON serialisation.
  • docs/internals.rst and docs/changelog.rst — documentation updates.

…form fields

Form submissions with multiple values for the same key (e.g. ``<select
multiple>``) previously had every value but the last silently discarded,
because ``post_vars()`` called ``dict(parse_qsl(...))``. Switch the
implementation to return a ``MultiParams`` object built from
``parse_qs(...)``, mirroring the existing ``request.args`` shape so both
sides of the GET/POST surface behave consistently.

``MultiParams`` gains an ``items()`` method that yields (key, first_value)
pairs, matching ``__getitem__`` semantics, so it works in patterns such as
``dict(post_vars())`` and the existing ``_coerce_execute_write_payload``
dict comprehension.

Internal callers of ``_json_or_form_payload`` (``execute_write`` and
``stored_queries``) had ``isinstance(data, dict)`` guards intended to
validate JSON shape but also rejected the new ``MultiParams`` return on
the form path. Gate those guards on ``is_json`` so the form path passes
through unchanged.

The bundled ``my_plugin`` test fixture wraps ``post_vars()`` in ``dict()``
before passing to ``Response.json``, demonstrating the migration path for
plugins that need a JSON-serialisable mapping.

Closes simonw#2425
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

request.post_vars() cannot handle multiple values for a key

1 participant