Opened 3 days ago
Last modified 2 days ago
#66484 new defect (bug)
WP_Http_Streams writes response headers into streamed files when headers span more than one read
| Reported by: | dpknauss | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | HTTP API | Version: | trunk |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: |
Description (last modified by )
Problem
In streamed mode, WP_Http_Streams::request() writes every block it reads to the destination file, including blocks read while it is still accumulating the response headers. Only the block that completes the \r\n\r\n header/body separator is replaced with the parsed body. Every header byte read in earlier blocks stays in the file.
This happens whenever the headers arrive over more than one fread():
- Headers larger than the 4,096-byte read size, wherever the separator falls.
- A separator that straddles the 4,096-byte boundary.
- Small headers delivered in more than one socket read.
fread()on a socket returns whatever is available, so a slow server, a proxy, or TLS record boundaries can split the headers.
The response is still parsed correctly and returns status 200, so the corrupted file is not reported as an error.
WP_Http_Streams has been deprecated since 6.4, and WP_Http no longer uses it, so this only affects code that calls the transport directly with 'stream' => true. It is present on trunk and was found while testing PR 14288 for #65817. That PR does not introduce or change it.
Reproduction
Serve each raw response below from a local socket and request it directly through WP_Http_Streams with:
array(
'stream' => true,
'filename' => $filename,
'timeout' => 5,
)
Results on trunk (a8f43c520d), PHP 8.1. Each response has the body hello:
| Raw response | Status | File size | File contents |
|---|---|---|---|
| Small headers, one write (control) | 200 | 5 | hello
|
| Separator at byte 4,094 (straddles the read boundary) | 200 | 4,101 | 4,096 header bytes + hello
|
| Separator at byte 6,000 | 200 | 4,101 | 4,096 header bytes + hello
|
| Separator at byte 10,000 | 200 | 8,197 | 8,192 header bytes + hello
|
HTTP/1.0 200 OK\r\nContent-Type: text/plain, a 200 ms pause, then \r\n\r\nhello | 200 | 46 | 41 header bytes + hello
|
The PR 14288 head (da0980d) gives identical results.
The straddling case makes a deterministic PHPUnit regression test, because no single read can contain a separator that ends at byte 4,098:
$prefix = "HTTP/1.0 200 OK\r\nX-Fill: "; $raw_response = $prefix . str_repeat( 'a', 4094 - strlen( $prefix ) ) . "\r\n\r\nhello";
Expected result
The destination file contains only the response body (hello).
Suggested fix
Write nothing to the file until the headers have been parsed, then write only the body part of the block that completes them. Regression tests should cover the straddling case and headers larger than 4 KB.
AI assistance: Yes
Tool(s): OpenAI Codex, Claude Code
Model(s): GPT-6.1, Claude Opus 5.5
Used for: Reviewing the transport, building and running the reproductions, and drafting this description; the reproductions and results were reviewed by me.
Change History (2)
#1
@
2 days ago
- Description modified (diff)
- Summary WP_Http_Streams writes partial headers to streamed files when the separator crosses a read boundary → WP_Http_Streams writes response headers into streamed files when headers span more than one read
This ticket was mentioned in PR #15487 on WordPress/wordpress-develop by @sarequl.
2 days ago
#2
- Keywords has-patch has-unit-tests added
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Fixes response headers being written into streamed files when the headers span more than one read.
See https://core.trac.wordpress.org/ticket/66484
What
WP_Http_Streams::request()in streamed mode writes eachfread()block to the destination file, even while the response headers are still accumulating. Only the block that completes the\r\n\r\nheader/body separator is replaced with the parsed body, so when the separator straddles the 4,096-byte read boundary, header bytes leak into the downloaded file. The response still parses as200, so the corruption is silent.Why
The headers can arrive over more than one
fread(): headers larger than the 4,096-byte read size, a separator straddling the boundary, or small headers split across socket reads. In every case the earlier header bytes stay in the file.How
Before the headers are complete, the block is skipped instead of written. Once
\r\n\r\nis present,WP_Http::processResponse()runs and only the body portion of the completing block is written. The non-streaming branch is untouched;WP_Http_Streamsis deprecated since 6.4 andWP_Httpno longer uses it, so the blast radius is direct callers with'stream' => true.Testing Instructions
vendor/bin/phpunit --group external-http tests/phpunit/tests/http/streams.php→OK (29 tests, 66 assertions). The class is annotated@group external-http, whichphpunit.xml.distexcludes by default; without--group external-httpPHPUnit reports "No tests executed!" (exit 0) — a false green.test_file_stream_headers_span_multiple_readsandtest_file_stream_header_separator_straddles_read_boundary) fail on the base commit with the full header block plushelloin the destination file, and pass with this change.vendor/bin/phpunit --group external-http tests/phpunit/tests/http/curl.php→OK (27 tests, 60 assertions).fill = 3925, the header block is 4,098 bytes and the separator straddles the 4,096-byte read boundary. The base commit writes a 4,101-byte file (headers +hello); this branch writes a 5-byte file (hello).composer lintreports only pre-existing violations;composer phpstanhas a pre-existing config error (DisallowedBacktickRulenot found);npm run buildpasses.