mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-07-26 11:49:16 +00:00
fix(http): strip zstd from Accept-Encoding so compression sets Content-Encoding (#1092)
compression@1.8.1's negotiator silently fails to set the Content-Encoding response header when Accept-Encoding carries an unknown token like zstd, even though the body is still compressed with brotli. Chromium 123+ sends "Accept-Encoding: zstd, gzip, br" by default, including Playwright's bundled Chromium over plain HTTP, so any homelab Sencho viewed without a TLS terminator triggered ERR_CONTENT_DECODING_FAILED in the browser and rendered as a blank page. Insert a tiny middleware (step 4 of the canonical pipeline) that drops zstd tokens from Accept-Encoding before compression's negotiator runs. The negotiator then picks br or gzip and writes the matching header. The fix is no-op when zstd is absent. If only zstd was offered, the header falls back to identity so the response is served uncompressed. Renumber the canonical-order JSDoc in app.ts from 17 to 18 steps and update the step-number references in hub-only-guard and proxy-mount-order test docstrings to match. 11 new tests (8 unit + 3 supertest integration against real compression with a 3000-byte body) lock in the behavior: zstd-stripping across casing and q-values, surviving tokens preserved verbatim, identity fallback when only zstd was offered, Content-Encoding header always written when at least one supported codec remains.
This commit is contained in:
@@ -9,8 +9,8 @@
|
||||
* on a remote instance, crossing a node-authority boundary that the UI
|
||||
* promises will not happen.
|
||||
*
|
||||
* The guard sits between `enforceApiTokenScope` (step 12) and
|
||||
* `createRemoteProxyMiddleware` (step 14).
|
||||
* The guard sits between `enforceApiTokenScope` (step 13) and
|
||||
* `createRemoteProxyMiddleware` (step 15).
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import request from 'supertest';
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
/**
|
||||
* Regression guard for `normalizeAcceptEncoding`.
|
||||
*
|
||||
* Background: `compression@1.8.1` (via `negotiator@1.0.0`) misbehaves when
|
||||
* `Accept-Encoding` carries an unknown token such as `zstd`. The library
|
||||
* compresses the response body but fails to set a `Content-Encoding` header,
|
||||
* so the browser decode-fails with `ERR_CONTENT_DECODING_FAILED` and the
|
||||
* page renders blank. Chromium 123+ sends `Accept-Encoding: zstd, gzip, br`
|
||||
* by default, including Playwright's bundled Chromium even on plain HTTP.
|
||||
*
|
||||
* The middleware drops `zstd` tokens before `compression` negotiates so the
|
||||
* negotiator falls back to `br` or `gzip` and writes the matching response
|
||||
* header. These tests cover the unit-level transform and an end-to-end pass
|
||||
* through `compression` + a route that returns a 2 KB body (above the
|
||||
* compression threshold).
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import express, { type Request, type Response } from 'express';
|
||||
import compression from 'compression';
|
||||
import request from 'supertest';
|
||||
import { normalizeAcceptEncoding } from '../middleware/normalizeAcceptEncoding';
|
||||
|
||||
function runNormalize(accept: string | undefined): string | undefined {
|
||||
const headers: Record<string, string | undefined> = {};
|
||||
if (accept !== undefined) headers['accept-encoding'] = accept;
|
||||
const req = { headers } as unknown as Parameters<typeof normalizeAcceptEncoding>[0];
|
||||
const res = {} as Parameters<typeof normalizeAcceptEncoding>[1];
|
||||
normalizeAcceptEncoding(req, res, () => { /* noop */ });
|
||||
const after = req.headers['accept-encoding'];
|
||||
return typeof after === 'string' ? after : undefined;
|
||||
}
|
||||
|
||||
function makeApp(): express.Express {
|
||||
const app = express();
|
||||
app.use(normalizeAcceptEncoding);
|
||||
app.use(compression());
|
||||
// Payload large enough to be compressed (default threshold is 1 KB).
|
||||
const body = 'abcdefghij'.repeat(300);
|
||||
app.get('/text', (_req: Request, res: Response) => {
|
||||
res.type('text/plain').send(body);
|
||||
});
|
||||
return app;
|
||||
}
|
||||
|
||||
describe('normalizeAcceptEncoding (unit)', () => {
|
||||
it('passes through when Accept-Encoding is missing', () => {
|
||||
expect(runNormalize(undefined)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('passes through gzip/deflate/br untouched', () => {
|
||||
expect(runNormalize('gzip')).toBe('gzip');
|
||||
expect(runNormalize('gzip, deflate, br')).toBe('gzip, deflate, br');
|
||||
expect(runNormalize('br;q=0.5, gzip;q=1.0')).toBe('br;q=0.5, gzip;q=1.0');
|
||||
});
|
||||
|
||||
it('strips a bare zstd token while preserving order', () => {
|
||||
expect(runNormalize('zstd, gzip, br')).toBe('gzip, br');
|
||||
expect(runNormalize('gzip, zstd, br')).toBe('gzip, br');
|
||||
expect(runNormalize('gzip, br, zstd')).toBe('gzip, br');
|
||||
});
|
||||
|
||||
it('strips zstd with a q-value', () => {
|
||||
expect(runNormalize('zstd;q=1.0, gzip;q=0.8')).toBe('gzip;q=0.8');
|
||||
expect(runNormalize('gzip, zstd;q=0.5, br')).toBe('gzip, br');
|
||||
});
|
||||
|
||||
it('strips zstd case-insensitively', () => {
|
||||
expect(runNormalize('ZSTD, gzip')).toBe('gzip');
|
||||
expect(runNormalize('Zstd;q=1.0, br')).toBe('br');
|
||||
});
|
||||
|
||||
it('falls back to identity when only zstd was offered', () => {
|
||||
expect(runNormalize('zstd')).toBe('identity');
|
||||
expect(runNormalize('zstd;q=1.0')).toBe('identity');
|
||||
});
|
||||
|
||||
it('leaves the wildcard token alone', () => {
|
||||
expect(runNormalize('*')).toBe('*');
|
||||
});
|
||||
|
||||
it('does not strip tokens that merely contain "zstd" as a substring', () => {
|
||||
// Should not match the zstd-only regex even though it starts with "zstd-".
|
||||
expect(runNormalize('zstd-future, gzip')).toBe('zstd-future, gzip');
|
||||
});
|
||||
});
|
||||
|
||||
describe('normalizeAcceptEncoding (integration with compression)', () => {
|
||||
// supertest (via superagent) auto-decompresses gzip/br response bodies, so
|
||||
// these tests assert the Content-Encoding response header and that the
|
||||
// decoded body matches the original payload. The header is what the browser
|
||||
// checks to decide whether to decompress; the F-3 bug was specifically that
|
||||
// it was absent.
|
||||
|
||||
it('sets Content-Encoding (br or gzip) when zstd, gzip, br is offered', async () => {
|
||||
const res = await request(makeApp())
|
||||
.get('/text')
|
||||
.set('Accept-Encoding', 'zstd, gzip, br');
|
||||
expect(res.status).toBe(200);
|
||||
expect(['br', 'gzip']).toContain(res.headers['content-encoding']);
|
||||
expect(res.text.startsWith('abcdefghij')).toBe(true);
|
||||
});
|
||||
|
||||
it('still serves gzip when only gzip is offered (regression baseline)', async () => {
|
||||
const res = await request(makeApp())
|
||||
.get('/text')
|
||||
.set('Accept-Encoding', 'gzip');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.headers['content-encoding']).toBe('gzip');
|
||||
expect(res.text.startsWith('abcdefghij')).toBe(true);
|
||||
});
|
||||
|
||||
it('omits Content-Encoding when only zstd is offered (identity fallback)', async () => {
|
||||
const res = await request(makeApp())
|
||||
.get('/text')
|
||||
.set('Accept-Encoding', 'zstd');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.headers['content-encoding']).toBeUndefined();
|
||||
expect(res.text.startsWith('abcdefghij')).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,7 @@
|
||||
* Regression guard for `createRemoteProxyMiddleware` mount order.
|
||||
*
|
||||
* `index.ts` mounts every local `/api/<group>` router before the remote proxy
|
||||
* at step 13 of the canonical middleware order. A remote-`nodeId` request
|
||||
* at step 15 of the canonical middleware order. A remote-`nodeId` request
|
||||
* must short-circuit into the proxy rather than match a local router. If the
|
||||
* order were reversed, a GET `/api/stacks` for a remote node would return the
|
||||
* control instance's local stack list instead of the remote's.
|
||||
|
||||
+29
-22
@@ -6,35 +6,37 @@ import helmet from 'helmet';
|
||||
import { globalApiLimiter, pollingLimiter } from './middleware/rateLimiters';
|
||||
import { conditionalJsonParser } from './middleware/jsonParser';
|
||||
import { nodeContextMiddleware } from './middleware/nodeContext';
|
||||
import { normalizeAcceptEncoding } from './middleware/normalizeAcceptEncoding';
|
||||
import './types/express';
|
||||
|
||||
/**
|
||||
* Build an Express app with the full middleware pipeline installed.
|
||||
*
|
||||
* Canonical middleware order (17 steps). Do not reorder without re-running the
|
||||
* Canonical middleware order (18 steps). Do not reorder without re-running the
|
||||
* regression checklist in `docs/internal/architecture/middleware-order.md`.
|
||||
*
|
||||
* 1. trust proxy
|
||||
* 2. helmet
|
||||
* 3. cors
|
||||
* 4. compression
|
||||
* 5. cookieParser
|
||||
* 6. globalApiLimiter (at /api)
|
||||
* 7. pollingLimiter (at /api)
|
||||
* 8. conditionalJsonParser
|
||||
* 9. nodeContextMiddleware
|
||||
* 10. authGate (at /api) -- registered in index.ts
|
||||
* 11. auditLog (at /api) -- registered in index.ts
|
||||
* 12. enforceApiTokenScope (at /api) -- registered in index.ts
|
||||
* 13. hubOnlyGuard (at /api) -- middleware/hubOnlyGuard.ts, registered in index.ts
|
||||
* 14. createRemoteProxyMiddleware -- proxy/remoteNodeProxy.ts, registered in index.ts
|
||||
* 15. routes -- registered in index.ts from routes/*
|
||||
* 16. static serving + SPA fallback -- registered in index.ts
|
||||
* 17. errorHandler -- registered in index.ts
|
||||
* 4. normalizeAcceptEncoding
|
||||
* 5. compression
|
||||
* 6. cookieParser
|
||||
* 7. globalApiLimiter (at /api)
|
||||
* 8. pollingLimiter (at /api)
|
||||
* 9. conditionalJsonParser
|
||||
* 10. nodeContextMiddleware
|
||||
* 11. authGate (at /api) -- registered in index.ts
|
||||
* 12. auditLog (at /api) -- registered in index.ts
|
||||
* 13. enforceApiTokenScope (at /api) -- registered in index.ts
|
||||
* 14. hubOnlyGuard (at /api) -- middleware/hubOnlyGuard.ts, registered in index.ts
|
||||
* 15. createRemoteProxyMiddleware -- proxy/remoteNodeProxy.ts, registered in index.ts
|
||||
* 16. routes -- registered in index.ts from routes/*
|
||||
* 17. static serving + SPA fallback -- registered in index.ts
|
||||
* 18. errorHandler -- registered in index.ts
|
||||
*
|
||||
* Steps 10 to 13 and 15 must run after the public auth routers (meta, auth,
|
||||
* Steps 11 to 14 and 16 must run after the public auth routers (meta, auth,
|
||||
* mfa, sso) are registered so those routes stay reachable without a session
|
||||
* cookie. index.ts mounts those public routers before step 10 to preserve
|
||||
* cookie. index.ts mounts those public routers before step 11 to preserve
|
||||
* that invariant.
|
||||
*/
|
||||
export function createApp(): express.Express {
|
||||
@@ -99,7 +101,12 @@ export function createApp(): express.Express {
|
||||
credentials: true,
|
||||
}));
|
||||
|
||||
// 4. Compression. SSE streams (Content-Type: text/event-stream) MUST NOT be
|
||||
// 4. Drop unknown Accept-Encoding tokens (e.g. `zstd` from Chromium 123+)
|
||||
// before compression negotiates. See middleware/normalizeAcceptEncoding.ts
|
||||
// for the symptom this prevents.
|
||||
app.use(normalizeAcceptEncoding);
|
||||
|
||||
// 5. Compression. SSE streams (Content-Type: text/event-stream) MUST NOT be
|
||||
// compressed because compression buffers output and would delay event delivery
|
||||
// until a flush, breaking live log and status streams.
|
||||
app.use(compression({
|
||||
@@ -112,18 +119,18 @@ export function createApp(): express.Express {
|
||||
},
|
||||
}));
|
||||
|
||||
// 5. Cookie parser must run before the rate limiters so the hybrid key
|
||||
// 6. Cookie parser must run before the rate limiters so the hybrid key
|
||||
// generator can read req.cookies for per-user rate limit bucketing.
|
||||
app.use(cookieParser());
|
||||
|
||||
// 6-7. Tiered rate limiting (see middleware/rateLimiters.ts for the model).
|
||||
// 7-8. Tiered rate limiting (see middleware/rateLimiters.ts for the model).
|
||||
app.use('/api/', globalApiLimiter);
|
||||
app.use('/api/', pollingLimiter);
|
||||
|
||||
// 8. Parse JSON on local requests; preserve the raw stream for remote proxy.
|
||||
// 9. Parse JSON on local requests; preserve the raw stream for remote proxy.
|
||||
app.use(conditionalJsonParser);
|
||||
|
||||
// 9. Resolve req.nodeId and short-circuit requests to deleted nodes.
|
||||
// 10. Resolve req.nodeId and short-circuit requests to deleted nodes.
|
||||
app.use(nodeContextMiddleware);
|
||||
|
||||
return app;
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
import type { RequestHandler } from 'express';
|
||||
|
||||
// compression@1.8.1's encoding negotiation (via negotiator@1.0.0) silently
|
||||
// breaks when Accept-Encoding carries an unknown token such as `zstd`, which
|
||||
// Chromium 123+ sends by default and which Playwright's bundled Chromium sends
|
||||
// even over plain HTTP. In that path the response body is still compressed
|
||||
// (brotli) but the Content-Encoding response header is never set, so the
|
||||
// browser decode-fails with ERR_CONTENT_DECODING_FAILED and the Sencho UI
|
||||
// renders blank. Drop the offending token before compression sees it so the
|
||||
// negotiator falls back to a supported encoding and the header is written.
|
||||
export const normalizeAcceptEncoding: RequestHandler = (req, _res, next) => {
|
||||
const raw = req.headers['accept-encoding'];
|
||||
if (typeof raw !== 'string' || !/\bzstd\b/i.test(raw)) {
|
||||
next();
|
||||
return;
|
||||
}
|
||||
const filtered = raw
|
||||
.split(',')
|
||||
.map((s) => s.trim())
|
||||
.filter((s) => s.length > 0 && !/^zstd(\s*;\s*q\s*=.*)?$/i.test(s))
|
||||
.join(', ');
|
||||
req.headers['accept-encoding'] = filtered || 'identity';
|
||||
next();
|
||||
};
|
||||
Reference in New Issue
Block a user