Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 task
…unk test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
adamziel
pushed a commit
that referenced
this pull request
Sep 28, 2026
) ## What `ZipEncoder` wrote the wrong CRC-32 for deflated entries. It computed the checksum over the **compressed** bytes, while the ZIP format (and this component's own `FileEntry` docblock) requires the CRC-32 of the **uncompressed** data. Stored (uncompressed) entries were unaffected. ## Why now The bug has been there for a while, but readers didn't notice: `ZipDecoder` doesn't verify CRCs, and `ZipArchive` on older PHP versions didn't either. The PHP 8.4.26 and 8.5.11 CI runners now reject these entries, which breaks CI on every PR: ``` ZipEncoderTest::testWriteFile with data set #0 (true) ZipArchive::getFromName(): Cannot read entry: CRC error ``` `InstallPluginStepTest` fails the same way ("Could not extract file from archive"), because its test plugin zip is built with `ZipEncoder`. The one CI job still green is Windows / PHP 8.5, which is still on 8.5.10. Zips produced by this encoder would also be rejected by other strict tools, not only in tests. Found while investigating the unrelated CI failures on #322. ## How - The checksum is now taken from the body before it's deflated. The deflate stream wraps the checksummed stream rather than the other way around. - The first pass now drains the stream until `reached_end_of_data()` instead of stopping at the first 0-byte `pull()`. A 0-byte pull doesn't mean end-of-data (`deflate_add()` often buffers small inputs). - Dropped the unused `9` argument to `DeflateReadStream` (its constructor takes two). ## Testing - New `testStoresCrcOfUncompressedData` (deflated and stored entries, ~45 KB body spanning several reads) asserts the CRC and size `ZipArchive` reports for the entry. The CRC check fails before this change (`1411619286` vs `946377986`) on any PHP version, not only the stricter ones. - `vendor/bin/phpunit components/Zip/Tests/` and `components/Blueprints/Tests/Unit/Steps` pass.
timeout_ms keeps capping the total transfer time (0 disables it). The new idle_timeout_ms fails requests that stop sending or receiving bytes. The Blueprints runner uses the idle timeout so large downloads on slow connections can finish. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wojtekn
changed the title
Contributor
Author
Makes sense, added.
Yes, I opened that PR to fix that issue. I merged the trunk into this branch, and all tests went through. |
wojtekn
added a commit
to Automattic/studio
that referenced
this pull request
Oct 1, 2026
## Related issues - Fixes #4932 - Fixes STU-2467 ## How AI was used in this PR Claude Code traced the failure to the HTTP client bundled in `blueprints.phar`, wrote the fix and tests, and ran them. I reviewed the change and the verification steps below. ## Proposed Changes On the native PHP runtime, creating a site from a Blueprint (e.g. the quickstart site) failed on slower connections with `cURL error 28: Operation timed out after 30000 milliseconds`. The Blueprint runner inside `blueprints.phar` limits each download to 30 seconds **in total**, so large plugins fail even while data is still arriving. Gutenberg (~48 MB) needs about 45 s at 1 MB/s, which is the reporter's connection speed. Fast connections never hit the limit, which is why it doesn't reproduce for most of us. - Studio now gives the Blueprint runner's HTTP client a 10-minute timeout, so large plugin and theme downloads finish on slow connections instead of failing site creation. - The phar has no CLI option for this. The override goes through the runner's existing `blueprint.http_client` filter, registered by a small PHP file loaded with `auto_prepend_file` only for the Blueprint run. `blueprints.phar` itself is unchanged. - This is a stopgap. The upstream fix, WordPress/php-toolkit#322, adds an idle timeout to the HttpClient and switches the Blueprint runner to it, so downloads fail only if no data moves for 30 s. Once a phar with that fix is bundled, this override can be removed. ## Testing Instructions To reproduce the original failure, throttle your network to about 1 MB/s (e.g. macOS Network Link Conditioner): 1. On `trunk`, create a quickstart site with the native PHP runtime → it fails with `cURL error 28 ... after 30000 milliseconds`. 2. On this branch, repeat → the site is created and Gutenberg is installed. Without throttling: - `npm run cli:build && node apps/cli/dist/cli/main.mjs site create --runtime native --name test --path /tmp/stu2467 --blueprint bp.json` with a Blueprint that installs `https://downloads.wordpress.org/plugin/gutenberg.24.0.0.zip` → the site is created with Gutenberg installed, and no `studio-blueprint-prepend-*` dir is left in the temp directory. - To confirm the override takes effect: temporarily change `BLUEPRINT_HTTP_TIMEOUT_MS` to `1500` and rebuild. The run should fail with `timed out after 1501 milliseconds` instead of `30000`. ## Pre-merge Checklist - [ ] Have you checked for TypeScript, React or other console errors? 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds an
idle_timeout_msoption to the HttpClient. A request fails when it hasn't sent or received any bytes for that long. The existingtimeout_msstill caps the total transfer time, and0now explicitly disables it.idle_timeout_msdefaults to0(disabled), so existing callers behave exactly as before.The Blueprints runner now uses
timeout_ms => 0, idle_timeout_ms => 30000, so plugin and theme downloads fail only when they stall, not when they're slow.Why
The Blueprints runner created its client with the default 30 s total timeout. On the native runtime, any download that took longer than 30 s failed while data was still arriving. WordPress Studio users on slower connections can't create sites whose Blueprints install large plugins:
(Automattic/studio#4932: ~1 MB/s connection, 48 MB Gutenberg zip.)
Per review feedback, both kinds of timeout are useful, so this adds the idle timeout as a separate option instead of changing what
timeout_msmeans.How
Connectionrecords when bytes last moved (mark_activity(),idle_time_ms()). Both transports update it on header, body, and upload I/O.idle_timeout_msis set, both transports fail requests idle for longer withRequest timed out after N ms without network activity.The check runs after each tick's reads, so data that arrived while the caller was busy between ticks counts as activity rather than tripping the timeout.timeout_ms => 0disables the total cap: cURL already treatsCURLOPT_TIMEOUT_MS = 0as no limit, and the socket transport now skips its elapsed-time check. The socket connect timeout falls back toidle_timeout_ms, then todefault_socket_timeout.await_next_event()waits for the next event up to the tighter of the two enabled limits (plus the existing 10% / 300 ms margin), or indefinitely when both are disabled. Before,timeout_ms => 0would have made it give up after 300 ms.Other callers that download potentially large files (the content importer's
import-content.php,AttachmentDownloader) still use the defaults. They could adopt the idle timeout in a follow-up.Testing
New tests, run on both transports:
test_idle_timeout_lets_a_slow_response_complete_while_data_flows:stream/slow(~3 s total, ≤600 ms gaps) completes withtimeout_ms => 0, idle_timeout_ms => 1500.test_total_timeout_still_applies_alongside_idle_timeout: the same response still fails withtimeout_ms => 1500when an idle timeout is also set.test_idle_timeout_fails_a_stalled_response:error/timeout-read-body(stalls for 5 s mid-body) fails within the 500 ms idle timeout.vendor/bin/phpunit components/HttpClient/Tests/(174 tests, 2 consecutive runs),components/Blueprints/Tests/Unitandcomponents/Svn/Testspass. phpcs reports nothing new in the changed lines.🤖 Generated with Claude Code