#35964 fuzz: add coverage for GetQueryParameterFromUri

ReviewabilityReviewsAgreementSize
#35964 laxmanacharya8 fuzz: add coverage for GetQueryParameterFromUriReady0 (+1)Positive0 + 25 tests
  • Add fuzz coverage for parsing remote unauthenticated REST query parameters
  • Detects parsing anomalies and crashes on malformed URI inputs
  • Ready for review now
  • Concept approval without stated reasons (nervana21)
  • Verified by testing the updated fuzz target (nervana21)
  • Resolved objection: harness did not prove keys after fragment delimiters are ignored (nervana21)
  • 0 lines added or modified outside tests
  • 25 lines added or modified in tests
  • 0 lines removed in total
  • 1 files, 1 commits
Test infrastructure#22 of 54
P3 · test coverage
  • P3 because it adds fuzz coverage for parsing unauthenticated remote REST query strings
  • It improves parser robustness but does not touch consensus, mempool, or P2P logic
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Adds a new fuzz target, `http_query_parameter`, to exercise `GetQueryParameterFromUri()`. The target tests arbitrary inputs as well as round-trip URL-encoded key-value pairs, parameter keys without an equals sign, duplicate keys, and query keys appearing after URI fragment delimiters (`#`).

Problem: `GetQueryParameterFromUri()` parses remote unauthenticated query strings for REST endpoints (such as `count`, `offset`, and `verbose`), but lacked dedicated fuzz coverage to detect parsing anomalies or crashes on malformed inputs.

Evidence of importance:

  • Commit message: 'GetQueryParameterFromUri() parses the query string of a request target supplied by a remote client. It is reached from the REST interface for the "count", "offset", "size", "verbose" and "mempool_sequence" parameters, but had no fuzz coverage.'

Risk of a mistake: low (tests). Touches only fuzz test harness code, with no effect on node runtime or consensus logic.

Changed since the previous assessment: Diff rebased or updated without changing the stated purpose or target mechanics.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • author: author to confirm whether PR is still being worked on or can be closed 2026-10-02
  • author: author response on whether work is continuing or if the PR can be closed 2026-10-02

Agreement

Positive

support without stated reasons, no open objection (nervana21, nervana21)

Thread notes: Re-evaluated under current schema rules. nervana21's objection rc:3786326174 updated to clears_with change and accepted status following Tested ACK on the updated commit. Added waiting_on entry for maflcko's query regarding PR status. / Updated rc:3786326174 clears_with to change as it was a defect in the added test. maflcko's comments on squashing and inquiring about status are process-related, leaving the PR waiting on the author.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
nervana21 (contributor)objection, correctnessthe test does not verify that keys after '#' are ignored because an identical key precedes it and first match winsfixed · fix pushed · fixed in follow-up push verified by reviewer with a tACKchangedno2026-08-14: 'because an identical key name sits before `#`, the first match wins and treating `#` like `&` still passes. In order to prove that statement, I think we need a lookup whose `key` exists only after `#`'
Status from: 2026-08-15: nervana21: 'tACK a7ecf9c83c60622b6af9b3c992099170dd15033a'

Support:

  • nervana21 (contributor) (Concept ACK, 2026-08-14): Concept ACK [not substantive]
  • nervana21 (contributor) (Tested ACK, 2026-08-15) [tested]: tACK a7ecf9c83c60622b6af9b3c992099170dd15033a [not substantive]

Participants:

  • nervana21 (contributor, objection, 4 statements 2026-08-14 to 2026-08-15): noted test did not prove keys after '#' are ignored, which was accepted after an update; left Concept ACK and tACK
  • maflcko (member, neutral, 2 statements 2026-09-28 to 2026-10-02): requested commit squash and asked if PR is still active or can be closed

Files

25 lines under test/bench/ci.

  • src/test/fuzz/http_request.cpp +25/-0

Card

The short text the ranking pass compares across PRs.

Adds the http_query_parameter fuzz target to test GetQueryParameterFromUri(). The parser processes unauthenticated URI query strings from remote clients for REST endpoints like count, offset, and verbose, but previously lacked fuzz coverage. The target checks arbitrary inputs as well as edge cases such as duplicate keys, missing values, and fragment separators. No dependencies.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-05T01:16, confidence high, input hash 9cad82cae9d58193
thread read through 2026-10-02 (6 statements), head cdd6222c4f
labels: Fuzzing

Categories

Test infrastructure

P3 · test coverage

Adds fuzz coverage for `GetQueryParameterFromUri()`, which parses unauthenticated query strings supplied by remote clients to REST endpoints. While remote parser fuzzing is valuable, it does not target consensus, mempool, or P2P logic.

Membership: Adds a new fuzz target in `src/test/fuzz/http_request.cpp` and carries the Fuzzing label.

Factors: security/stability 1, bug 0, performance 0, user value 0, leverage 0