Repository navigation
fix(relay): saturate filter limits instead of casting - #9230
Draft
ThePumpingLemma wants to merge 1 commit into
Draft
ThePumpingLemma wants to merge 1 commit into
ThePumpingLemma wants to merge 1 commit into
Conversation
Problem: four request paths turned a client-supplied `usize` filter limit into the query limit with an inline `as` cast before clamping: `(l as i64).min(max)` (REQ query params, bridge feed) and `(l as u32).min(max)` (WS NIP-50 search, bridge channel window). A limit above i64::MAX becomes negative (`usize::MAX as i64 == -1`): on REQ it reaches Postgres as `LIMIT -1` and fails the query with a database error; on the bridge feed it trips `feed_count >= limit` and returns an empty feed. A limit that is a multiple of 2^32 wraps to 0 through `as u32`, silently returning no rows (or, in the window, one row via `.max(1)`) instead of the page max. Bridge thread and HTTP search already clamped before casting. Change: add `buzz_core::filter::clamp_limit(limit, default, max) -> u32`, which saturates the request to u32 before applying the per-site max, and use it at every filter-limit site (REQ/COUNT query params, WS search, bridge window, feed, thread, HTTP search). The inline casts are deleted. `BRIDGE_FEED_MAX_LIMIT` becomes u32 to match. Why safe: each site keeps its default, max, and `limit: 0` behaviour (Some(0) still yields 0; the window still applies `.max(1)`); in-range limits produce the same values as before. COUNT drops the row limit before querying, so its result is unchanged. Tests: `clamp_limit_table` covers absent, zero, in-range, at-max, over-max, u32 boundary and usize::MAX. `huge_filter_limit_is_clamped_to_page_max` drives `filter_to_query_params` with `usize::MAX`; before the fix it yielded `Some(-1)` instead of `Some(1000)`. Signed-off-by: David Grochowski <dgrochowski@squareup.com>
ThePumpingLemma
force-pushed
the
dgrochowski/clamped-filter-limit
branch
from
November 7, 2026 04:55
8043281 to
88d76f7
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Four request paths converted a client-supplied
usizefilter limit with an inlineascast before clamping:(l as i64).min(max)(REQ query params, bridge feed) and(l as u32).min(max)(WS NIP-50 search, bridge channel window).i64::MAXturns negative (usize::MAX as i64 == -1). On REQ and the bridge/querygeneral path it reaches Postgres asLIMIT -1, which fails the query. On the bridge feed it tripsfeed_count >= limit, so the feed comes back empty.0throughas u32. That returns no rows (one row in the window, via.max(1)) instead of the page max.Bridge thread and HTTP search already clamped before casting. COUNT drops the row limit, so it was not affected.
Change
buzz_core::filter::clamp_limit(limit, default, max) -> u32. It returnsdefaultwhen the limit is absent; otherwise it saturates the request tou32and then caps it atmax.filter_to_query_paramsand WS search inhandlers/req.rs; channel window, feed, thread and HTTP search inapi/bridge.rs. The inline casts are deleted, andBRIDGE_FEED_MAX_LIMITis nowu32.limit: 0behaviour. In-range limits are unchanged.Testing
Failing before the fix (
cargo test -p buzz-relay --lib huge_filter_limit):After:
cargo test -p buzz-core --lib clamp_limit→ 1 passed (table test covering absent, 0, in-range, at/over max, the u32 boundary andusize::MAX)cargo test -p buzz-relay --lib -- huge_filter_limit req:: bridge→ 152 passed, 0 failed (135 Postgres tests ignored)cargo clippy -p buzz-core -p buzz-relay --all-targets -- -D warnings→ clean;cargo fmt --all→ cleanNot covered
No Postgres-backed tests were run locally. The bridge call sites rely on the shared helper's table test; they have no per-handler tests.
Review
Every server-side read of
Filter.limitnow goes throughclamp_limit. The per-site default, max andlimit: 0behaviour is unchanged. No inline casts are left. Scores: minimalism 9, elegance 9, correctness 9.Blockers: none.
Fixed in review:
PAGE_LIMIT_MAXhad been inserted betweenfilter_to_query_params's doc comment and the function, so the doc comment ended up on the const. Moved the const above it.Optional:
bridge.rsnow has two functions calledclamp_limit: the local moderationclamp_limit(Option<i64>)andbuzz_core::filter::clamp_limit. They don't collide, but they are easy to confuse.Exercised on a local relay
Commit tested:
88d76f7f1. Dev profile, isolated harness relay on :3030, authenticated channel member. Human rate limits were raised so seeding wasn't throttled. I seeded 1,100 kind-9 events in one channel. The filters were sent as raw JSON so the large integers arrived exactly.limit18446744073709551615limit4294967296limit50 (control)REQREQwithsearch/query/querywithsearch/querytop_level(channel window)/queryfeed_typesEvery huge limit came back as that path's maximum. None of them produced a database error, and none came back empty. The relay log has no error lines, and the access log records the raw
limitas 18446744073709551615 withoutcome:"served".Postgres-gated tests: the PR adds or modifies none. As a related check I ran
cargo test -p buzz-relay --lib -- --ignored limit. Run in parallel, it gave 10 passed and 1 failed:rate_limited_query_logs_one_denied_line. That test passes on its own, and the whole set gives 11 passed with--test-threads=1. It looks like interference between tests that share the access-log capture, not a problem with this change.Not exercised: