fix(http): lost request body on retries & redirects

[dwisiswant0]

Proposed changes

fix(http): lost request body on retries & redirects

Updates the HTTP protocol to use
(*retryablehttp.Request).SetBodyString instead
of direct Body assignment.

This fixes #6665 where the request body was
dropped during retries or 307/308 redirects
because GetBody was not being populated.

Thanks to @zzyjsj for reporting the bug in the
upstream dependency and the hints!

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

• Chores
  • Updated HTTP client library to v1.1.0 for improved reliability and performance.
  • Upgraded compression support with latest optimizations.
  • Streamlined internal request body handling for enhanced efficiency.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Upgraded retryablehttp-go dependency to v1.1.0 and refactored HTTP request body handling to use SetBodyString method instead of manual reader creation, which properly implements GetBody for retry and redirect operations.

Changes

Cohort / File(s)	Change Summary
Dependency Management go.mod	Upgraded github.com/projectdiscovery/retryablehttp-go from v1.0.133 to v1.1.0 and indirect dependency github.com/klauspost/compress from v1.18.1 to v1.18.2.
HTTP Request Building pkg/protocols/http/build_request.go	Replaced readerutil.NewReusableReadCloser body setup with direct req.SetBodyString(body) call; removed readerutil import and bodyReader variable; updated error handling accordingly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10–15 minutes

• Straightforward dependency upgrade with clear semantic versioning
• Refactoring follows a simple pattern: replacement of one method call with another
• Limited file scope (2 files)
• Areas to verify: Confirm that SetBodyString properly implements the GetBody field internally to support HTTP retries and 307/308 redirects as intended by the linked issue

Poem

🐰 A hop, a fix, a retry's delight,
SetBodyString ensures the flow is right,
No more empty bodies in the night,
Redirects follow, every redirect's flight,
Our requests now bounce back just right! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main fix: addressing lost request bodies during retries and redirects in HTTP protocol handling.
Linked Issues check	✅ Passed	The pull request directly addresses all objectives from issue #6665 by using SetBodyString to populate GetBody, ensuring request bodies are preserved during retries and 307/308 redirects.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the HTTP request body preservation issue: dependency upgrade for the fix, removal of problematic readerutil usage, and SetBodyString implementation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dwisiswant0/fix/http/lost-request-body-on-retries-redirects

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[dwisiswant0]

Note that this PR also bump github.com/klauspost/compress dep.

$ go get -v github.com/projectdiscovery/[email protected]
go: warning: github.com/klauspost/[email protected]: retracted by module author: https://github.com/klauspost/compress/issues/1114
go: to switch to the latest unretracted version, run:
	go get github.com/klauspost/compress@latest
go: upgraded github.com/projectdiscovery/retryablehttp-go v1.0.133 => v1.1.0

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

pkg/protocols/http/build_request.go (1)

458-471: Using SetBodyString correctly wires request bodies for retries/redirects

Switching from manually setting req.Body to req.SetBodyString(body) after evaluation is the right fix to ensure the underlying http.Request can replay the body on retries and 307/308 redirects. The surrounding logic (Interactsh replacement, expression evaluation, headers) stays intact, and the new error wrap is fine, though you may eventually want to standardize on errkit for consistency.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c46f2c and ce0771d.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (2)

• go.mod (2 hunks)
• pkg/protocols/http/build_request.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Format Go code using go fmt ./...
Run static analysis using go vet ./... on Go code

Files:

• pkg/protocols/http/build_request.go

pkg/protocols/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

pkg/protocols/**/*.go: Each protocol implementation should implement the Request interface with Compile(), ExecuteWithResults(), Match(), and Extract() methods
Protocol implementations should embed Operators for matching/extraction functionality

Files:

• pkg/protocols/http/build_request.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Lint

🔇 Additional comments (1)

go.mod (1)

30-30: Dependency bumps look appropriate for new HTTP body handling

Upgrading github.com/projectdiscovery/retryablehttp-go to v1.1.0 and github.com/klauspost/compress to v1.18.2 is consistent with the new SetBodyString usage and should be low‑risk, especially with tests and lint already passing. No issues from the diff itself; just keep an eye out for any subtle retry/redirect or compression behavior changes in downstream runs.

