Disable default request headers when proxying in modern Node (22.13+)

pimterry · pimterry · commit e45d02383558 · 2025-05-20T17:01:29.000+02:00
This is a breaking change as it changes the logic around message framing
when overriding with a custom body. In the past, we attempted to fix the
framing (i.e. setting a content-length header) any time you didn't
specifically disable this. Now, we _always_ fix the framing if you
modify a request or response body, to ensure it's always valid. This was
somewhat handled in the past by the default request headers, but now
that those are disabled the UX is very poor without this.

This should not affect you, unless you're intentionally setting an
invalid content-length or sending requests (not responses) with a body
but no framing (in which case they're likely unparseable anyway). Normal
valid requests will never be changed.
diff --git a/src/rules/passthrough-handling.ts b/src/rules/passthrough-handling.ts
@@ -286,50 +286,94 @@ export function getH2HeadersAfterModification(
     };
 }
 
-// Helper to handle content-length nicely for you when rewriting requests with callbacks
-export function getContentLengthAfterModification(
+// When modifying requests, we ensure you always have correct framing, as it's impossible
+// to send a request with framing that doesn't match the body.
+export function getRequestContentLengthAfterModification(
     body: string | Uint8Array | Buffer,
     originalHeaders: Headers | RawHeaders,
     replacementHeaders: Headers | RawHeaders | undefined,
-    mismatchAllowed: boolean = false
+    context: {
+        httpVersion: 1 | 2
+        // N.b. we ignore the method though - you can proxy requests that include a body
+        // even if they really shouldn't, as long as it's plausibly parseable.
+    }
 ): string | undefined {
     // If there was a content-length header, it might now be wrong, and it's annoying
     // to need to set your own content-length override when you just want to change
-    // the body. To help out, if you override the body but don't explicitly override
-    // the (now invalid) content-length, then we fix it for you.
+    // the body. To help out, if you override the body in a way that results in invalid
+    // content-length headers, we fix them for you.
+
+    // For HTTP/2, framing is optional/advisory so we can just skip this entirely.
+    if (context.httpVersion !== 1) return undefined;
+
+    const resultingHeaders = replacementHeaders || originalHeaders;
 
-    if (getHeaderValue(originalHeaders, 'content-length') === undefined) {
-        // Nothing to override - use the replacement value, or undefined
-        return getHeaderValue(replacementHeaders || {}, 'content-length');
+    if (getHeaderValue(resultingHeaders, 'transfer-encoding')?.includes('chunked')) {
+        return undefined; // No content-length header games needed
     }
 
-    if (!replacementHeaders) {
-        // There was a length set, and you've provided a body but not changed it.
-        // You probably just want to send this body and have it work correctly,
-        // so we should fix the content length for you automatically.
-        return byteLength(body).toString();
+    const expectedLength = byteLength(body).toString();
+    const contentLengthHeader = getHeaderValue(resultingHeaders, 'content-length');
+
+    if (contentLengthHeader === expectedLength) return undefined;
+    if (contentLengthHeader === undefined) return expectedLength; // Differs from responses
+
+    // The content-length is expected, but it's wrong or missing.
+
+    // If there is a wrong content-length set, and it's not just leftover from the original headers (i.e.
+    // you intentionally set it) then we show a warning since we're ignoring your (invalid) instructions.
+    if (contentLengthHeader && contentLengthHeader !== getHeaderValue(originalHeaders, 'content-length')) {
+        console.warn(`Invalid request content-length header was ignored - resetting from ${
+            contentLengthHeader
+        } to ${
+            expectedLength
+        }`);
     }
 
-    // There was a content length before, and you're replacing the headers entirely
-    const lengthOverride = getHeaderValue(replacementHeaders, 'content-length')?.toString();
+    return expectedLength;
+}
 
-    // If you're setting the content-length to the same as the origin headers, even
-    // though that's the wrong value, it *might* be that you're just extending the
-    // existing headers, and you're doing this by accident (we can't tell for sure).
-    // We use invalid content-length as instructed, but print a warning just in case.
-    if (
-        lengthOverride === getHeaderValue(originalHeaders, 'content-length') &&
-        lengthOverride !== byteLength(body).toString() &&
-        !mismatchAllowed // Set for HEAD responses
-    ) {
-        console.warn(oneLine`
-            Passthrough modifications overrode the body and the content-length header
-            with mismatched values, which may be a mistake. The body contains
-            ${byteLength(body)} bytes, whilst the header was set to ${lengthOverride}.
-        `);
+// When modifying responses, we ensure you always have correct framing, but in a slightly more
+// relaxed way than for requests: we allow no framing and HEAD responses, we just block invalid values.
+export function getResponseContentLengthAfterModification(
+    body: string | Uint8Array | Buffer,
+    originalHeaders: Headers | RawHeaders,
+    replacementHeaders: Headers | RawHeaders | undefined,
+    context: {
+        httpMethod: string
+        httpVersion: 1 | 2
+    }
+): string | undefined {
+    // For HEAD requests etc, you can set an arbitrary content-length header regardless
+    // of the empty body, so we don't bother checking anything. For HTTP/2, framing is
+    // optional/advisory so we can just skip this entirely.
+    if (context.httpVersion !== 1 || context.httpMethod === 'HEAD') return undefined;
+
+    const resultingHeaders = replacementHeaders || originalHeaders;
+
+    if (getHeaderValue(resultingHeaders, 'transfer-encoding')?.includes('chunked')) {
+        return undefined; // No content-length header games needed
+    }
+
+    const expectedLength = byteLength(body).toString();
+    const contentLengthHeader = getHeaderValue(resultingHeaders, 'content-length');
+
+    if (contentLengthHeader === expectedLength) return undefined;
+    if (contentLengthHeader === undefined) return undefined; // Differs from requests - we do allow this for responses
+
+    // The content-length is set, but it's wrong.
+
+    // If there is a wrong content-length set, and it's not just leftover from the original headers (i.e.
+    // you intentionally set it) then we show a warning since we're ignoring your (invalid) instructions.
+    if (contentLengthHeader && contentLengthHeader !== getHeaderValue(originalHeaders, 'content-length')) {
+        console.warn(`Invalid response content-length header was ignored - resetting from ${
+            contentLengthHeader
+        } to ${
+            expectedLength
+        }`);
     }
 
-    return lengthOverride;
+    return expectedLength;
 }
 
 // Function to check if we should skip https errors for the current hostname and port,
diff --git a/src/rules/requests/request-handlers.ts b/src/rules/requests/request-handlers.ts
@@ -74,7 +74,8 @@ import {
     PassThroughLookupOptions,
 } from '../passthrough-handling-definitions';
 import {
-    getContentLengthAfterModification,
+    getRequestContentLengthAfterModification,
+    getResponseContentLengthAfterModification,
     getHostAfterModification,
     getH2HeadersAfterModification,
     MODIFIABLE_PSEUDOHEADERS,
@@ -556,19 +557,23 @@ export class PassThroughHandler extends PassThroughHandlerDefinition {
                 }
             }
 
-            if (reqBodyOverride) {
+            if (reqBodyOverride) { // Can't check framing without body changes, since we won't have the body yet
                 // We always re-encode the body to match the resulting content-encoding header:
                 reqBodyOverride = await encodeBodyBuffer(
                     reqBodyOverride,
                     rawHeaders
                 );
 
-                const updatedCLHeader = getContentLengthAfterModification(
+                const updatedCLHeader = getRequestContentLengthAfterModification(
                     reqBodyOverride,
                     clientReq.headers,
-                    (updateHeaders && getHeaderValue(updateHeaders, 'content-length') !== undefined)
-                        ? rawHeaders // Iff you replaced the content length
-                        : replaceHeaders
+                    (updateHeaders && (
+                        getHeaderValue(updateHeaders, 'content-length') !== undefined ||
+                        getHeaderValue(updateHeaders, 'transfer-encoding')?.includes('chunked')
+                    ))
+                        ? rawHeaders // Iff you replaced the relevant headers
+                        : replaceHeaders,
+                    { httpVersion: isH2Downstream ? 2 : 1 }
                 );
 
                 if (updatedCLHeader !== undefined) {
@@ -637,12 +642,13 @@ export class PassThroughHandler extends PassThroughHandlerDefinition {
 
             reqBodyOverride = await buildOverriddenBody(modifiedReq, headers);
 
-            if (reqBodyOverride) {
+            if (reqBodyOverride || modifiedReq?.headers) {
                 // Automatically match the content-length to the body, unless it was explicitly overriden.
-                headers['content-length'] = getContentLengthAfterModification(
-                    reqBodyOverride,
+                headers['content-length'] = getRequestContentLengthAfterModification(
+                    reqBodyOverride || completedRequest.body.buffer,
                     clientHeaders,
-                    modifiedReq?.headers
+                    modifiedReq?.headers,
+                    { httpVersion: isH2Downstream ? 2 : 1 }
                 );
             }
 
@@ -747,6 +753,7 @@ export class PassThroughHandler extends PassThroughHandlerDefinition {
                 headers: shouldTryH2Upstream
                     ? rawHeadersToObjectPreservingCase(rawHeaders)
                     : flattenPairedRawHeaders(rawHeaders) as any,
+                setDefaultHeaders: shouldTryH2Upstream, // For now, we need this for unexpected H2->H1 header fallback
                 lookup: getDnsLookupFunction(this.lookupOptions) as typeof dns.lookup,
                 // ^ Cast required to handle __promisify__ type hack in the official Node types
                 agent,
@@ -889,21 +896,21 @@ export class PassThroughHandler extends PassThroughHandlerDefinition {
                         }
                     }
 
-                    if (resBodyOverride) {
+                    if (resBodyOverride) { // Can't check framing without body changes, since we won't have the body yet
                         // In the above cases, the overriding data is assumed to always be in decoded form,
                         // so we re-encode the body to match the resulting content-encoding header:
                         resBodyOverride = await encodeBodyBuffer(
                             resBodyOverride,
                             serverRawHeaders
                         );
 
-                        const updatedCLHeader = getContentLengthAfterModification(
+                        const updatedCLHeader = getResponseContentLengthAfterModification(
                             resBodyOverride,
                             serverRes.headers,
                             (updateHeaders && getHeaderValue(updateHeaders, 'content-length') !== undefined)
                                 ? serverRawHeaders // Iff you replaced the content length
                                 : replaceHeaders,
-                            method === 'HEAD' // HEAD responses are allowed mismatched content-length
+                            { httpMethod: method, httpVersion: serverRes.httpVersion.startsWith('1.') ? 1 : 2 }
                         );
 
                         if (updatedCLHeader !== undefined) {
@@ -980,13 +987,20 @@ export class PassThroughHandler extends PassThroughHandlerDefinition {
 
                     resBodyOverride = await buildOverriddenBody(modifiedRes, serverHeaders);
 
-                    if (resBodyOverride) {
-                        serverHeaders['content-length'] = getContentLengthAfterModification(
-                            resBodyOverride,
+                    if (resBodyOverride || modifiedRes?.headers) {
+                        const updatedContentLength = getResponseContentLengthAfterModification(
+                            resBodyOverride || originalBody,
                             serverRes.headers,
                             modifiedRes?.headers,
-                            method === 'HEAD' // HEAD responses are allowed mismatched content-length
+                            {
+                                httpMethod: method,
+                                httpVersion: serverRes.httpVersion.startsWith('1.') ? 1 : 2
+                            }
                         );
+
+                        if (updatedContentLength !== undefined) {
+                            serverHeaders['content-length'] = updatedContentLength;
+                        }
                     }
 
                     serverRawHeaders = objectHeadersToRaw(serverHeaders);
diff --git a/test/integration/proxying/proxy-transforms.spec.ts b/test/integration/proxying/proxy-transforms.spec.ts
@@ -1,15 +1,17 @@
 import _ = require("lodash");
 import * as path from 'path';
 import * as http from 'http';
+import * as zlib from 'zlib';
 
 import request = require("request-promise-native");
-import * as zlib from 'zlib';
 
 import { getLocal, Mockttp } from "../../..";
 import {
     expect,
     nodeOnly,
-    defaultNodeConnectionHeader
+    defaultNodeConnectionHeader,
+    nodeSatisfies,
+    DEFAULT_REQ_HEADERS_DISABLED
 } from "../../test-utils";
 import { streamToBuffer } from "../../../src/util/buffer-utils";
 
@@ -310,6 +312,7 @@ nodeOnly(() => {
                 await server.forAnyRequest().thenPassThrough({
                     transformRequest: {
                         replaceHeaders: {
+                            'transfer-encoding': 'chunked', // Required for body
                             'custom-header': 'replaced-value'
                         }
                     }
@@ -324,11 +327,10 @@ nodeOnly(() => {
                 expect(response.url).to.equal(`http://localhost:${remoteServer.port}/abc`); // From tunnel, even without the host header
                 expect(response.method).to.equal('POST');
                 expect(response.headers).to.deep.equal({
-                    // Default Node headers:
-                    'connection': defaultNodeConnectionHeader,
+                    ...(!nodeSatisfies(DEFAULT_REQ_HEADERS_DISABLED)
+                        ? { 'connection': defaultNodeConnectionHeader }
+                    : {}),
                     'transfer-encoding': 'chunked',
-
-                    // No other headers, only injected value:
                     'custom-header': 'replaced-value'
                 });
                 expect(response.body).to.equal(JSON.stringify({ a: 1 }));
diff --git a/test/integration/remote-client.spec.ts b/test/integration/remote-client.spec.ts
@@ -254,7 +254,6 @@ nodeOnly(() => {
                     await remoteServer.forPost(targetServer.urlFor('/res')).thenPassThrough({
                         transformResponse: {
                             updateHeaders: {
-                                'custom-header': undefined, // Remove
                                 'injected-header': 'injected-value' // Add
                             },
                             updateJsonBody: {
diff --git a/test/test-utils.ts b/test/test-utils.ts
@@ -308,6 +308,7 @@ export const BROKEN_H1_OVER_H2_TUNNELLING = "^18.8";
 export const DEFAULT_KEEP_ALIVE = ">=19";
 export const FIXED_KEEP_ALIVE_BEHAVIOUR = ">=20";
 export const BROKEN_H2_OVER_H2_TUNNELLING = "~20.12"; // https://github.com/nodejs/node/issues/52344
+export const DEFAULT_REQ_HEADERS_DISABLED = "^22.13.0 || >=23.5.0";
 
 export const defaultNodeConnectionHeader = nodeSatisfies(DEFAULT_KEEP_ALIVE)
     ? 'keep-alive'
