Skip to content

fix(fuzz): preserve form parameters with shared prefixes - #7515

Merged
dwisiswant0 merged 1 commit into
projectdiscovery:devfrom
james-yusuke:test
Jul 16, 2026
Merged

fix(fuzz): preserve form parameters with shared prefixes#7515
dwisiswant0 merged 1 commit into
projectdiscovery:devfrom
james-yusuke:test

Conversation

@james-yusuke

@james-yusuke james-yusuke commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

Fixes incorrect reconstruction of form/query parameters when different parameter names share a prefix.

Before this change, decoding and re-encoding a query like:

foo=a&foobar=b&foobar=c

could incorrectly treat foobar_1 as if it belonged to foo, producing:

foobar=c&foo=b&foo=a

This PR makes normalized duplicate-parameter handling match the exact base parameter name before merging values. It preserves the existing duplicate-parameter behavior for cases like foo=a&foo=b&foo=c, while avoiding accidental merges for prefix-sharing names such as foo and foobar.

Proof

Tested with:

go test ./pkg/fuzz/dataformat -run 'TestFormDecodeEncode' -count=1
go test ./pkg/fuzz/component -run 'TestQueryComponent' -count=1
go test ./pkg/fuzz/dataformat ./pkg/fuzz/component
go test -race ./pkg/fuzz/dataformat ./pkg/fuzz/component
go test ./pkg/fuzz/...

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of form and query parameters so names with shared prefixes stay separate.
    • Preserved duplicate parameters and their original order when decoding and re-encoding form data.
  • Tests

    • Added coverage for duplicate parameter handling.
    • Added coverage to ensure prefix-matching parameter names are not incorrectly merged.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The key-normalization logic in form.go's Encode function was changed from a strings.HasPrefix-based check to a new normalizedKeyBase helper that derives base keys via regex matching, preventing prefix-sharing parameter names from being incorrectly merged. Tests were added covering query and form components.

Changes

Prefix Key Merge Fix

Layer / File(s) Summary
Normalization key base helper and Encode fix
pkg/fuzz/dataformat/form.go
Replaces strings.HasPrefix-based normalized key detection with a new normalizedKeyBase helper using reNormalized.FindStringIndex to compute base keys, removing the strings import.
Tests for prefix parameter preservation
pkg/fuzz/component/query_test.go, pkg/fuzz/dataformat/form_test.go
Adds tests verifying query and form parameters with shared prefixes (foo/foobar) and duplicate keys remain distinct after parse/rebuild or decode/encode cycles.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit hopped through foo and foobar too,
No merging allowed, each name stays true.
With regex in paw, the base key found,
Prefix confusion no longer around.
Tests all pass — hop hop, hooray! 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main fix: preserving form parameters with shared prefixes during fuzz reconstruction.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@dwisiswant0 dwisiswant0 changed the title fix(fuzz): preserve form parameters with shared prefixes test'(fuzz): preserve form parameters with shared prefixes Jul 3, 2026
@dwisiswant0 dwisiswant0 changed the title test'(fuzz): preserve form parameters with shared prefixes test(fuzz): preserve form parameters with shared prefixes Jul 3, 2026
@dwisiswant0 dwisiswant0 changed the title test(fuzz): preserve form parameters with shared prefixes fix(fuzz): preserve form parameters with shared prefixes Jul 3, 2026

@dwisiswant0 dwisiswant0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dwisiswant0

Copy link
Copy Markdown
Member

@coderabbitai create an issue for this.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="projectdiscovery/nuclei"
pr_number="7515"

echo "=== CodeRabbit review comments ==="
gh api --paginate "repos/$repo/pulls/$pr_number/comments" --jq '
  .[] | select((.user.login | ascii_downcase) == "coderabbitai") |
  {url: .html_url, path: .path, line: .line, body: .body}
'

echo "=== CodeRabbit PR conversation comments ==="
gh api --paginate "repos/$repo/issues/$pr_number/comments" --jq '
  .[] | select((.user.login | ascii_downcase) == "coderabbitai") |
  {url: .html_url, body: .body}
'

Length of output: 750


@dwisiswant0 I couldn’t find an earlier CodeRabbit comment or an outstanding follow-up on #7515 to use as the issue scope. Please paste the concern or link the review comment you want tracked, and I’ll open and assign the GitHub issue to you.

@dwisiswant0

Copy link
Copy Markdown
Member

@coderabbitai just create a tracking issue for this.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="projectdiscovery/nuclei"
pr_url="https://github.com/projectdiscovery/nuclei/pull/7515"
comment_url=$(gh api "repos/$repo/issues/7515/comments" \
  --jq '.[] | select(.user.login == "dwisiswant0" and (.body | contains("just create a tracking issue for this"))) | .html_url' \
  | tail -n1)

if [ -z "$comment_url" ]; then
  comment_url="$pr_url"
fi

body=$(cat <<EOF
## Summary
Track post-merge validation and any follow-up work for the fuzzing parameter-normalization change in `#7515`.

## Rationale
Fuzzing request reconstruction must preserve the identity and order of parameters whose names share prefixes. For example, \`foo\` must remain distinct from \`foobar\`, while true duplicate keys such as repeated \`foo\` parameters must continue to merge according to the existing normalization behavior.

## Affected areas
- \`pkg/fuzz/dataformat/form.go\`
- \`pkg/fuzz/component/query_test.go\`
- Form and query parameter decode/encode/rebuild paths

## Acceptance criteria
- Confirm prefix-sharing parameter names are never merged during form or query reconstruction.
- Confirm true duplicate parameters retain the established normalization behavior and value order.
- Add or adjust regression coverage if additional request formats or edge cases are discovered.

## Backlinks
- Pull request: $pr_url
- Requesting comment: $comment_url
- Requested by: `@dwisiswant0`
EOF
)

gh issue create \
  --repo "$repo" \
  --title "Track fuzzing parameter normalization follow-up" \
  --body "$body" \
  --assignee "dwisiswant0"

Length of output: 1748


@dwisiswant0 Created and assigned the tracking issue: #7551.

@dwisiswant0 dwisiswant0 linked an issue Jul 16, 2026 that may be closed by this pull request
@dwisiswant0
dwisiswant0 merged commit c928dcc into projectdiscovery:dev Jul 16, 2026
19 checks passed
@james-yusuke
james-yusuke deleted the test branch July 16, 2026 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Track fuzzing parameter normalization follow-up

2 participants