Skip to content

fix(responseInterceptor): remove disallowed headers when response headers contains trailer #830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ziboilihua wants to merge 3 commits into
base: master
Choose a base branch
from
+41 −2
Open

fix(responseInterceptor): remove disallowed headers when response headers contains trailer #830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa410e3
fix(http-proxy): remove disallowed headers when response header conta…
ziboilihua Sep 22, 2022
677e0fc
Add link to verify the completeness disallow headers
ziboilihua Oct 9, 2022
d852f09
fix(responseInterceptor): fix test
chimurai Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions src/handlers/response-interceptor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,23 @@ type Interceptor<TReq = http.IncomingMessage, TRes = http.ServerResponse> = (
res: TRes,
) => Promise<Buffer | string>;

/**
* Disallow headers when response contains trailer
* source: https://developer.mozilla.org/docs/Web/HTTP/Headers/Trailer
*/
const TrailerDisallowHeaders: string[] = [
Copy link
Copy Markdown
Owner

@chimurai chimurai Oct 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a link to verify the completeness of these headers?

Copy link
Copy Markdown
Author

@ziboilihua ziboilihua Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://developer.mozilla.org/docs/Web/HTTP/Headers/Trailer

'content-length',
'host',
'content-type',
'authorization',
'cache-control',
'max-forwards',
'te',
'set-cookie',
'content-encoding',
'content-range',
];
Comment on lines +16 to +31
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

MDN Trailer header disallowed headers meaning declared as trailer vs response headers

💡 Result:

The Trailer header indicates which HTTP fields (headers) the sender plans to include in the trailer section at the end of a chunked message body. It is sent in the header section (initial headers block) of the HTTP message. Disallowed headers in trailers: Certain headers cannot be used as trailer fields because they must be processed before the body (e.g., for routing, framing, authentication). MDN lists: Content-Encoding, Content-Type, Content-Range, Trailer; authentication headers (e.g., Authorization, Set-Cookie); message framing (Transfer-Encoding, Content-Length); routing (Host); request modifiers (Cache-Control, Max-Forwards, TE). This matches RFC 9110 §6.5.1 limitations on trailers. "Declared as trailer" means listed in the Trailer header (e.g., Trailer: Server-Timing), signaling those fields will appear in the trailers section, not the initial response headers. Trailer fields are processed/stored separately from header fields (RFC 9110 §6.5). They cannot override or duplicate critical headers needed pre-body. Example: HTTP/1.1 200 OK Transfer-Encoding: chunked Trailer: Server-Timing [body] Server-Timing: custom-metric;dur=123.4 Server-Timing is declared via Trailer header and appears only in trailers, not response headers.

Citations:

🏁 Script executed:

cat -n src/handlers/response-interceptor.ts | head -50

Repository: chimurai/http-proxy-middleware

Length of output: 1872

🏁 Script executed:

cat -n src/handlers/response-interceptor.ts | sed -n '40,150p'

Repository: chimurai/http-proxy-middleware

Length of output: 4550

The TrailerDisallowHeaders list is misapplied—these headers cannot be declared as trailers, not stripped from responses.

Per MDN and RFC 9110, the disallowed headers list specifies which header names cannot appear in the trailer section at the end of a chunked response. The current code incorrectly strips these headers from the response itself whenever an upstream trailer is present (lines 73–75).

This breaks legitimate responses: content-type, set-cookie, cache-control, authorization, and content-range are removed from the response headers, causing cookies to be lost, content-type to be missing, and auth to fail in downstream clients.

The actual issue is that content-length is incompatible with trailers (which require chunked transfer encoding). The fix should only remove content-length when a trailer is present, not the entire list of headers.

