fix(#346 review F1-F4): no 206-compress + Vary + precompress VAD + cache test
- F1 [HIGH — data corruption]: @fastify/compress was compressing 206/Range attachment responses while Content-Range still described the RAW offsets, so a resuming client (curl -C -, download managers) appended encoded bytes as raw → corrupted file. sendFileResponse now sets the request header `x-no-compression` (the documented @fastify/compress opt-out — its onSend skips when the request carries it; the reviewer's `Content-Encoding: identity` does NOT work because compress explicitly excludes `identity` and overwrites it). This opts the whole download route (both 200 full-file and 206 range) out of on-the-fly compression — correct, since attachment bytes are final and mostly binary. - F2: static responses now emit `Vary: Accept-Encoding` (the preCompressed content-negotiated /assets/* were `immutable` without Vary → shared-cache could serve a brotli variant to an identity/gzip-only client). - F3: vite compression `include` extended to .wasm/.onnx so the VAD binaries (~26MB .wasm, ~2.3MB .onnx under public/vad) are precompressed at build (.br emitted) instead of runtime-brotli'd on every request. (include REPLACES the plugin default, so the default js/css/json/html set is re-listed.) - F4: extracted the cache classification into a pure `resolveStaticAssetHeaders` + static.module.spec.ts (3 tests: /assets/* immutable+Vary, index.html no-store, non-hashed not-immutable). Gate: server tsc 0 (deps present), static.module.spec 3/3, client build emits .wasm.br/.onnx.br, frozen install 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -60,6 +60,14 @@ export default defineConfig(({ mode }) => {
|
||||
// precompressed copy (see @fastify/static preCompressed in static.module.ts).
|
||||
compression({
|
||||
algorithms: ["brotliCompress", "gzip"],
|
||||
// vite-plugin-compression2's default `include` only covers text-ish
|
||||
// bundle output (js/mjs/json/css/html/svg/…). Extend it with the large
|
||||
// VAD binaries copied from public/vad (.wasm ~26MB, .onnx ~2.3MB) so
|
||||
// they are brotli/gzip'd once at build time and served via
|
||||
// @fastify/static preCompressed — otherwise @fastify/compress would
|
||||
// re-brotli them on EVERY request. The default types are repeated here
|
||||
// because setting `include` replaces (does not extend) the default.
|
||||
include: /\.(html|xml|css|json|js|mjs|svg|yaml|yml|toml|wasm|onnx)$/,
|
||||
// index.html is rewritten at server boot (window.CONFIG injection); a
|
||||
// precompressed copy would go stale — NEVER precompress it.
|
||||
exclude: [/index\.html$/],
|
||||
|
||||
@@ -474,6 +474,19 @@ export class AttachmentController {
|
||||
const fileSize = Number(attachment.fileSize);
|
||||
const rangeHeader = req.headers.range;
|
||||
|
||||
// Opt this download route out of the global @fastify/compress hook.
|
||||
// Attachment bytes are final and mostly binary, so on-the-fly compression
|
||||
// only burns CPU — and on the 206/Range branch it is actively corrupting:
|
||||
// compress decides purely by Content-Type, so for a compressible mime
|
||||
// (application/octet-stream fallback, image/svg+xml, text/*) it would gzip
|
||||
// the byte slice and drop Content-Length while Content-Range still
|
||||
// describes the RAW offsets and the status stays 206. A resuming client
|
||||
// (`curl -C -`, download managers) then appends the encoded bytes as if
|
||||
// raw and ends up with a broken file. @fastify/compress skips whenever the
|
||||
// request carries `x-no-compression` (see its onSend hook), so setting it
|
||||
// here covers both the 200 (full file) and 206 (range) responses.
|
||||
req.headers['x-no-compression'] = 'true';
|
||||
|
||||
res.header('Accept-Ranges', 'bytes');
|
||||
res.header(
|
||||
'Content-Security-Policy',
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
import { resolveStaticAssetHeaders } from './static.module';
|
||||
|
||||
// Unit tests for the static-asset cache classifier extracted from the
|
||||
// @fastify/static setHeaders callback (precedent: sandbox.controller.spec.ts).
|
||||
describe('resolveStaticAssetHeaders', () => {
|
||||
it('marks a content-hashed /assets/ file immutable and sets Vary', () => {
|
||||
const headers = resolveStaticAssetHeaders(
|
||||
'/app/apps/client/dist/assets/index-a1b2c3.js',
|
||||
);
|
||||
expect(headers['cache-control']).toBe(
|
||||
'public, max-age=31536000, immutable',
|
||||
);
|
||||
expect(headers['vary']).toBe('Accept-Encoding');
|
||||
});
|
||||
|
||||
it('makes index.html always revalidate (never immutable)', () => {
|
||||
const headers = resolveStaticAssetHeaders(
|
||||
'/app/apps/client/dist/index.html',
|
||||
);
|
||||
expect(headers['cache-control']).toBe(
|
||||
'no-cache, no-store, must-revalidate',
|
||||
);
|
||||
expect(headers['vary']).toBe('Accept-Encoding');
|
||||
});
|
||||
|
||||
it('does NOT mark a non-hashed asset immutable but still sets Vary', () => {
|
||||
const headers = resolveStaticAssetHeaders(
|
||||
'/app/apps/client/dist/locales/en.json',
|
||||
);
|
||||
// No immutable cache-control — this path keeps @fastify/static's default
|
||||
// etag/last-modified revalidation.
|
||||
expect(headers['cache-control']).toBeUndefined();
|
||||
expect(headers['vary']).toBe('Accept-Encoding');
|
||||
});
|
||||
});
|
||||
@@ -5,6 +5,46 @@ import * as fs from 'node:fs';
|
||||
import fastifyStatic from '@fastify/static';
|
||||
import { EnvironmentService } from '../environment/environment.service';
|
||||
|
||||
/**
|
||||
* Resolve the response headers for a statically served client asset.
|
||||
*
|
||||
* Extracted from the @fastify/static `setHeaders` callback so the cache
|
||||
* classification stays a pure, unit-testable function (see
|
||||
* static.module.spec.ts).
|
||||
*
|
||||
* `Vary: Accept-Encoding` is emitted for every static response because
|
||||
* @fastify/static negotiates a precompressed .br/.gz neighbour by the client's
|
||||
* Accept-Encoding but does NOT set Vary itself. Without it a shared/proxy cache
|
||||
* keyed on the URL alone could store the brotli variant and later serve it to a
|
||||
* client that only sent `Accept-Encoding: identity`/gzip → an undecodable body.
|
||||
* This matters most for the immutable /assets/ files, which proxies may keep
|
||||
* for a year.
|
||||
*/
|
||||
export function resolveStaticAssetHeaders(
|
||||
filePath: string,
|
||||
): Record<string, string> {
|
||||
const headers: Record<string, string> = { vary: 'Accept-Encoding' };
|
||||
|
||||
// Content-hashed files under /assets/ never change for a given URL, so they
|
||||
// can be cached forever and skip revalidation entirely.
|
||||
if (filePath.includes('/assets/')) {
|
||||
headers['cache-control'] = 'public, max-age=31536000, immutable';
|
||||
return headers;
|
||||
}
|
||||
|
||||
// index.html is rewritten at boot (window.CONFIG injection) and on every
|
||||
// deploy — it must be revalidated on every load.
|
||||
if (filePath.endsWith('index.html')) {
|
||||
headers['cache-control'] = 'no-cache, no-store, must-revalidate';
|
||||
return headers;
|
||||
}
|
||||
|
||||
// Everything else (locales, vad, icons, manifest) is NOT content-hashed and
|
||||
// changes between deploys, so it keeps @fastify/static's default
|
||||
// etag/last-modified revalidation — do NOT mark it immutable.
|
||||
return headers;
|
||||
}
|
||||
|
||||
@Module({})
|
||||
export class StaticModule implements OnModuleInit {
|
||||
constructor(
|
||||
@@ -76,27 +116,11 @@ export class StaticModule implements OnModuleInit {
|
||||
// (see vite-plugin-compression2 in apps/client/vite.config.ts).
|
||||
preCompressed: true,
|
||||
setHeaders: (res, filePath) => {
|
||||
// Content-hashed files under /assets/ never change for a given URL,
|
||||
// so they can be cached forever and skip revalidation entirely.
|
||||
if (filePath.includes('/assets/')) {
|
||||
res.setHeader(
|
||||
'cache-control',
|
||||
'public, max-age=31536000, immutable',
|
||||
);
|
||||
return;
|
||||
for (const [name, value] of Object.entries(
|
||||
resolveStaticAssetHeaders(filePath),
|
||||
)) {
|
||||
res.setHeader(name, value);
|
||||
}
|
||||
// index.html is rewritten at boot (window.CONFIG injection) and on
|
||||
// every deploy — it must be revalidated on every load.
|
||||
if (filePath.endsWith('index.html')) {
|
||||
res.setHeader(
|
||||
'cache-control',
|
||||
'no-cache, no-store, must-revalidate',
|
||||
);
|
||||
return;
|
||||
}
|
||||
// Everything else (locales, vad, icons, manifest) is NOT content-hashed
|
||||
// and changes between deploys, so it keeps @fastify/static's default
|
||||
// etag/last-modified revalidation — do NOT mark it immutable.
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user