diff --git a/nginx.example.conf b/nginx.example.conf index 79984c0..0bccab7 100644 --- a/nginx.example.conf +++ b/nginx.example.conf @@ -6,17 +6,22 @@ server { client_max_body_size 60m; - # ihasmail serves its bundle uncompressed and leaves this to the proxy, so - # without these directives the browser downloads about 915 KB where 307 KB - # would do. text/event-stream is deliberately absent from gzip_types: the - # push stream must not be compressed or buffered. + # Compression. The bundle is the bulk of first load -- about 933 KB + # uncompressed against 311 KB gzipped -- and nginx passes through anything + # the upstream already encoded rather than re-encoding it, so this is + # correct whether or not ihasmail compresses on its own. + # + # text/event-stream is deliberately absent from gzip_types: the push stream + # must not be compressed or buffered, which is also why proxy_buffering is + # off below. gzip on; gzip_vary on; gzip_proxied any; gzip_comp_level 5; gzip_min_length 1024; - # ihasmail serves scripts as text/javascript, so listing only - # application/javascript silently leaves the largest asset uncompressed. + # text/javascript is listed explicitly: ihasmail serves scripts with that + # type rather than application/javascript, so a conventional gzip_types + # list compresses the stylesheet and leaves the largest asset alone. gzip_types application/javascript application/json diff --git a/server/src/app.ts b/server/src/app.ts index ed0156e..f543a20 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -1,6 +1,7 @@ import { Hono } from "hono"; import type { Context, MiddlewareHandler } from "hono"; import { getCookie, setCookie, deleteCookie } from "hono/cookie"; +import { compress } from "hono/compress"; import { getConnInfo } from "@hono/node-server/conninfo"; import { config } from "./config.js"; import { SessionStore, type SessionBackend, type LiveSession } from "./sessions.js"; @@ -109,6 +110,62 @@ const securityHeaders: MiddlewareHandler = async (c, next) => { }; /** CSRF: require our custom header on all API calls; reject cross-site fetches. */ +/** + * Routes that forward somebody else's bytes rather than producing our own. + * + * Compression is right for the app shell, the bundle and our JSON; it is not + * worth the risk on the proxy paths. Those carry a content-length copied from + * upstream under the rules in `forwardedContentLength`, and issue #76 was a + * silent truncation caused by exactly that header disagreeing with the body. + * Re-encoding them would be safe in principle -- the length is dropped and the + * response goes out chunked -- but the payloads are attachments, images and + * calendar data that are already compressed or too small to matter, so there + * is nothing to win and a scar to respect. + * + * `/api/events` needs no entry here: Hono skips `text/event-stream` by content + * type. It is listed anyway, because a future change to that route's type + * should not quietly start buffering the push stream. + */ +const UNCOMPRESSED_ROUTES = [ + "/api/blob/", + "/api/image", + "/api/ics", + "/api/upload/", + "/api/events", + /* + * The liveness probe, which is small enough that gzip makes it bigger: 53 + * bytes becomes 73. Hono's size threshold cannot catch this on its own, + * because it only applies when the response carries a content-length and + * `c.json()` does not set one. Every other JSON route is left compressed -- + * a JMAP response can run to hundreds of kilobytes and its length is just as + * unknown -- so this is the one place worth naming. + */ + "/api/health", +]; + +/** + * gzip for what we generate. + * + * The bundle ships uncompressed otherwise: 915 KB on the wire where 307 KB + * would do, on every first load. `Caddyfile.example` and + * `nginx.example.conf` both compress at the proxy, but that only helps the + * deployments that use them, and the default should not depend on reading the + * examples. + * + * Hono's middleware declines anything already carrying `Content-Encoding` or + * `Transfer-Encoding`, so a proxy compressing in front of us wins and we do + * not double-encode. + */ +function compressResponses(basePath: string): MiddlewareHandler { + const inner = compress({ threshold: 1024 }); + const skip = UNCOMPRESSED_ROUTES.map((r) => `${basePath}${r}`); + return async (c, next) => { + const path = new URL(c.req.url).pathname; + if (skip.some((prefix) => path.startsWith(prefix))) return next(); + return inner(c, next); + }; +} + const csrfGuard: MiddlewareHandler = async (c, next) => { const site = c.req.header("sec-fetch-site"); if (site && site !== "same-origin" && site !== "none") { @@ -178,6 +235,7 @@ function upstreamFailure(c: Context, err: unknown) { export function createApp(basePath = config.basePath): Hono { const app = new Hono(); app.use("*", securityHeaders); + app.use("*", compressResponses(basePath)); const api = new Hono(); api.use("*", csrfGuard); diff --git a/server/src/compress.test.ts b/server/src/compress.test.ts new file mode 100644 index 0000000..49da5e6 --- /dev/null +++ b/server/src/compress.test.ts @@ -0,0 +1,76 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, writeFileSync, mkdirSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +/* + * A static root of our own, built before the app is imported. + * + * CI runs `npm test` before `npm run build`, so `web/dist` does not exist when + * these run: pointing at it would serve the "web build not found" fallback, + * which is short, plain text and rightly uncompressed. That failure looked + * exactly like compression being broken. + */ +const root = mkdtempSync(join(tmpdir(), "ihasmail-compress-")); +mkdirSync(join(root, "assets")); +const script = `/* ${"x".repeat(40_000)} */\n`; +writeFileSync(join(root, "assets", "app.js"), script); +writeFileSync(join(root, "index.html"), `t${"

hello

".repeat(400)}`); + +process.env.STATIC_DIR = root; +process.env.STALWART_URL = "http://127.0.0.1:1"; +const { createApp } = await import("./app.js"); + +test("an asset is gzipped when the client asks for it", async () => { + const res = await createApp().request("/assets/app.js", { headers: { "accept-encoding": "gzip" } }); + assert.equal(res.status, 200); + assert.equal(res.headers.get("content-encoding"), "gzip"); + assert.match(res.headers.get("vary") ?? "", /accept-encoding/i); +}); + +test("a client that does not ask for gzip does not get it", async () => { + const res = await createApp().request("/assets/app.js", { headers: { "accept-encoding": "identity" } }); + assert.equal(res.status, 200); + assert.equal(res.headers.get("content-encoding"), null); +}); + +test("gzip actually makes the asset smaller", async () => { + const plain = await (await createApp().request("/assets/app.js", { headers: { "accept-encoding": "identity" } })).arrayBuffer(); + const gz = await (await createApp().request("/assets/app.js", { headers: { "accept-encoding": "gzip" } })).arrayBuffer(); + assert.ok(gz.byteLength < plain.byteLength / 2, `${gz.byteLength} should be well under ${plain.byteLength}`); +}); + +test("a gzipped response decodes to the bytes we would have sent plain", async () => { + const plain = await (await createApp().request("/assets/app.js", { headers: { "accept-encoding": "identity" } })).arrayBuffer(); + const res = await createApp().request("/assets/app.js", { headers: { "accept-encoding": "gzip" } }); + const decoded = await new Response(res.body!.pipeThrough(new DecompressionStream("gzip"))).arrayBuffer(); + assert.deepEqual(Buffer.from(decoded), Buffer.from(plain)); +}); + +test("the app shell is gzipped", async () => { + const res = await createApp().request("/", { headers: { "accept-encoding": "gzip" } }); + assert.equal(res.status, 200); + assert.equal(res.headers.get("content-encoding"), "gzip"); +}); + +test("proxy routes that forward upstream bytes are never compressed", async () => { + // Unauthenticated, so these stop at 401 -- enough to prove the middleware + // declines the path, which is what issue #76 was about. + const app = createApp(); + for (const path of ["/api/blob/a/b/c.pdf", "/api/image?url=https://example.com/x.png", "/api/ics?url=https://example.com/x.ics"]) { + const res = await app.request(path, { headers: { "accept-encoding": "gzip" } }); + assert.equal(res.headers.get("content-encoding"), null, `${path} must not be compressed`); + } +}); + +test("the push stream is never compressed", async () => { + const res = await createApp().request("/api/events", { headers: { "accept-encoding": "gzip" } }); + assert.equal(res.headers.get("content-encoding"), null); +}); + +test("the liveness probe is not compressed, since gzip would make it bigger", async () => { + const res = await createApp().request("/api/health", { headers: { "accept-encoding": "gzip" } }); + assert.equal(res.status, 200); + assert.equal(res.headers.get("content-encoding"), null); +});