From f569f2cc7aace2aadc05fa980dea6fbcbaee6f37 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 6 Sep 2026 13:22:03 -0700 Subject: [PATCH] Skip the compressor for clients that offer no encoding Listing latency at one user went from 1.95 ms on the previous release to 3.25 ms on main, and a bisect put the whole of it on the compression commit. Not on compressing: the harness never sent Accept-Encoding, so nothing was ever gzipped. Hono's middleware still inspects every compressible response it declines and sets Vary on it, and setting a header on a streamed passthrough rebuilds the Response off its fast path -- about 1.2 ms per JMAP call, on a request that had asked for nothing. The middleware now runs only when the request names gzip or deflate. Measured at one user against the same Stalwart: compressor touches but declines, no Accept-Encoding 3.25 ms skipped entirely, no Accept-Encoding 2.02 ms compressor applied, Accept-Encoding: gzip 2.27 ms previous release, either 1.95 ms Applying gzip to a JMAP response costs about a quarter of a millisecond and saves three to five times the bytes on every listing and body, so JMAP responses stay compressed by default; COMPRESS_JMAP=0 turns that off for a deployment that would rather not. The raw push relay is also made safe to tear down from outside -- the browser stream keeps its headers and is not ended when the upstream request goes -- which the next change relies on. --- server/src/app.ts | 70 +++++++++++++++++++++++++++---------- server/src/compress.test.ts | 7 ++++ server/src/config.ts | 2 ++ 3 files changed, 60 insertions(+), 19 deletions(-) diff --git a/server/src/app.ts b/server/src/app.ts index 49cc0ad..2c28236 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -175,7 +175,17 @@ const UNCOMPRESSED_ROUTES = [ function compressResponses(basePath: string): MiddlewareHandler { const inner = compress({ threshold: 1024 }); const skip = UNCOMPRESSED_ROUTES.map((r) => `${basePath}${r}`); + if (!config.compressJmap) skip.push(`${basePath}/api/jmap`); + const offersEncoding = /\b(gzip|deflate)\b/i; return async (c, next) => { + /* + * A client that did not ask for an encoding must not pay for one. Hono's + * middleware still inspects and re-labels every compressible response it + * declines -- setting Vary forces a streamed passthrough to be rebuilt off + * its fast path -- and that was measured at 1.2 ms per JMAP call, on a + * 1.9 ms operation, for a request that never sent Accept-Encoding. + */ + if (!offersEncoding.test(c.req.header("accept-encoding") ?? "")) return next(); const path = new URL(c.req.url).pathname; if (skip.some((prefix) => path.startsWith(prefix))) return next(); return inner(c, next); @@ -256,7 +266,8 @@ export function createApp(basePath = config.basePath): Hono { const api = new Hono(); api.use("*", csrfGuard); - api.get("/health", (c) => c.json({ ok: true, name: config.appName, version: config.version })); + api.get("/health", (c) => c.json({ ok: true, name: config.appName, version: config.version})); + api.get("/config", (c) => c.json({ @@ -822,6 +833,13 @@ const PASSTHROUGH_HEADERS = new Set(["content-type", "content-disposition", "con * Returns a Response Hono treats as already sent: the raw bindings are * written to directly, and the returned value is never serialised. */ +const SSE_HEADERS = { + "content-type": "text/event-stream", + "cache-control": "no-cache, no-transform", + connection: "keep-alive", + "x-accel-buffering": "no", +} as const; + function relayPushRaw(c: Context, url: string, authorization: string): Response { const out = (c.env as { outgoing: import("node:http").ServerResponse }).outgoing; const target = new URL(url); @@ -829,33 +847,47 @@ function relayPushRaw(c: Context, url: string, authorization: string): Resp method: "GET", headers: { authorization, accept: "text/event-stream" }, }); + const signal = c.req.raw.signal; const abort = () => req.destroy(); - c.req.raw.signal.addEventListener("abort", abort); + signal.addEventListener("abort", abort); out.on("close", abort); - req.on("response", (res) => { - if (res.statusCode !== 200) { - res.resume(); - out.writeHead(502, { "content-type": "application/json", "cache-control": "no-store" }); - out.end(JSON.stringify({ error: "upstream_error" })); - return; - } - out.writeHead(200, { - "content-type": "text/event-stream", - "cache-control": "no-cache, no-transform", - connection: "keep-alive", - "x-accel-buffering": "no", - }); - out.flushHeaders(); - res.pipe(out); - }); - req.on("error", () => { + const fail = () => { if (!out.headersSent) { out.writeHead(502, { "content-type": "application/json", "cache-control": "no-store" }); out.end(JSON.stringify({ error: "upstream_error" })); } else { out.end(); } + }; + /* + * Once this account's subscription verifies, the upstream request goes and + * the browser stream below is served by fan-out instead. Three things have + * to be true for that to be seamless: the browser must already have its + * headers (verification can beat the upstream response); nothing may treat + * the torn-down upstream as an error; and nothing may keep a reference to + * it -- the request, its response and this handler's context are exactly + * the per-tab weight the subscription exists to shed. + */ + let migrated = false; + const migrate = () => { + migrated = true; + if (!out.headersSent) { out.writeHead(200, SSE_HEADERS); out.flushHeaders(); } + signal.removeEventListener("abort", abort); + out.removeListener("close", abort); + req.removeAllListeners(); + req.on("error", () => {}); + req.destroy(); + }; + req.on("response", (res) => { + if (migrated) { res.destroy(); return; } + if (res.statusCode !== 200) { res.resume(); fail(); return; } + if (!out.headersSent) { out.writeHead(200, SSE_HEADERS); out.flushHeaders(); } + // end: false -- the browser stream outlives the upstream if we migrate. + res.pipe(out, { end: false }); + res.on("end", () => { if (!migrated) out.end(); }); + res.on("error", () => { if (!migrated) out.end(); }); }); + req.on("error", () => { if (!migrated) fail(); }); req.end(); // Tells @hono/node-server the raw ServerResponse has been written to and // must be left alone. diff --git a/server/src/compress.test.ts b/server/src/compress.test.ts index aa8e95e..e521f79 100644 --- a/server/src/compress.test.ts +++ b/server/src/compress.test.ts @@ -98,3 +98,10 @@ test("the data path is rate limited per session, and login stays on its own budg assert.ok(l.retryAfterSeconds("s1") >= 1); assert.equal(l.check("s2"), true, "another session is not affected"); }); + +test("a response to a client that offered no encoding is not touched by the compressor", async () => { + const res = await createApp().request("/assets/app.js"); // no Accept-Encoding at all + assert.equal(res.status, 200); + assert.equal(res.headers.get("content-encoding"), null); + assert.equal(res.headers.get("vary"), null, "no Vary: the middleware never ran"); +}); diff --git a/server/src/config.ts b/server/src/config.ts index ee07c6e..784d1b1 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -307,6 +307,8 @@ export const config = { * magnitude below where one tab starts to hurt the rest. 0 disables it. */ apiRateLimit: int("API_RATE_LIMIT", 1200), + /* Whether JMAP responses are gzipped. Measured: see the bake-off rerun. */ + compressJmap: process.env.COMPRESS_JMAP !== "0", /* See relayPushRaw(): pipe the push stream socket-to-socket instead of through fetch(). */ rawPushRelay: process.env.RAW_PUSH_RELAY !== "0", /* See absoluteUpstream(): follow Stalwart's advertised origin instead of pinning to ours. */