Merge a47f7fed4c into 10088dfa1d

datasette/utils/__init__.py

@@ -1086,6 +1086,11 @@ class MultiParams:
         """Return full list"""
         return self._data.get(name) or []
 
+    def items(self):
+        """Yield (key, first_value) pairs, matching ``__getitem__`` semantics."""
+        for key, values in self._data.items():
+            yield key, values[0]
+
 
 class ConnectionProblem(Exception):
     pass

datasette/utils/asgi.py

@@ -17,7 +17,7 @@ from datasette.utils.multipart import (
     DEFAULT_MIN_FREE_DISK_BYTES,
 )
 from mimetypes import guess_type
-from urllib.parse import parse_qs, urlunparse, parse_qsl
+from urllib.parse import parse_qs, urlunparse
 from pathlib import Path
 from http.cookies import SimpleCookie, Morsel
 import aiofiles
@@ -193,7 +193,7 @@ class Request:
 
     async def post_vars(self):
         body = await self.post_body()
-        return dict(parse_qsl(body.decode("utf-8"), keep_blank_values=True))
+        return MultiParams(parse_qs(qs=body.decode("utf-8"), keep_blank_values=True))
 
     async def json(self):
         body = await self.post_body()

datasette/views/query_helpers.py

@@ -279,9 +279,9 @@ def _execute_write_disabled_reason(sql, analysis_error, analysis_rows):
 
 
 def _coerce_execute_write_payload(data, is_json):
-    if not isinstance(data, dict):
-        raise QueryValidationError("JSON must be a dictionary")
     if is_json:
+        if not isinstance(data, dict):
+            raise QueryValidationError("JSON must be a dictionary")
         invalid_keys = set(data) - {"sql", "params"}
         if invalid_keys:
             raise QueryValidationError(

datasette/views/stored_queries.py

@@ -370,14 +370,17 @@ class QueryStoreView(QueryCreateView):
         query_data = {}
         try:
             data, is_json = await _json_or_form_payload(request)
-            if not isinstance(data, dict):
-                raise QueryValidationError("JSON must be a dictionary")
-            query_data = data.get("query") if is_json else data
-            if not isinstance(query_data, dict):
-                raise QueryValidationError("JSON must contain a query dictionary")
+            if is_json:
+                if not isinstance(data, dict):
+                    raise QueryValidationError("JSON must be a dictionary")
+                query_data = data.get("query")
+                if not isinstance(query_data, dict):
+                    raise QueryValidationError("JSON must contain a query dictionary")
+            else:
+                query_data = data
             prepared = await _prepare_query_create(self.ds, request, db, query_data)
         except QueryValidationError as ex:
-            if not is_json and isinstance(query_data, dict):
+            if not is_json:
                 return await self._error_response(
                     request, db, query_data, ex.message, ex.status
                 )
@@ -388,7 +391,7 @@ class QueryStoreView(QueryCreateView):
         try:
             await self.ds.add_query(db.name, name, replace=False, **prepared)
         except sqlite3.IntegrityError as ex:
-            if not is_json and isinstance(query_data, dict):
+            if not is_json:
                 return await self._error_response(request, db, query_data, str(ex), 400)
             return Response.error([str(ex)], 400)
 
@@ -452,8 +455,8 @@ class QueryUpdateView(BaseView):
             )
 
         try:
-            data, _ = await _json_or_form_payload(request)
-            if not isinstance(data, dict):
+            data, is_json = await _json_or_form_payload(request)
+            if is_json and not isinstance(data, dict):
                 raise QueryValidationError("JSON must be a dictionary")
             invalid_keys = set(data) - {"update", "return"}
             if invalid_keys:
@@ -555,8 +558,8 @@ class QueryEditView(BaseView):
         if existing.is_trusted:
             return Response.error(["Trusted queries cannot be edited"], 403)
 
-        data, _ = await _json_or_form_payload(request)
-        if not isinstance(data, dict):
+        data, is_json = await _json_or_form_payload(request)
+        if is_json and not isinstance(data, dict):
             return Response.error(["Invalid form submission"], 400)
         sql = data.get("sql")
         sql = existing.sql if sql is None else sql.strip()

docs/changelog.rst

@@ -196,6 +196,7 @@ Bug fixes
 
 - Fixed a bug where visiting ``/<database>/-/query`` without a ``?sql=`` parameter returned a 500 error. (:issue:`2743`)
 - The ``datasette inspect`` command now correctly records row counts for tables with more than 10,000 rows. (:issue:`2712`)
