From b4c280c27aa4e24afb115b3a691f0ed59734edef Mon Sep 17 00:00:00 2001 From: clombion <8268300+clombion@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:28:44 +0200 Subject: [PATCH] Check _next on SQL views: 400 for a non-integer, clamp a negative to zero A view pages by row offset, so table_view_data converted _next with a bare int() and any other value raised an unhandled ValueError (HTTP 500). Raise BadRequest instead, as the same function does for a bad _size. Clamp a negative offset to zero before it is used for both the query and the next link, so a view handles a negative _next as a table does: the first page, with a next link that continues from it. Previously the next link was computed from the negative value and came out missing, negative or overlapping the first page. Fixes #2982 --- datasette/views/table.py | 11 +++++++++-- tests/test_table_api.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/datasette/views/table.py b/datasette/views/table.py index 0a908c69..0192dae4 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -2146,8 +2146,15 @@ async def table_view_data( if _next: sort_value = None if is_view: - # _next is an offset - offset = f" offset {int(_next)}" + # _next is an offset. A negative one starts from the first row, as a + # negative keyset token does for a table, and pagination continues + # from there. + try: + offset_value = max(int(_next), 0) + except ValueError: + raise BadRequest("_next must be an integer") + _next = str(offset_value) + offset = f" offset {offset_value}" else: components = urlsafe_components(_next) # If a sort order is applied and there are multiple components, diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 58a5ddc3..a0c49134 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -355,6 +355,37 @@ async def test_validate_page_size(ds_client, path, expected_error): assert response.status_code == 400 +@pytest.mark.asyncio +@pytest.mark.parametrize( + "next_value", + ( + "dog", + "1e3", + # A facet link whose "§" a client decoded as an HTML entity + "25%C2%A7or_name%3DOil", + ), +) +async def test_validate_view_next(ds_client, next_value): + # Views paginate by offset, so _next has to be an integer + response = await ds_client.get(f"/fixtures/paginated_view.json?_next={next_value}") + assert response.status_code == 400 + assert response.json()["error"] == "_next must be an integer" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("next_value", ("-2", "-5", "-10")) +async def test_view_negative_next_starts_from_first_row(ds_client, next_value): + # As for a table, a negative _next starts from the first row and the + # next link continues the pagination from there + negative = await ds_client.get( + f"/fixtures/paginated_view.json?_size=5&_next={next_value}" + ) + first = await ds_client.get("/fixtures/paginated_view.json?_size=5") + assert negative.status_code == 200 + assert negative.json()["rows"] == first.json()["rows"] + assert negative.json()["next"] == first.json()["next"] == "5" + + @pytest.mark.asyncio async def test_page_size_zero(ds_client): """For _size=0 we return the counts, empty rows and no continuation token"""