Suggested fix 
-      // some headers are disallowed when response headers contains trailer
-      if (proxyRes.headers.trailer === undefined) {
-        res.setHeader('content-length', Buffer.byteLength(interceptedBuffer, 'utf8'));
-      } else {
-        TrailerDisallowHeaders.forEach((value) => {
-          res.removeHeader(value);
-        });
-      }
+      // content-length is incompatible with trailers (which require chunked
+      // transfer encoding), so only set it when no trailer is declared.
+      if (proxyRes.headers.trailer === undefined) {
+        res.setHeader('content-length', Buffer.byteLength(interceptedBuffer, 'utf8'));
+      } else {
+        res.removeHeader('content-length');
+      }

Remove the TrailerDisallowHeaders constant (lines 20–31) as it is no longer needed.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/handlers/response-interceptor.ts` around lines 16 - 31, Remove the
incorrect TrailerDisallowHeaders constant and change the logic that strips
headers when an upstream trailer is present so it only removes the
content-length header; specifically, delete TrailerDisallowHeaders and update
the code block that iterates/removes headers (the section that currently
references TrailerDisallowHeaders) to only delete 'content-length' from the
response headers when trailers exist, leaving content-type, set-cookie,
cache-control, authorization, content-range, etc. untouched.


/**
* Intercept responses from upstream.
* Automatically decompress (deflate, gzip, brotli).
Expand Down Expand Up @@ -49,8 +66,14 @@ export function responseInterceptor<

// set correct content-length (with double byte character support)
debug('set content-length: %s', Buffer.byteLength(interceptedBuffer, 'utf8'));
res.setHeader('content-length', Buffer.byteLength(interceptedBuffer, 'utf8'));

// some headers are disallowed when response headers contains trailer
if (proxyRes.headers.trailer === undefined) {
res.setHeader('content-length', Buffer.byteLength(interceptedBuffer, 'utf8'));
} else {
TrailerDisallowHeaders.forEach((value) => {
res.removeHeader(value);
});
}
Comment on lines +69 to +76
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Also note: the interceptor mutates the response buffer, so the upstream content-length copied in copyHeaders is stale when a trailer is present.

In the trailer branch, neither content-length is updated nor removed under the narrower fix unless explicitly handled. If you keep the broader header-stripping approach, make sure this branch is exercised by a test that actually triggers a response carrying both Trailer and Content-Length from the upstream — the new E2E test (see test/e2e/response-interceptor.spec.ts) does not do this.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/handlers/response-interceptor.ts` around lines 69 - 76, The trailer
branch currently strips headers via TrailerDisallowHeaders but fails to update
or remove the stale content-length copied earlier (see copyHeaders) even though
the interceptor mutates interceptedBuffer; modify the branch guarded by
proxyRes.headers.trailer to explicitly remove or recalculate the content-length
header on res (remove content-length or set it to
Buffer.byteLength(interceptedBuffer,'utf8')) and ensure TrailerDisallowHeaders
handling still runs; update tests to exercise an upstream response that includes
both Trailer and Content-Length so this branch behavior is validated (refer to
TrailerDisallowHeaders, copyHeaders, interceptedBuffer, and
proxyRes.headers.trailer).

debug('write intercepted response');
res.write(interceptedBuffer);
res.end();
Expand Down
16 changes: 16 additions & 0 deletions test/e2e/response-interceptor.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ describe('responseInterceptor()', () => {
'content-type': 'image/png',
});

await targetServer
.forGet('/response-headers')
.withExactQuery('?Trailer=X-Stream-Error&Host=localhost')
.thenReply(200, '', {
'transfer-encoding': 'chunked',
trailer: 'X-Stream-Error',
host: 'localhost',
});

agent = request(
createApp(
createProxyMiddleware({
Expand Down Expand Up @@ -65,6 +74,13 @@ describe('responseInterceptor()', () => {
const response = await agent.get(`/json`).expect(200);
expect(response.body.favorite).toEqual('叉燒包');
});

it('should not contains disallow headers to trailer in response headers', async () => {
const response = await agent
.get('/response-headers?Trailer=X-Stream-Error&Host=localhost')
.expect(200);
expect(response.header['host']).toBeUndefined();
});
});

describe('intercept responses with original headers', () => {
Expand Down