+- ``await request.post_vars()`` now returns a :ref:`MultiParams <internals_multiparams>` object instead of a ``dict``, so multiple values for the same form field are preserved. Use ``.getlist(key)`` to retrieve every value. Existing ``post_vars[key]`` access continues to work, but now returns the *first* submitted value rather than the *last* (matching ``request.args`` semantics). (:issue:`2425`)
 
 .. _v1_0_a30:
 

docs/internals.rst

@@ -106,8 +106,8 @@ The object also has the following awaitable methods:
 
     Don't forget to read about :ref:`internals_csrf`!
 
-``await request.post_vars()`` - dictionary
-    Returns a dictionary of form variables that were submitted in the request body via ``POST`` using ``application/x-www-form-urlencoded`` encoding. For multipart forms or file uploads, use ``request.form()`` instead.
+``await request.post_vars()`` - MultiParams
+    Returns a :ref:`MultiParams <internals_multiparams>` object of form variables that were submitted in the request body via ``POST`` using ``application/x-www-form-urlencoded`` encoding. This has the same shape as ``request.args``, so use ``.getlist(key)`` to retrieve every value submitted for keys with multiple values (such as ``<select multiple>`` fields). For multipart forms or file uploads, use ``request.form()`` instead.
 
 ``await request.json()`` - Any
     Returns the parsed JSON body of a request submitted by ``POST``.

tests/plugins/my_plugin.py

@@ -237,7 +237,8 @@ def register_routes():
         if request.method == "GET":
             return Response.html(request.scope["csrftoken"]())
         else:
-            return Response.json(await request.post_vars())
+            # post_vars() returns a MultiParams; convert to a plain dict for JSON
+            return Response.json(dict(await request.post_vars()))
 
     async def csrftoken_form(request, datasette):
         return Response.html(

tests/test_internals_request.py

@@ -33,28 +33,44 @@ def _receive_chunks(chunks):
     return receive
 
 
+def _form_request(body: bytes) -> Request:
+    return Request(
+        _post_scope(headers=[[b"content-type", b"application/x-www-form-urlencoded"]]),
+        _receive_chunks([body]),
+    )
+
+
 @pytest.mark.asyncio
 async def test_request_post_vars():
-    scope = {
-        "http_version": "1.1",
-        "method": "POST",
-        "path": "/",
-        "raw_path": b"/",
-        "query_string": b"",
-        "scheme": "http",
-        "type": "http",
-        "headers": [[b"content-type", b"application/x-www-form-urlencoded"]],
-    }
+    request = _form_request(b"foo=bar&baz=1&empty=")
+    post_vars = await request.post_vars()
+    assert post_vars["foo"] == "bar"
+    assert post_vars["baz"] == "1"
+    assert post_vars["empty"] == ""
+    assert post_vars.get("missing") is None
+    assert set(post_vars.keys()) == {"foo", "baz", "empty"}
+    assert dict(post_vars.items()) == {"foo": "bar", "baz": "1", "empty": ""}
 
-    async def receive():
-        return {
-            "type": "http.request",
-            "body": b"foo=bar&baz=1&empty=",
-            "more_body": False,
-        }
 
-    request = Request(scope, receive)
-    assert {"foo": "bar", "baz": "1", "empty": ""} == await request.post_vars()
+@pytest.mark.asyncio
+async def test_request_post_vars_multi():
+    # post_vars() returns a MultiParams so multiple values for the same key are
+    # preserved, matching the behaviour of request.args. See issue #2425.
+    request = _form_request(b"multi=1&multi=2&single=3")
+    post_vars = await request.post_vars()
+    assert post_vars.get("multi") == "1"
+    assert post_vars.get("single") == "3"
+    assert post_vars["multi"] == "1"
+    assert post_vars["single"] == "3"
+    assert post_vars.getlist("multi") == ["1", "2"]
+    assert post_vars.getlist("single") == ["3"]
+    assert post_vars.getlist("missing") == []
+    assert "multi" in post_vars
+    assert "missing" not in post_vars
+    assert list(post_vars.keys()) == ["multi", "single"]
+    assert len(post_vars) == 2
+    with pytest.raises(KeyError):
+        post_vars["missing"]
 
 
 @pytest.mark.asyncio