From aee8f0ca5b3de5770886d72c4d704c9f800bc823 Mon Sep 17 00:00:00 2001 From: Zain Dana Harper Date: Mon, 29 Jun 2026 22:03:25 -0700 Subject: [PATCH] Return 400 not 500 for wrong-arity composite-PK row URLs A row URL with the wrong number of comma-separated primary key components for a compound (multi-column) primary key table raised an uncaught sqlite3.ProgrammingError, surfacing as an HTTP 500. row_sql_params_pks builds one SQL bind placeholder per primary-key column but only binds params for the supplied URL components. When too few components are supplied, a trailing placeholder is left unbound and Datasette.resolve_row executes the SQL with no arity guard, so SQLite raises "You did not supply a value for binding parameter :pN" before the 404 recovery can run. When too many components are supplied the extra params are silently ignored and a misleading result is returned. Add a primary-key arity guard in resolve_row that raises BadRequest (HTTP 400), mirroring the existing guard in datasette/views/table.py (len(pk_values) != len(row_pks) -> BadRequest). BadRequest was not previously imported into app.py, so add it to the .utils.asgi import. Fixes #2811 Co-Authored-By: Claude Opus 4.8 --- datasette/app.py | 5 +++++ tests/test_api.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/datasette/app.py b/datasette/app.py index 9c9b7de434..61249a125f 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -132,6 +132,7 @@ ) from .utils.asgi import ( AsgiLifespan, + BadRequest, Forbidden, NotFound, DatabaseNotFound, @@ -2789,6 +2790,10 @@ async def resolve_row(self, request): db, table_name, _ = await self.resolve_table(request) pk_values = urlsafe_components(request.url_vars["pks"]) sql, params, pks = await row_sql_params_pks(db, table_name, pk_values) + if len(pk_values) != len(pks): + raise BadRequest( + "URL row identifier does not match the primary key for this table" + ) results = await db.execute(sql, params, truncate=True) row = results.first() if row is None: diff --git a/tests/test_api.py b/tests/test_api.py index f57d02064b..ecc4214f76 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -371,6 +371,40 @@ async def test_row(ds_client): assert response.json()["rows"] == [{"id": 1, "content": "hello"}] +@pytest.mark.asyncio +@pytest.mark.parametrize("suffix", ("", ".json")) +@pytest.mark.parametrize( + "row_path", + ( + "a", # too few components for a two-column primary key + "a,b,c", # too many components for a two-column primary key + ), +) +async def test_row_pk_arity_mismatch_returns_400(ds_client, row_path, suffix): + # A row URL with the wrong number of comma-separated primary key + # components used to raise an uncaught sqlite3.ProgrammingError (HTTP 500) + # because the SQL had one bind placeholder per PK column but params were + # only bound for the supplied components. It should be a 400 instead, + # mirroring the existing guard in datasette/views/table.py. + response = await ds_client.get( + "/fixtures/compound_primary_key/{}{}".format(row_path, suffix) + ) + assert response.status_code == 400 + if suffix == ".json": + assert response.json()["ok"] is False + assert response.json()["status"] == 400 + + +@pytest.mark.asyncio +async def test_row_compound_pk_correct_arity(ds_client): + # The valid two-component URL still resolves the row. + response = await ds_client.get( + "/fixtures/compound_primary_key/a,b.json?_shape=objects" + ) + assert response.status_code == 200 + assert response.json()["rows"] == [{"pk1": "a", "pk2": "b", "content": "c"}] + + @pytest.mark.asyncio async def test_row_strange_table_name(ds_client): response = await ds_client.get(