http: unfold response headers earlier #19949
Conversation
|
augment review |
There was a problem hiding this comment.
Pull request overview
This PR refactors HTTP header folding handling by moving the unfolding logic from the high-level header storage layer (headers.c) to the low-level HTTP parsing layer (http.c). This simplification allows the rest of the codebase to ignore header folding entirely, as folded headers are now expanded during initial parsing. The implementation intentionally preserves all whitespace from continuation lines rather than reducing it to a single space, which simplifies the state machine logic.
Key changes:
- Introduces early header unfolding in the HTTP parser with a new
unfold_header()function - Removes complex
unfold_value()logic from headers.c in favor of simple leading whitespace stripping - Adds
maybe_foldedstate bit to handle headers that end at buffer boundaries
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/http.c | Adds unfold_header() function and folding detection logic in http_parse_headers() to handle header continuation lines at parse time |
| lib/headers.c | Removes unfold_value() function and simplifies Curl_headers_push() to only strip leading blanks from already-unfolded headers |
| lib/urldata.h | Adds maybe_folded BIT flag to track when a header line ends at a buffer boundary and might be folded |
| tests/data/test798 | New test case for folded Set-Cookie headers to verify proper parsing of continuation lines |
| tests/data/test1940 | Updates expected output to reflect whitespace preservation (multiple spaces instead of single space) |
| tests/data/test1274 | Updates expected output to show unfolded headers on single lines with preserved whitespace |
| tests/data/Makefile.am | Adds test798 to the test suite list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
Analysis of PR #19949 at c38dc6a9: Test ../../tests/http/test_03_goaway.py::TestGoAway::test_03_02_h3_goaway failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Generated by Testclutch |
Make the low-level HTTP header "builder" unfold headers so that everything else can keep pretending folding does not exist. This code no longer tries to reduce repeated leading whitespace (in the continued folded header) to a single one. To avoid having to have a special state for that. Adjusted two test cases accordingly
test 798 - incoming cookie header in a folded line test 1665 - verify HTTP headers without final CRLF. Make sure all complete headers are delivered even if the reponse is partial
bagder
force-pushed
the
bagder/unfold-headers
branch
from
47a9db7 to
af1d745
Compare
2 participants
Make the low-level HTTP header "builder" unfold headers so that everything else can keep pretending folding does not exist.
This code no longer tries to reduce repeated leading whitespace (in the continued folded header) to a single one. To avoid having to have a special state for that.
Adjusted two test cases accordingly