[tcp-over-fetch] Buffer request body for non-HTTPS fetches#3356
Merged
[tcp-over-fetch] Buffer request body for non-HTTPS fetches#3356
Conversation
…rrors HTTPS requests from PHP that fail direct fetch (CORS) and fall back to a CORS proxy on the local dev server (http://) were failing because Chrome requires HTTP/2 for streaming request bodies (duplex: 'half') but the dev server only speaks HTTP/1.1. Instead of scattering buffering decisions across tcp-over-fetch and parseHttpRequest, this introduces httpSafeFetch() – a fetch() wrapper in fetchWithCorsProxy that buffers the request body whenever the target URL is http://. Every fetch() call in fetchWithCorsProxy now goes through this wrapper, handling all cases uniformly: direct requests, localhost, same-origin, and CORS proxy fallbacks. As a side effect, the duplicated request-parsing logic between fetchOverTLS and fetchOverHTTP is consolidated into a shared parseRequestAndFetch helper.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes request-body buffering to avoid Chrome HTTP/1.1 streaming-body failures (e.g. ERR_ALPN_NEGOTIATION_FAILED) by routing all fetch() calls in the CORS-proxy path through a single wrapper, and consolidates duplicated request parsing/fetch piping logic.
Changes:
- Introduces
httpSafeFetch()infetchWithCorsProxy()to buffer bodies forhttp://targets before callingfetch(). - Refactors tcp-over-fetch websocket code to use a shared
parseRequestAndFetch()helper (includingExpect: 100-continuehandling). - Removes the per-call-site “needsBodyBuffering” logic from
RawBytesFetch.parseHttpRequest()and its callers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/php-wasm/web/src/lib/tcp-over-fetch-websocket.ts | Consolidates request parsing + 100-continue handshake + response piping into parseRequestAndFetch() and removes local body-buffering logic. |
| packages/php-wasm/web-service-worker/src/fetch-with-cors-proxy.ts | Adds httpSafeFetch() wrapper and routes all fetch attempts (direct, same-origin, localhost, and proxy fallback) through it to buffer HTTP bodies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/php-wasm/web-service-worker/src/fetch-with-cors-proxy.ts
Outdated
Show resolved
Hide resolved
packages/php-wasm/web-service-worker/src/fetch-with-cors-proxy.ts
Outdated
Show resolved
Hide resolved
packages/php-wasm/web-service-worker/src/fetch-with-cors-proxy.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Cover the three key behaviors: HTTP requests get their streaming body buffered before fetch(), HTTPS requests pass through without buffering, and the CORS proxy retry path correctly buffers and preserves the body when routed through an http:// proxy (including when init is forwarded).
The previous tests only checked that the body content survived the round-trip. They didn't verify that buffering (via Response.arrayBuffer()) actually happened for http:// URLs or that it was correctly skipped for https:// URLs. Spy on Response.prototype.arrayBuffer to prove the buffering contract. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the Response.prototype.arrayBuffer spy (which only proved an implementation detail) with a Proxy on the Request constructor that records whether each Request was built with a ReadableStream body or a buffer body. This directly asserts the property that matters: the body type that reaches new Request() determines whether duplex: 'half' is needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Drop the Proxy-on-Request-constructor infrastructure. Instead, check bodyUsed on the original request (true = body was consumed for buffering) and identity on the sent request (same object = no transformation). Much simpler, same coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
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.
Summary
HTTPS requests from PHP (e.g. curl to
https://example.com) go through the TLS code path in tcp-over-fetch, which decrypts the traffic and runs afetch(). When the direct fetch fails due to CORS,fetchWithCorsProxyretries through the CORS proxy. On the local dev server, that proxy ishttp://localhost— and Chrome refuses to stream request bodies over HTTP/1.1 (ERR_ALPN_NEGOTIATION_FAILED).The body buffering logic was previously scattered in
tcp-over-fetch-websocket.tsand only covered thehttp://protocol path, missing the TLS-via-HTTP-CORS-proxy case entirely. This PR moves all buffering decisions into a singlehttpSafeFetch()wrapper insidefetchWithCorsProxy. Everyfetch()call in that function now goes through the wrapper, which buffers the request body whenever the target URL ishttp://. This handles all cases uniformly: direct requests, localhost, same-origin playground URLs, and CORS proxy fallbacks.As a bonus, the duplicated request-parsing logic between
fetchOverTLSandfetchOverHTTPis consolidated into a sharedparseRequestAndFetchhelper.Test plan
npx nx test php-wasm-web --testFile=tcp-over-fetch-websocket.spec.tsandnpx nx test php-wasm-web-service-worker --testFile=fetch-with-cors-proxy.spec.ts)npm run dev), use a Blueprint that does a curl HTTPS request (e.g. installing a plugin from wordpress.org), confirm it succeeds