Also applies to: 273-273

[dwisiswant0]

This wraps the body in a sync primitive to control the timing of the read for race condition testing. Using SetBody* here would obscure or break this timing mechanism, so I will leave it as is.

[dwisiswant0]

Any other occurrences of direct (*retryablehttp.Request).Body assignment were left on purpose. None of them affect the reliability of retries/redirects in the main logic anyway.

[dwisiswant0]

Any other occurrences of direct (*retryablehttp.Request).Body assignment were left on purpose. None of them affect the reliability of retries/redirects in the main logic anyway.

Okay, so I was wrong about that. I'm still finding a couple more occurrences that might be impacted too. @coderabbitai create an issue for that.

[coderabbitai]

Tip

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

🧩 Analysis chain

@dwisiswant0: I'll search for additional occurrences of direct (*retryablehttp.Request).Body assignments and create an issue for you.

---

🏁 Script executed:

#!/bin/bash
# Search for direct Body assignments on retryablehttp.Request
# Look for patterns like: req.Body = ...

# Find direct Body field assignments
echo "=== Direct .Body assignments ==="
rg -nP --type=go 'req\.Body\s*=' -C3

echo -e "\n=== SetBody method calls (for comparison) ==="
rg -nP --type=go '\.SetBody(String)?\(' -C2

echo -e "\n=== Files with retryablehttp imports ==="
rg -l --type=go 'retryablehttp'

Length of output: 6168

---

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for the additional occurrences

gh issue create \
  --title "Fix additional direct Body assignments affecting retries/redirects" \
  --body "## Description

Following up on PR #6666, additional occurrences of direct \`(*retryablehttp.Request).Body\` assignments have been identified that may impact the reliability of retries and 307/308 redirects.

## Background

Direct assignment to \`.Body\` leaves the \`GetBody\` field nil, which prevents the http client from rewinding and resending request bodies during retries or 307/308 redirects. The fix is to use \`SetBodyString()\` or \`SetBody()\` methods instead.

## Affected Locations

Based on codebase analysis, the following files contain direct Body assignments that may need review:

1. **pkg/reporting/exporters/splunk/splunkhec.go:118**
   \`\`\`go
   req.Body = io.NopCloser(bytes.NewReader(b))
   \`\`\`

2. **pkg/reporting/exporters/es/elasticsearch.go:134**
   \`\`\`go
   req.Body = io.NopCloser(bytes.NewReader(b))
   \`\`\`

3. **pkg/fuzz/component/body.go:50**
   \`\`\`go
   req.Body = io.NopCloser(bytes.NewReader(data))
   \`\`\`

4. **pkg/protocols/http/build_request.go:403** (race condition wrapper)
   \`\`\`go
   req.Body = race.NewOpenGateWithTimeout(req.Body, time.Duration(2)*time.Second)
   \`\`\`

5. **pkg/protocols/http/request.go:919** (curl command generation)
   \`\`\`go
   req.Body = io.NopCloser(bytes.NewReader(bodyBytes))
   \`\`\`

## Recommendation

Review each occurrence to determine:
- Whether the request can be retried or redirected
- If so, migrate from direct \`.Body\` assignment to \`.SetBodyString()\` or appropriate method
- Add test coverage to validate body preservation during retries/redirects

## References

- PR #6666: https://github.com/projectdiscovery/nuclei/pull/6666
- Original issue: #6665
- Reported by: @dwisiswant0" \
  --assignee dwisiswant0 \
  --repo projectdiscovery/nuclei

Length of output: 54

---

@dwisiswant0: I've created issue #6692 to track the additional occurrences of direct Body assignments.

I found 5 locations that may be impacted:

• pkg/reporting/exporters/splunk/splunkhec.go:118
• pkg/reporting/exporters/es/elasticsearch.go:134
• pkg/fuzz/component/body.go:50
• pkg/protocols/http/build_request.go:403 (race condition wrapper)
• pkg/protocols/http/request.go:919 (curl command generation)

The issue includes detailed context and recommendations for reviewing each occurrence. Some may be less critical (like the curl generation), but they should all be evaluated to ensure retry/redirect reliability.