diff --git a/app/assets/services/cursor.py b/app/assets/services/cursor.py index f3eba7e9f..4e291a355 100644 --- a/app/assets/services/cursor.py +++ b/app/assets/services/cursor.py @@ -9,11 +9,10 @@ runtimes. Payload JSON uses short keys to keep the encoded length small: The `o` key binds the cursor to the sort direction it was minted under, so replaying a `desc` cursor against an `asc` request fails with ``INVALID_CURSOR`` rather than silently walking the wrong direction. -Decoders accept payloads without `o` for backward compatibility with -cursors minted before the binding was introduced (these skip the order -check); new cursors always include it. Cloud has a follow-up to mirror -the field — until then, Python and cloud cursors differ by exactly the -`o` key. +`o` is mandatory on every payload — a cursor without it is rejected as +malformed. Cloud will land the same field in its mirror PR; until then, +Python and cloud cursors differ by exactly the `o` key, and a cloud- +minted cursor cannot be decoded by this endpoint. Encoding is base64url with no padding. JSON serialization escapes `<`, `>`, `&`, U+2028, and U+2029 to match Go's default `json.Marshal` @@ -61,9 +60,7 @@ class CursorPayload: sort_field: str value: str id: str - # None means "minted by a producer that did not bind order" (e.g. a cloud - # cursor from before BE-944's follow-up lands). New cursors always set it. - order: str | None = None + order: str # Order direction tokens. Mirrored on the cloud follow-up so cursors carrying @@ -123,9 +120,8 @@ def decode_cursor( timestamp string compared against a ``name`` column). ``expected_order`` (``"asc"``/``"desc"``), when supplied, must match the - payload's ``o`` field. Cursors minted without ``o`` (e.g. by an older - cloud build) pass the check unconditionally — the binding is best-effort - until both runtimes ship the field. + payload's ``o`` field. ``o`` is required on every payload; a cursor + missing it is rejected as malformed. Passing no allowed fields rejects every cursor. """ @@ -151,7 +147,7 @@ def decode_cursor( sort_field = decoded.get("s") value = decoded.get("v") id = decoded.get("id") - order = decoded.get("o") # may be absent on legacy cursors + order = decoded.get("o") if not isinstance(sort_field, str) or not isinstance(value, str) or not isinstance(id, str): raise InvalidCursorError("payload: missing or non-string s/v/id") @@ -166,11 +162,11 @@ def decode_cursor( if sort_field not in allowed_sort_fields: raise InvalidCursorError(f"unsupported sort field {sort_field!r}") - if order is not None and not isinstance(order, str): - raise InvalidCursorError("payload: non-string o") - if order is not None and order not in _VALID_ORDERS: + if not isinstance(order, str): + raise InvalidCursorError("missing or non-string o") + if order not in _VALID_ORDERS: raise InvalidCursorError(f"unsupported order {order!r}") - if expected_order is not None and order is not None and order != expected_order: + if expected_order is not None and order != expected_order: raise InvalidCursorError( f"cursor order {order!r} does not match request order {expected_order!r}" ) diff --git a/tests-unit/assets_test/services/test_cursor.py b/tests-unit/assets_test/services/test_cursor.py index 7b5103866..fb6acefee 100644 --- a/tests-unit/assets_test/services/test_cursor.py +++ b/tests-unit/assets_test/services/test_cursor.py @@ -61,7 +61,7 @@ class TestTimeCursor: assert decoded == ts def test_decode_returns_utc(self): - payload = CursorPayload(sort_field="created_at", value="1716200000123456", id="id-1") + payload = CursorPayload(sort_field="created_at", value="1716200000123456", id="id-1", order="desc") decoded = decode_cursor_time(payload) assert decoded.tzinfo == timezone.utc @@ -72,7 +72,7 @@ class TestTimeCursor: def test_non_integer_value_rejected_on_decode(self): with pytest.raises(InvalidCursorError): - decode_cursor_time(CursorPayload("created_at", "not-a-number", "id-1")) + decode_cursor_time(CursorPayload("created_at", "not-a-number", "id-1", "desc")) def test_none_payload_rejected(self): with pytest.raises(InvalidCursorError): @@ -89,11 +89,11 @@ class TestTimeCursor: class TestIntCursor: def test_decode_int(self): - assert decode_cursor_int(CursorPayload("size", "1024", "id-1")) == 1024 + assert decode_cursor_int(CursorPayload("size", "1024", "id-1", "desc")) == 1024 def test_decode_int_rejects_non_int(self): with pytest.raises(InvalidCursorError): - decode_cursor_int(CursorPayload("size", "abc", "id-1")) + decode_cursor_int(CursorPayload("size", "abc", "id-1", "desc")) def test_decode_int_rejects_none(self): with pytest.raises(InvalidCursorError): @@ -223,14 +223,14 @@ class TestOrderBinding: with pytest.raises(InvalidCursorError, match="unsupported order"): decode_cursor(encoded, ALLOWED) - def test_legacy_cursor_without_order_accepted(self): - """Cursors minted by a producer that didn't include `o` (e.g. an older - cloud build) must still decode — the order binding is best-effort - until cloud mirrors the field.""" + def test_cursor_without_order_rejected(self): + """`o` is mandatory. A cursor minted without it is rejected as + malformed rather than silently walking the keyset in whatever + direction the request happens to ask for.""" raw = b'{"s":"name","v":"x","id":"id-1"}' encoded = base64.urlsafe_b64encode(raw).rstrip(b"=").decode("ascii") - payload = decode_cursor(encoded, ALLOWED, expected_order="desc") - assert payload.order is None # binding skipped, decode succeeds + with pytest.raises(InvalidCursorError, match="missing or non-string o"): + decode_cursor(encoded, ALLOWED, expected_order="desc") class TestGoCompatJsonEscaping: