fuzz: add coverage for GetQueryParameterFromUri #35964

laxmanacharya8 commented at 4:46 PM on August 13, 2026: contributor

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.

This adds an http_query_parameter target. Besides running the parser on arbitrary input, it round-trips a URL-encoded key/value pair back through it and checks the cases that are easy to get wrong:

a parameter with no =, which is an empty value rather than a missing one
a repeated key, where the first occurrence wins
a key that occurs both before and after a #, where the parser must return the value found before the # and ignore the later occurrence, since everything from the fragment separator onwards is not part of the query string

Testing

Built the fuzz binary and ran the new target against a temporary corpus directory, capped at 10,000 executions:

FUZZ=http_query_parameter build_fuzz/bin/fuzz /tmp/corpus_http_query_parameter -runs=10000

DrahtBot added the label Fuzzing on Aug 13, 2026

DrahtBot commented at 4:47 PM on August 13, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/35964
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/35964.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/35964#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Stale ACK	nervana21

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

in src/test/fuzz/http_request.cpp:72 in ec1f8dca31 outdated

  66 | @@ -66,3 +67,23 @@ FUZZ_TARGET(http_request)
  67 |          assert(body.empty());
  68 |      }
  69 |  }
  70 | +
  71 | +FUZZ_TARGET(http_query_parameter)
  72 | +{

nervana21 commented at 6:26 PM on August 14, 2026:

ec1f8dca31238b7a5c97afc88332b27f026ab588: fuzz: add coverage for GetQueryParameterFromUri

nit: Prefer matching the style of the above and using the shortened path names.

{
   using http_bitcoin::GetQueryParameterFromUri;

in src/test/fuzz/http_request.cpp:88 in ec1f8dca31

  83 | +    const std::string encoded_value{UrlEncode(value)};
  84 | +    const std::string query_uri{"/endpoint?" + encoded_key + "=" + encoded_value};
  85 | +    assert(http_bitcoin::GetQueryParameterFromUri(query_uri, key) == value);
  86 | +    assert(http_bitcoin::GetQueryParameterFromUri("/endpoint?" + encoded_key, key) == "");
  87 | +    assert(http_bitcoin::GetQueryParameterFromUri(query_uri + "&" + encoded_key + "=ignored", key) == value);
  88 | +    assert(http_bitcoin::GetQueryParameterFromUri(query_uri + "#?" + encoded_key + "=ignored", key) == value);

nervana21 commented at 6:40 PM on August 14, 2026:

ec1f8dca31238b7a5c97afc88332b27f026ab588: fuzz: add coverage for GetQueryParameterFromUri

The PR text states that this proves we "ignore the later occurrence" after #. However, 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 #.

The ? character must precede # and the dummy name cannot match key name.

    assert(http_bitcoin::GetQueryParameterFromUri(query_uri + "#?" + encoded_key + "=ignored", key) == value);
    // First '?' must precede '#', and this name must not be key.
    const std::string dummy{key == "n" ? "m" : "n"};
    assert(!GetQueryParameterFromUri("/endpoint?" + dummy + "=1#?" + encoded_key + "=" + encoded_value, key));

nervana21 commented at 6:42 PM on August 14, 2026: contributor

Concept ACK

Left a few minor suggestions. Please let me know what you think

laxmanacharya8 force-pushed on Aug 15, 2026

laxmanacharya8 requested review from nervana21 on Aug 15, 2026

nervana21 commented at 10:46 PM on August 15, 2026: contributor

tACK a7ecf9c83c60622b6af9b3c992099170dd15033a

DrahtBot added the label CI failed on Aug 17, 2026

DrahtBot removed the label CI failed on Aug 17, 2026

laxmanacharya8 requested review from nervana21 on Aug 22, 2026

DrahtBot added the label Needs rebase on Sep 25, 2026

DrahtBot removed the label Needs rebase on Sep 26, 2026

DrahtBot added the label CI failed on Sep 26, 2026

DrahtBot commented at 4:23 PM on September 26, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task ASan + LSan + UBSan + integer: https://github.com/bitcoin/bitcoin/actions/runs/36252212318/job/108432231941</sub> <sub>LLM reason (✨ experimental): CI failed to compile the fuzz target due to an undeclared identifier http_bitcoin in src/test/fuzz/http_request.cpp.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

maflcko commented at 6:41 AM on September 28, 2026: member

No opinion on the changes here, but please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

git fetch upstream && git merge upstream/master  # Make sure we are on current master
git reset --soft upstream/master && git commit -m 'bla: Fix ...'  # Squash everything into a single commit
git push origin master -f  # Force push

maflcko commented at 9:23 AM on October 2, 2026: member

are you still working on this or can it be closed?

DrahtBot marked this as a draft on Oct 2, 2026

fuzz: add coverage for GetQueryParameterFromUri

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.

cdd6222c4f

laxmanacharya8 force-pushed on Oct 3, 2026

laxmanacharya8 marked this as ready for review on Oct 3, 2026

DrahtBot removed the label CI failed on Oct 3, 2026