Repository navigation
fix(httputilz): keep the last raw request header without a trailing newline - #2658
theluckystrike wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
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
ChangesRaw request header parsing
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Raw request files whose last header has no trailing newline now keep that header. No actionable merge risk remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to 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 Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
A rabbit reads the final line Comment |
|
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, 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, |
Proposed changes
Closes #2657
ParseRequestreads the raw request header block line by line and stopped on any read error,io.EOFincluded. When the-rrfile ends right after the last header, that line comes back together withio.EOFand 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
TestParseRequestKeepsLastHeaderWithoutTrailingNewlinecovers 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 ./...andgolangci-lint run ./...(v2.14.0) report no issues.go build ./cmd/httpxworks.go test ./...onlyTestDo,TestRunner_probeall_targetsandTestRunner_probeall_targets_with_portfail on my machine. They need outside network, which was blocked for the run, and they fail the same way on dev.X-Api-Key: secretis present. With a request file that has no final newline, dev prints[401]and this branch prints[200].Checklist
Summary by CodeRabbit