Deduplicate multiformat fuzz HTTP inputs - #7580
Conversation
WalkthroughChangesHTTP request deduplication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant JSONL as JSONL input
participant Provider as NewHttpInputProvider
participant Deduper as RequestDeduplicator
participant Iterate as HttpInputProvider.Iterate
JSONL->>Provider: parse HTTP requests
Provider->>Deduper: IsDuplicate(request)
Deduper-->>Provider: duplicate status
Provider-->>Iterate: unique requests and dupeCount
Iterate->>Deduper: IsDuplicate(request)
Deduper-->>Iterate: duplicate status
Iterate-->>Provider: unique inputs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/input/dedupe/dedupe.go (1)
125-142: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueHost casing and URL fragment are not normalized.
NormalizeURLpreserves the original host casing and fragment. Per RFC 3986 hosts are case-insensitive, and fragments are never sent to the server, so two otherwise-identical requests differing only in host case or fragment will not be deduplicated (a missed-dedup false negative, not a false positive). Low risk since it only reduces dedup effectiveness rather than dropping legitimate requests; consider lowercasing scheme/host and stripping the fragment for more complete normalization.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/input/dedupe/dedupe.go` around lines 125 - 142, Update NormalizeURL to lowercase the URL scheme and host, and clear the fragment before returning the normalized string. Preserve the existing query sorting, root-path defaulting, nil handling, and cloned URL behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/input/dedupe/dedupe.go`:
- Around line 97-122: Update the fingerprint construction in the request hashing
function to delimit every serialized field, including method, normalizedURL,
each sorted header key and value, and body, before computing the SHA-256 hash.
Use an unambiguous separator or length-prefixed encoding consistently so
distinct field boundaries cannot produce the same hash input, while preserving
the existing header ordering and duplicate-detection flow.
---
Nitpick comments:
In `@pkg/input/dedupe/dedupe.go`:
- Around line 125-142: Update NormalizeURL to lowercase the URL scheme and host,
and clear the fragment before returning the normalized string. Preserve the
existing query sorting, root-path defaulting, nil handling, and cloned URL
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b061612-8e1b-46a7-84de-76f59d1b7a39
📒 Files selected for processing (5)
internal/server/dedupe.gopkg/input/dedupe/dedupe.gopkg/input/dedupe/dedupe_test.gopkg/input/provider/http/multiformat.gopkg/input/provider/http/multiformat_test.go
| var hashContent strings.Builder | ||
| method := "GET" | ||
| var body string | ||
| var headers mapsutil.OrderedMap[string, string] | ||
| if req.Request != nil { | ||
| if req.Request.Method != "" { | ||
| method = strings.ToUpper(req.Request.Method) | ||
| } | ||
| body = req.Request.Body | ||
| headers = req.Request.Headers | ||
| } | ||
| hashContent.WriteString(method) | ||
| hashContent.WriteString(normalizedURL) | ||
|
|
||
| for _, header := range sortedNonDynamicHeaders(headers) { | ||
| hashContent.WriteString(header.Key) | ||
| hashContent.WriteString(header.Value) | ||
| } | ||
|
|
||
| if len(body) > 0 { | ||
| hashContent.WriteString(body) | ||
| } | ||
|
|
||
| hash := sha256.Sum256([]byte(hashContent.String())) | ||
| return hex.EncodeToString(hash[:]), nil | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fingerprint fields are concatenated without delimiters — collision risk.
method, normalizedURL, each header key/value, and body are written back-to-back into hashContent with no separators (Lines 108-118). Different (header-key, header-value) pairs can serialize to the identical string (e.g. key="ab", value="c" vs key="a", value="bc"), producing the same SHA-256 fingerprint for genuinely different requests. Since IsDuplicate treats identical fingerprints as duplicates, this can silently drop distinct requests from fuzzing/DAST coverage — the exact failure mode this feature is meant to avoid.
🐛 Proposed fix: add delimiters between fields
hashContent.WriteString(method)
+ hashContent.WriteString("\x00")
hashContent.WriteString(normalizedURL)
for _, header := range sortedNonDynamicHeaders(headers) {
+ hashContent.WriteString("\x00")
hashContent.WriteString(header.Key)
+ hashContent.WriteString("\x00")
hashContent.WriteString(header.Value)
}
if len(body) > 0 {
+ hashContent.WriteString("\x00")
hashContent.WriteString(body)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var hashContent strings.Builder | |
| method := "GET" | |
| var body string | |
| var headers mapsutil.OrderedMap[string, string] | |
| if req.Request != nil { | |
| if req.Request.Method != "" { | |
| method = strings.ToUpper(req.Request.Method) | |
| } | |
| body = req.Request.Body | |
| headers = req.Request.Headers | |
| } | |
| hashContent.WriteString(method) | |
| hashContent.WriteString(normalizedURL) | |
| for _, header := range sortedNonDynamicHeaders(headers) { | |
| hashContent.WriteString(header.Key) | |
| hashContent.WriteString(header.Value) | |
| } | |
| if len(body) > 0 { | |
| hashContent.WriteString(body) | |
| } | |
| hash := sha256.Sum256([]byte(hashContent.String())) | |
| return hex.EncodeToString(hash[:]), nil | |
| } | |
| var hashContent strings.Builder | |
| method := "GET" | |
| var body string | |
| var headers mapsutil.OrderedMap[string, string] | |
| if req.Request != nil { | |
| if req.Request.Method != "" { | |
| method = strings.ToUpper(req.Request.Method) | |
| } | |
| body = req.Request.Body | |
| headers = req.Request.Headers | |
| } | |
| hashContent.WriteString(method) | |
| hashContent.WriteString("\x00") | |
| hashContent.WriteString(normalizedURL) | |
| for _, header := range sortedNonDynamicHeaders(headers) { | |
| hashContent.WriteString("\x00") | |
| hashContent.WriteString(header.Key) | |
| hashContent.WriteString("\x00") | |
| hashContent.WriteString(header.Value) | |
| } | |
| if len(body) > 0 { | |
| hashContent.WriteString("\x00") | |
| hashContent.WriteString(body) | |
| } | |
| hash := sha256.Sum256([]byte(hashContent.String())) | |
| return hex.EncodeToString(hash[:]), nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/input/dedupe/dedupe.go` around lines 97 - 122, Update the fingerprint
construction in the request hashing function to delimit every serialized field,
including method, normalizedURL, each sorted header key and value, and body,
before computing the SHA-256 hash. Use an unambiguous separator or
length-prefixed encoding consistently so distinct field boundaries cannot
produce the same hash input, while preserving the existing header ordering and
duplicate-detection flow.
1 participant
Summary
pkg/input/dedupe) and use it in multiformat HTTP inputs (burp, openapi, swagger, jsonl, yaml) so equivalent requests are skipped during count/iterateCloses #4962
Summary by CodeRabbit
New Features
Bug Fixes