Reject malformed record queries instead of silently ignoring them

[eastagiletracker]

This PR proposes rejecting malformed record-query parameters instead of silently ignoring them, so a partition_filter is never quietly widened to every partition of a topic. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/386. You can sign in with your GitHub ID to claim ownership of the project.

What goes wrong today

POST /topic/{topic}/records takes its query string through Option<Query<TopicIterationQuery>>. Because Option<T> turns an extractor rejection into None, a single parameter that fails to parse discards the whole query string and every parameter silently reverts to its default — partition_filter included. A caller who scopes a read to one partition and mistypes an unrelated parameter gets records from every partition back, with a 200 and nothing in the response to say the filter was dropped.

PartitionSelector has a matching fallback of its own: it is deserialized through From<T: AsRef<str>>, which treats a regex: selector that does not compile as an exact partition name. That name matches no partition, so the request reads as an empty topic — again with a 200.

Reproduced on main at 5c3d741, with 5 records appended to partition alpha and 5 to beta, iterating the topic over HTTP:

partition_filter[]=alpha                     ->  HTTP 200 , 5 records
partition_filter[]=alpha&order=desc          ->  HTTP 200 , 5 records
partition_filter[]=alpha&order=descending    ->  HTTP 200 , 10 records
partition_filter[]=regex:^alpha$             ->  HTTP 200 , 5 records
partition_filter[]=regex:(?=alpha)           ->  HTTP 200 , 0 records

The third row is the filter being dropped: descending is not a valid TopicIterationOrder, so the filter goes with it and the response carries both partitions. The fifth row is the silent selector fallback — the regex crate has no look-around, so (?=alpha) becomes the literal partition name regex:(?=alpha).

The change

Query now treats a missing query string as an empty one, so a request with no parameters still parses to the defaults, and it reports a serde_qs failure through ErrorReply as the same 400 with a JSON {code, message} body that the rest of the API returns (it was a bare 406 with an empty body). With that in place the iteration route can take Query<TopicIterationQuery> as a required extractor, so a malformed query string is reported rather than discarded. PartitionSelector gains a fallible parse, used by a hand-written Deserialize, so an uncompilable regex: selector is reported when it arrives over the wire.

On backward compatibility: every valid query is unaffected, including requests that send no query string at all, and PartitionSelector::from keeps its infallible fallback so in-process callers and your partition_selector_invalid_regex test behave exactly as before. The visible change is that a query string that could not be parsed now returns 400 instead of being ignored (iteration route) or answered with 406 (partition route). ErrorReply::InvalidQueryString is additive, leaving the existing variants alone.

Verification

The four tests added to server/tests/server.rs drive the real server over HTTP. Three of them fail on main before the fix and pass after it; the fourth is a control that passes on both, covering an unfiltered request with no query string, an exact-name filter and an equivalent regex: filter:

$ cargo test -p plateau-server --test server
---- topic_iterate_rejects_malformed_query stdout ----
assertion `left == right` failed
  left: 200
 right: 400
---- topic_iterate_rejects_uncompilable_partition_regex stdout ----
assertion `left == right` failed
  left: 200
 right: 400
---- partition_records_rejects_malformed_query stdout ----
assertion `left == right` failed
  left: 406
 right: 400
test result: FAILED. 1 passed; 3 failed

Two unit tests in transport cover PartitionSelector directly: uncompilable patterns (look-around, an unbalanced group, and one past the compiled size limit) are rejected by parse and by Deserialize while from still falls back, and valid exact and regex: selectors still deserialize and match.

make test (cargo test --workspace --features batch,polars,replicate) goes from 168 passing to 174 passing with no failures before or after, and make check-fmt and make lint are both clean. One thing worth flagging: #94 edits server/src/axum_util/query.rs and server/src/http.rs for the axum 0.8 move, so this will want a rebase if that lands first. The port is small — under axum 0.8 the OptionalFromRequestParts impl that PR adds is what keeps the silent fallback alive, and the iteration route would simply stay on the required Query extractor.

How this was managed

This work was tracked as a single story on a board imported from this repository's own issues and pull requests: Reject malformed record-query parameters instead of silently ignoring them, on the board at https://eastagiletracker.com/projects/386.

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.

---

Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com