Skip to content

Commit 7a7ffd7

Browse files
fix: prevent double decompression in node:http by disabling fetch auto-decode
The node:http ClientRequest uses fetch() internally, which auto-decompresses gzip/brotli responses by default. However, it preserves the Content-Encoding header, causing callers that follow the Node.js convention of manually handling Content-Encoding to attempt double decompression. Pass encodeResponseBody: 'manual' to fetch() so raw compressed bytes are delivered to IncomingMessage, matching Node.js http behavior where callers handle decompression themselves.
1 parent f403e9b commit 7a7ffd7

File tree

5 files changed

+63
-10
lines changed

5 files changed

+63
-10
lines changed

src/node/internal/internal_http_client.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -365,24 +365,29 @@ export class ClientRequest extends OutgoingMessage implements _ClientRequest {
365365
url = new URL(this.path, url);
366366
}
367367

368-
// Our fetch implementation has the following limitations.
368+
// Our fetch implementation has the following limitation:
369369
//
370-
// 1. Content decoding is handled automatically by fetch,
371-
// but expectation is that it's not handled in http.
372-
// 2. Nothing is directly waiting for fetch promise here.
373-
// It's up to the user of the HTTP API to arrange for
374-
// the request to be held open until the fetch completes,
375-
// typically by passing some promise to ctx.waitUntil()
376-
// and resolving that promise when the request is complete.
370+
// Nothing is directly waiting for fetch promise here.
371+
// It's up to the user of the HTTP API to arrange for
372+
// the request to be held open until the fetch completes,
373+
// typically by passing some promise to ctx.waitUntil()
374+
// and resolving that promise when the request is complete.
377375
//
378-
// TODO(soon): Address these concerns and limitations.
376+
// TODO(soon): Address this limitation.
377+
378+
// We use encodeResponseBody: 'manual' to prevent fetch from automatically
379+
// decompressing the response body. Node.js http does not auto-decompress;
380+
// callers are expected to handle Content-Encoding themselves.
381+
// The type assertion is needed because the DOM RequestInit type does not
382+
// include the workerd-specific encodeResponseBody property.
379383
fetch(url, {
380384
method: this.method,
381385
headers,
382386
body: body ?? null,
383387
signal: this.#abortController.signal,
384388
redirect: 'manual',
385-
})
389+
encodeResponseBody: 'manual',
390+
} as unknown as RequestInit)
386391
.then(this.#handleFetchResponse.bind(this))
387392
.catch(this.#handleFetchError.bind(this));
388393
}

src/workerd/api/node/tests/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,7 @@ wd_test(
510510
"DEFAULT_HEADERS_EXIST_PORT",
511511
"REQUEST_ARGUMENTS_PORT",
512512
"HELLO_WORLD_SERVER_PORT",
513+
"GZIP_SERVER_PORT",
513514
],
514515
)
515516

src/workerd/api/node/tests/http-client-nodejs-server.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// It is executed using the appropriate Node.js version defined in build/deps/nodejs.MODULE.bazel.
77
const http = require('node:http');
88
const assert = require('node:assert/strict');
9+
const zlib = require('node:zlib');
910

1011
function listenTo(server, port) {
1112
server.listen(port, process.env.SIDECAR_HOSTNAME, () => {
@@ -120,3 +121,14 @@ const helloWorldServer = http.createServer((req, res) => {
120121
});
121122

122123
listenTo(helloWorldServer, process.env.HELLO_WORLD_SERVER_PORT);
124+
125+
const gzipServer = http.createServer((_req, res) => {
126+
const body = zlib.gzipSync(Buffer.from('hello from gzip server'));
127+
res.writeHead(200, {
128+
'Content-Encoding': 'gzip',
129+
'Content-Type': 'text/plain',
130+
});
131+
res.end(body);
132+
});
133+
134+
listenTo(gzipServer, process.env.GZIP_SERVER_PORT);

src/workerd/api/node/tests/http-client-nodejs-test.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import http from 'node:http';
66
import { strictEqual, ok, throws, deepStrictEqual } from 'node:assert';
77
import { once } from 'node:events';
88
import { get } from 'node:http';
9+
import { gunzipSync } from 'node:zlib';
910

1011
export const checkPortsSetCorrectly = {
1112
test(_ctrl, env) {
@@ -16,6 +17,7 @@ export const checkPortsSetCorrectly = {
1617
'DEFAULT_HEADERS_EXIST_PORT',
1718
'REQUEST_ARGUMENTS_PORT',
1819
'HELLO_WORLD_SERVER_PORT',
20+
'GZIP_SERVER_PORT',
1921
];
2022
for (const key of keys) {
2123
strictEqual(typeof env[key], 'string');
@@ -529,6 +531,38 @@ export const testBodyDataDuplicationRegression = {
529531
},
530532
};
531533

534+
// Verify that the http module does not automatically decompress response bodies.
535+
// Node.js http passes raw bytes through, leaving Content-Encoding handling to the caller.
536+
export const testHttpClientGzipResponseNotAutoDecompressed = {
537+
async test(_ctrl, env) {
538+
const { promise, resolve, reject } = Promise.withResolvers();
539+
http.get(
540+
{
541+
hostname: env.SIDECAR_HOSTNAME,
542+
port: env.GZIP_SERVER_PORT,
543+
},
544+
(res) => {
545+
strictEqual(res.headers['content-encoding'], 'gzip');
546+
const chunks = [];
547+
res.on('data', (chunk) => chunks.push(chunk));
548+
res.on('end', () => {
549+
try {
550+
const raw = Buffer.concat(chunks);
551+
// The body should still be compressed — manually gunzipping
552+
// should recover the original text.
553+
const decompressed = gunzipSync(raw);
554+
strictEqual(decompressed.toString(), 'hello from gzip server');
555+
resolve();
556+
} catch (err) {
557+
reject(err);
558+
}
559+
});
560+
}
561+
);
562+
await promise;
563+
},
564+
};
565+
532566
// Relevant Node.js tests
533567
// - [ ] test/parallel/test-http-client-abort-destroy.js
534568
// - [ ] test/parallel/test-http-client-abort-event.js

src/workerd/api/node/tests/http-client-nodejs-test.wd-test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const unitTests :Workerd.Config = (
1515
(name = "DEFAULT_HEADERS_EXIST_PORT", fromEnvironment = "DEFAULT_HEADERS_EXIST_PORT"),
1616
(name = "REQUEST_ARGUMENTS_PORT", fromEnvironment = "REQUEST_ARGUMENTS_PORT"),
1717
(name = "HELLO_WORLD_SERVER_PORT", fromEnvironment = "HELLO_WORLD_SERVER_PORT"),
18+
(name = "GZIP_SERVER_PORT", fromEnvironment = "GZIP_SERVER_PORT"),
1819
],
1920
)
2021
),

0 commit comments

Comments
 (0)