[tcp-over-fetch] Strip Content-Encoding from response headers#3355
Merged
[tcp-over-fetch] Strip Content-Encoding from response headers#3355
Conversation
The browser decompresses response bodies transparently (gzip, brotli, deflate), but the Content-Encoding header may still be present on the Response object. The tcp-over-fetch bridge reconstructs raw HTTP bytes from the browser's Response to feed back to PHP's curl. If it includes Content-Encoding, PHP's curl sees a compression header that doesn't match the already-decompressed body, causing decode failures. The existing code already handled an analogous problem for Content-Length (compressed size vs decompressed body size) by stripping it and using chunked transfer encoding instead. This applies the same reasoning to Content-Encoding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where PHP's curl inside WASM would fail to decode response bodies because the Content-Encoding header (e.g., br for Brotli) was being passed through even though the browser had already transparently decompressed the body. The fix mirrors the existing Content-Length stripping logic.
Changes:
- Strip
Content-Encodingheader inheadersAsBytes()alongside the existingContent-Lengthdeletion - Add a new unit test verifying
Content-Encodingis stripped from the reconstructed raw HTTP response - Extend an existing test to assert
content-encodingis absent from gzipped responses
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/php-wasm/web/src/lib/tcp-over-fetch-websocket.ts |
Deletes content-encoding header to prevent curl decode failures |
packages/php-wasm/web/src/lib/tcp-over-fetch-websocket.spec.ts |
Adds and extends tests to cover the Content-Encoding stripping behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+744
to
+749
| let raw = ''; | ||
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
| raw += new TextDecoder().decode(value); | ||
| } |
There was a problem hiding this comment.
A new TextDecoder instance is created on every chunk iteration inside the loop. When a stream has many chunks, this is wasteful. A single TextDecoder should be instantiated once before the loop and reused, or all chunks should be accumulated as Uint8Array buffers and decoded once after the loop completes.
…d responses Boots PHP 8.4 in the browser with tcpOverFetch networking and runs curl against a mock HTTP server that returns gzip and brotli compressed responses. Verifies PHP receives the correct decompressed body with the right length – the full round-trip from PHP's curl through the tcp-over-fetch bridge and back. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Under JSPI, calling php.exit() before reading response.stdoutText causes the stream to hang indefinitely. Read the output first, then exit. 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
In the browser, curl requests failed with content decoding error when the server provided a response header such as
Content-Encoding: br. This is because the TCP-over-fetch bridge passed both of these back to curl:Content-encoding: brheaderThe PR makes curl work again by removing the
Content-Encodingheader from the list of headers passed back to curl. The upside (curl not failing) wins over the downside (PHP can't see that header even though the server provided it).Test plan
npx nx test php-wasm-web --testFile=tcp-over-fetch-websocket.spec.ts— all 28 vitest tests pass (unit test verifies Content-Encoding is stripped from raw bytes; gzipped e2e test asserts absence of content-encoding header)npx nx e2e php-wasm-web— Playwright tests boot PHP 8.4 in the browser with tcpOverFetch networking and run PHPcurl_exec()against a mock server returning gzip and brotli compressed responses; PHP receives the correct decompressed body with the right length