Skip to content

fix(httputilz): keep the last raw request header without a trailing newline - #2658

Closed
theluckystrike wants to merge 1 commit into
projectdiscovery:devfrom
theluckystrike:fix/httpx-payer-proof
Closed

theluckystrike wants to merge 1 commit into
projectdiscovery:devfrom
theluckystrike:fix/httpx-payer-proof

Conversation

@theluckystrike

@theluckystrike theluckystrike commented Oct 6, 2026 •

Copy link
Copy Markdown

Proposed changes

Closes #2657

ParseRequest reads the raw request header block line by line and stopped on any read error, io.EOF included. When the -rr file ends right after the last header, that line comes back together with io.EOF and was dropped, so the header wasn't sent.

The loop still stops on the first empty line. On a read error it now keeps the line it got and then stops. Files that end with a blank line or carry a body are parsed the same as before.

Proof

  • New test TestParseRequestKeepsLastHeaderWithoutTrailingNewline covers CRLF and LF files in safe and unsafe mode. It fails on dev and passes with this change.
  • go test ./common/httputilz/ passes. go vet ./... and golangci-lint run ./... (v2.14.0) report no issues. go build ./cmd/httpx works.
  • In go test ./... only TestDo, TestRunner_probeall_targets and TestRunner_probeall_targets_with_port fail on my machine. They need outside network, which was blocked for the run, and they fail the same way on dev.
  • Manual check against a local server that returns 200 only when X-Api-Key: secret is present. With a request file that has no final newline, dev prints [401] and this branch prints [200].

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
    • Requests whose final header line has no trailing newline are now parsed correctly, preserving the header and allowing the request to be processed.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 736a1bf3-2c2c-43a8-af62-f7796a023873
📥 Commits

Reviewing files that changed from the base of the PR and between d3b9d3d and 861e09b.

📒 Files selected for processing (2)
  • common/httputilz/httputilz.go
  • common/httputilz/httputilz_test.go

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

ParseRequest now retains a final header line without a trailing newline. Regression tests cover CRLF and LF requests in safe and unsafe modes.

Changes

Raw request header parsing

Layer / File(s) Summary
Parse final header and verify request
common/httputilz/httputilz.go, common/httputilz/httputilz_test.go
ParseRequest processes a nonempty header line before stopping on a read error. Tests check CRLF and LF requests in safe and unsafe modes, including header presence and an empty body.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 861e0

Raw request files whose last header has no trailing newline now keep that header. No actionable merge risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 861e0

The fix restores explicitly supplied headers while preserving existing request validation and authorization ordering. No introduced security flaw was identified, but credential forwarding through every execution path is not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A newly retained credential header can reach each configured scan target through the existing custom-header flow. Control of the configured request content is required. This propagation already occurred for the same header with a terminating newline; no additional privilege or independent tenant scope was established.

Trust Boundaries and Controls

  • observed — The primary safe client disables redirects unless configured otherwise and offers a same-host redirect restriction. The locally available standard-library header copier treats Authorization and several cookie/proxy headers as sensitive, but not arbitrary credentials such as X-Api-Key. These mechanisms predate the PR; they do not establish equivalent enforcement in the alternate or unsafe execution paths.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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 describes the main change: retaining the final raw request header when it has no trailing newline.
Linked Issues check ✅ Passed Issue #2657 requires -rr to retain the final header when the raw request file has no trailing newline, including LF input and unsafe mode. ParseRequest now processes the returned line before stopp…
Out of Scope Changes check ✅ Passed The changes are limited to ParseRequest and its regression test. Both directly support the fix requested by issue #2657. No unrelated changes appear in the reviewed summary or inspected files.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit reads the final line
No newline hides the key this time
Through CRLF and LF it goes
Safe and unsafe parsing knows
The empty body rests in place
The header keeps its proper place

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

@Mzack9999

Copy link
Copy Markdown
Member

Thanks for the fix. The same header handling landed in #2661, which also covers a request line that ends at EOF, the runner log, and a runner test. Closing this in favor of that.

@Mzack9999 Mzack9999 closed this Oct 10, 2026
@theluckystrike

Copy link
Copy Markdown
Author

@Mzack9999, that's fair. #2661 covers more than mine did, with the request line at EOF and a runner test too. Thanks for closing it with a note.

Thank you,
Michael Lip
Founder of https://zovo.one, 108 Chrome extensions with 150,000+ users
Also building https://makeroll.com and https://ml0x.com, open-source guardrails for Claude Code agents
Open source has to win, and small fixes like this one are my part of it

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.

-rr drops the last header when the raw request file has no trailing newline

2 participants