From b72b1adc6329f460cd0423a23fd7a46b71a19869 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 17 Oct 2026 23:53:11 +0900 Subject: [PATCH 1/4] Use srvx for the Node.js HTTP server Pass forwarded requests through without buffering their bodies, keeping Host intact for Fedify signature verification. Retain native Request and Response constructors and the existing static-file middleware. Coordinate partial startup and HTTP/worker shutdown under the existing deadline, including keep-alive connections that finish after close begins. Cover the transport boundary and SQL cancellation with real HTTP tests. https://github.com/fedify-dev/hollo/issues/672 Assisted-by: Codex:gpt-6.1-sol Assisted-by: Claude Code:claude-fable-5-1 --- CHANGES.md | 12 + bin/server.ts | 184 ++++---- changes.d/streaming-http-server.md | 14 + docs/src/content/docs/install/env.mdx | 6 + docs/src/content/docs/ja/install/env.mdx | 5 + docs/src/content/docs/ko/install/env.mdx | 5 + docs/src/content/docs/zh-cn/install/env.mdx | 4 + docs/src/content/docs/zh-tw/install/env.mdx | 4 + package.json | 2 +- pnpm-lock.yaml | 47 +- pnpm-workspace.yaml | 1 + src/http-server.db.test.ts | 72 +++ src/http-server.test.ts | 484 ++++++++++++++++++++ src/http-server.ts | 67 +++ src/server-lifecycle.test.ts | 97 ++++ src/server-lifecycle.ts | 41 ++ 16 files changed, 930 insertions(+), 115 deletions(-) create mode 100644 changes.d/streaming-http-server.md create mode 100644 src/http-server.db.test.ts create mode 100644 src/http-server.test.ts create mode 100644 src/http-server.ts create mode 100644 src/server-lifecycle.test.ts create mode 100644 src/server-lifecycle.ts diff --git a/CHANGES.md b/CHANGES.md index 83546760..d1545522 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -146,6 +146,16 @@ To be released. 14 adds post-quantum ML-DSA passkey support on runtimes that provide the algorithms. \[[GHSA-2g3p-m8c9-hhwh], [GHSA-j3h4-m3m2-7p7j]] + - Changed the HTTP server to avoid buffering entire uploads and incoming + activities before handling them behind a reverse proxy. Shutdown now waits + for active HTTP requests alongside background workers within the existing + 30-second deadline. + + Proxy mode now uses the first nonempty forwarded host and protocol in a + comma-separated list, and ignores invalid forwarded hosts in favor of the + original `Host` header. Trusted proxies must overwrite client-supplied + forwarded headers. [[#672], [#673]] + [FEP-044f]: https://w3id.org/fep/044f [FEP-c0e0]: https://w3id.org/fep/c0e0 [FEP-f228]: https://w3id.org/fep/f228 @@ -175,6 +185,8 @@ To be released. [#661]: https://github.com/fedify-dev/hollo/issues/661 [#663]: https://github.com/fedify-dev/hollo/pull/663 [#664]: https://github.com/fedify-dev/hollo/pull/664 +[#672]: https://github.com/fedify-dev/hollo/issues/672 +[#673]: https://github.com/fedify-dev/hollo/pull/673 Version 0.9.24 diff --git a/bin/server.ts b/bin/server.ts index d713c62d..64142e22 100644 --- a/bin/server.ts +++ b/bin/server.ts @@ -1,12 +1,9 @@ import { isIP } from "node:net"; -import type { ServerType } from "@hono/node-server"; -import { serve } from "@hono/node-server"; -import { behindProxy } from "x-forwarded-fetch"; - import "../src/logging"; import { checkHandleHostConsistency } from "../src/handle-host-check"; import { configureSentry } from "../src/sentry"; +import { createServerLifecycle } from "../src/server-lifecycle"; // oxlint-disable-next-line typescript/dot-notation configureSentry(process.env["SENTRY_DSN"]); @@ -42,95 +39,106 @@ if (!["all", "web", "worker"].includes(NODE_TYPE)) { process.exit(1); } -// Warn if the configured HANDLE_HOST disagrees with an existing account. -await checkHandleHostConsistency(); - -let webServer: ServerType | undefined; - -// Start web server if running as web or all node -if (NODE_TYPE === "web" || NODE_TYPE === "all") { - const { default: app } = await import("../src/index"); - webServer = serve( - { - fetch: BEHIND_PROXY - ? behindProxy(app.fetch.bind(app)) - : app.fetch.bind(app), +const lifecycle = createServerLifecycle([ + async () => { + // Warn if HANDLE_HOST disagrees with an existing account. + await checkHandleHostConsistency(); + }, + async (registerStop) => { + if (NODE_TYPE === "worker") return; + // Initialize Sentry before loading the HTTP adapter and application. + const { createHttpServer } = await import("../src/http-server"); + const { default: app } = await import("../src/index"); + if (lifecycle.stopping) return; + const server = createHttpServer(app.fetch.bind(app), { port: PORT, hostname: BIND, - }, - (info) => { - let host = info.address; - // We override it here to show localhost instead of what it resolves to: - if (BIND === "localhost") { - host = "localhost"; - } else if (info.family === "IPv6") { - host = `[${info.address}]`; + behindProxy: BEHIND_PROXY, + }); + registerStop(() => server.close()); + await server.ready(); + if (lifecycle.stopping) return; + const url = new URL(server.url!); + if (BIND === "localhost") url.hostname = "localhost"; + console.log(`Listening on ${url.href}`); + }, + async (registerStop) => { + if (NODE_TYPE === "web") return; + const [ + { federation }, + { backgroundJobs, replyScrapes, pollNotifications }, + { itemLeases }, + { WEB_ORIGIN }, + ] = await Promise.all([ + import("../src/federation"), + import("../src/federation/federation"), + import("../src/background/lease"), + import("../src/env"), + ]); + if (lifecycle.stopping) return; + + const controller = new AbortController(); + let queue: Promise | undefined; + let stopRecovery: (() => Promise) | undefined; + // Publish cleanup before any fallible worker initialization. + registerStop(async () => { + controller.abort(); + try { + await stopRecovery?.(); + await queue; + } finally { + await itemLeases.close(); } - - console.log(`Listening on http://${host}:${info.port}/`); - }, - ); -} - -// Start workers if running as worker or all node -let stopWorkers: (() => Promise) | undefined; -if (NODE_TYPE === "worker" || NODE_TYPE === "all") { - const [ - { federation }, - { backgroundJobs, replyScrapes, pollNotifications }, - { itemLeases }, - ] = await Promise.all([ - import("../src/federation"), - import("../src/federation/federation"), - import("../src/background/lease"), - ]); - - // Start the Fedify message queue - const controller = new AbortController(); - backgroundJobs.setSignal(controller.signal); - replyScrapes.setSignal(controller.signal); - pollNotifications.setSignal(controller.signal); - const queue = federation - .startQueue(undefined, { signal: controller.signal }) - .catch((error) => { - console.error("Error starting Fedify queue:", error); - process.exit(1); }); - - // Start the workers for background job processing - const { WEB_ORIGIN } = await import("../src/env"); - const stopRecovery = backgroundJobs.startRecovery( - federation.createContext( - new URL(WEB_ORIGIN ?? "http://localhost"), - undefined, - ), - controller.signal, - ); - - stopWorkers = async () => { - controller.abort(); - await stopRecovery(); - await queue; - await itemLeases.close(); - }; - console.log("Worker started (Fedify activity/task queues + Job recovery)"); -} - -// Graceful shutdown handling -let shuttingDown = false; -const shutdown = async () => { - if (shuttingDown) return; - shuttingDown = true; + backgroundJobs.setSignal(controller.signal); + replyScrapes.setSignal(controller.signal); + pollNotifications.setSignal(controller.signal); + queue = federation + .startQueue(undefined, { signal: controller.signal }) + .catch((error) => { + console.error("Error starting Fedify queue:", error); + void shutdown(1); + }); + stopRecovery = backgroundJobs.startRecovery( + federation.createContext( + new URL(WEB_ORIGIN ?? "http://localhost"), + undefined, + ), + controller.signal, + ); + console.log("Worker started (Fedify activity/task queues + Job recovery)"); + }, +]); + +let exitCode = 0; +let shutdownPromise: Promise | undefined; +function shutdown(code = 0): Promise { + exitCode = Math.max(exitCode, code); + if (shutdownPromise) return shutdownPromise; // Forced termination releases DB locks; durable unfinished items recover on // restart. Do not reuse a slot while its external operation is still alive. - const deadline = setTimeout(() => process.exit(0), 30_000); + const deadline = setTimeout(() => process.exit(exitCode), 30_000); deadline.unref(); - webServer?.close(); - await stopWorkers?.(); - // The legacy federation queue does not drain its handlers. Keep the main - // client available until process exit rather than rejecting their queries. - process.exit(0); -}; + shutdownPromise = lifecycle.stop().then( + () => { + // The legacy federation queue does not drain its handlers. Keep the main + // client available until process exit rather than rejecting their queries. + process.exit(exitCode); + }, + (error) => { + console.error("Error stopping server:", error); + process.exit(1); + }, + ); + return shutdownPromise; +} -process.on("SIGTERM", shutdown); -process.on("SIGINT", shutdown); +// Handle signals even during asynchronous startup. +process.on("SIGTERM", () => void shutdown()); +process.on("SIGINT", () => void shutdown()); +try { + await lifecycle.start(); +} catch (error) { + console.error("Error starting server:", error); + await shutdown(1); +} diff --git a/changes.d/streaming-http-server.md b/changes.d/streaming-http-server.md new file mode 100644 index 00000000..9939322b --- /dev/null +++ b/changes.d/streaming-http-server.md @@ -0,0 +1,14 @@ +--- +links: + '#672': https://github.com/fedify-dev/hollo/issues/672 + '#673': https://github.com/fedify-dev/hollo/pull/673 +--- + - Changed the HTTP server to avoid buffering entire uploads and incoming + activities before handling them behind a reverse proxy. Shutdown now waits + for active HTTP requests alongside background workers within the existing + 30-second deadline. + + Proxy mode now uses the first nonempty forwarded host and protocol in a + comma-separated list, and ignores invalid forwarded hosts in favor of the + original `Host` header. Trusted proxies must overwrite client-supplied + forwarded headers. [[#672], [#673]] diff --git a/docs/src/content/docs/install/env.mdx b/docs/src/content/docs/install/env.mdx index c8d340b0..94d07fa9 100644 --- a/docs/src/content/docs/install/env.mdx +++ b/docs/src/content/docs/install/env.mdx @@ -84,6 +84,12 @@ Turned off by default. security reasons. +When enabled, Hollo uses the first nonempty value in comma-separated +`X-Forwarded-Host` and `X-Forwarded-Proto` headers. Invalid forwarded hosts +are ignored in favor of the original `Host` header. Configure your proxy to +replace client-supplied forwarded headers and allow only trusted proxies to +connect directly to Hollo. + ### `HANDLE_HOST` The fediverse handle domain when running a [split-domain WebFinger diff --git a/docs/src/content/docs/ja/install/env.mdx b/docs/src/content/docs/ja/install/env.mdx index b3039cf2..ee4a2065 100644 --- a/docs/src/content/docs/ja/install/env.mdx +++ b/docs/src/content/docs/ja/install/env.mdx @@ -82,6 +82,11 @@ HolloがL7ロードバランサーの後ろにある場合（通常はそうす この動作はセキュリティ上注意が必要です。 +有効にすると、カンマ区切りの `X-Forwarded-Host` と `X-Forwarded-Proto` +ヘッダーの最初の空でない値を使用します。無効な転送ホストは無視され、元の +`Host` ヘッダーが使用されます。クライアントが送信した転送ヘッダーをプロキシで +上書きし、信頼できるプロキシだけが Hollo に直接接続できるようにしてください。 + ### `HANDLE_HOST` [ドメイン分離 WebFinger 設定][split-domain]で使用するフェディバースの diff --git a/docs/src/content/docs/ko/install/env.mdx b/docs/src/content/docs/ko/install/env.mdx index 78b896cb..f20cc03d 100644 --- a/docs/src/content/docs/ko/install/env.mdx +++ b/docs/src/content/docs/ko/install/env.mdx @@ -80,6 +80,11 @@ Hollo가 L7 로드 밸런서 뒤에 위치할 경우 (일반적으로 그래야 이 동작은 보안상 주의를 기울여야 합니다. +이 옵션을 켜면, 쉼표로 구분된 `X-Forwarded-Host`와 `X-Forwarded-Proto` +헤더에서 첫 번째 비어 있지 않은 값을 사용합니다. 유효하지 않은 전달 호스트는 +무시하고 원래 `Host` 헤더를 사용합니다. 프록시가 클라이언트가 보낸 전달 헤더를 +덮어쓰도록 설정하고, 신뢰할 수 있는 프록시만 Hollo에 직접 연결할 수 있게 하세요. + ### `HANDLE_HOST` [도메인 분리 WebFinger 설정][split-domain]에서 사용할 페디버스 핸들 diff --git a/docs/src/content/docs/zh-cn/install/env.mdx b/docs/src/content/docs/zh-cn/install/env.mdx index afc1762a..baac7a5b 100644 --- a/docs/src/content/docs/zh-cn/install/env.mdx +++ b/docs/src/content/docs/zh-cn/install/env.mdx @@ -73,6 +73,10 @@ openssl rand -hex 32 启用此选项后，Hollo将信任来自反向代理的`X-Forwarded-For`、`X-Forwarded-Proto`和`X-Forwarded-Host`头。这对于安全来说非常重要。 +启用后，Hollo 使用逗号分隔的 `X-Forwarded-Host` 和 `X-Forwarded-Proto` +头中的第一个非空值。无效的转发主机将被忽略，并使用原始 `Host` 头。 +请配置代理以覆盖客户端提供的转发头，并仅允许受信任的代理直接连接到 Hollo。 + ### `HANDLE_HOST` 在[分域 WebFinger 配置][split-domain]中用作联邦宇宙账号句柄的域名。 diff --git a/docs/src/content/docs/zh-tw/install/env.mdx b/docs/src/content/docs/zh-tw/install/env.mdx index 2ceec6c9..99f56083 100644 --- a/docs/src/content/docs/zh-tw/install/env.mdx +++ b/docs/src/content/docs/zh-tw/install/env.mdx @@ -73,6 +73,10 @@ openssl rand -hex 32 啟用此選項後，Hollo 將信任來自反向代理的 `X-Forwarded-For`、`X-Forwarded-Proto` 和 `X-Forwarded-Host` 標頭。這對於安全來說非常重要。 +啟用後，Hollo 使用以逗號分隔的 `X-Forwarded-Host` 和 `X-Forwarded-Proto` +標頭中的第一個非空值。無效的轉送主機會被忽略，並使用原始 `Host` 標頭。 +請設定代理以覆寫用戶端提供的轉送標頭，並只允許受信任的代理直接連線至 Hollo。 + ### `HANDLE_HOST` 在[分域 WebFinger 設定][split-domain]中用作聯邦宇宙帳號句柄的網域名稱。 diff --git a/package.json b/package.json index 785a9d4a..ad1cccec 100644 --- a/package.json +++ b/package.json @@ -84,10 +84,10 @@ "qrcode": "^1.5.4", "semver": "^7.8.1", "sharp": "^0.35.5", + "srvx": "^1.0.6", "ssrfcheck": "^1.4.0", "tsx": "^4.23.15", "uuidv7-js": "^1.1.4", - "x-forwarded-fetch": "^0.2.0", "xss": "^1.0.15", "zod": "^4.6.5" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 62501c1c..048f2481 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -332,6 +332,9 @@ importers: sharp: specifier: ^0.35.5 version: 0.35.5(@types/node@26.1.1) + srvx: + specifier: ^1.0.6 + version: 1.0.6 ssrfcheck: specifier: ^1.4.0 version: 1.4.0 @@ -341,9 +344,6 @@ importers: uuidv7-js: specifier: ^1.1.4 version: 1.1.4 - x-forwarded-fetch: - specifier: ^0.2.0 - version: 0.2.0 xss: specifier: ^1.0.15 version: 1.0.15 @@ -410,7 +410,7 @@ importers: version: '@typescript/typescript6@6.0.2' unocss: specifier: ^66.10.5 - version: 66.10.5(cac@7.0.0)(rolldown@1.2.0)(rollup@4.62.2)(srvx@1.0.5)(vite@8.2.2(@types/node@26.1.1)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.15)(yaml@2.9.0)) + version: 66.10.5(cac@7.0.0)(rolldown@1.2.0)(rollup@4.62.2)(srvx@1.0.6)(vite@8.2.2(@types/node@26.1.1)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.15)(yaml@2.9.0)) vitest: specifier: ^5.0.2 version: 5.0.2(@opentelemetry/api@1.9.1)(@types/node@26.1.1)(@vitest/coverage-v8@5.0.2)(vite@8.2.2(@types/node@26.1.1)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.15)(yaml@2.9.0)) @@ -5744,8 +5744,8 @@ packages: sprintf-js@1.1.3: resolution: {integrity: sha512-Oo+0REFV59/rz3gfJNKQiBlwfHaSESl1pcGyABQsnnIfWOFt6JNj5gCog2U6MLZ//IGYD+nA8nI+mTShREReaA==} - srvx@1.0.5: - resolution: {integrity: sha512-KvSKRpgPG/oaq3cyT614OQ2bAa7DynuGamivCZeoZzIUULOdbQz6wUtnnVuC4+DCx2Zglzo8v5gBYmWIWYhDxA==} + srvx@1.0.6: + resolution: {integrity: sha512-QeTKgAhk8OSsfeq60bl3wuVs9PmBftzmk7owBInA/F1YQne+TRw0UcPymNpURldAmx8hfmM2PNNGH/zgo3GpLA==} engines: {node: '>=20.16.0'} hasBin: true @@ -6399,9 +6399,6 @@ packages: resolution: {integrity: sha512-42AtmgqjV+X1VpdOfyTGOYRi0/zsoLqtXQckTmqTeybT+BDIbM/Guxo7x3pE2vtpr1ok6xRqM9OpBe+Jyoqyww==} engines: {node: '>=18'} - x-forwarded-fetch@0.2.0: - resolution: {integrity: sha512-2qsguQrLHFvP3/rnvxSSaw3DlK9eOOx9iyL+Qv+1YH8jzq6nbRk+P6gyFFNdWanyusyCHuE3/CnX3+gmLeYEeg==} - xss@1.0.15: resolution: {integrity: sha512-FVdlVVC67WOIPvfOwhoMETV72f6GbW7aOabBC3WxN/oUdoEMDyLz4OgRv5/gck2ZeNqEQu+Tb0kloovXOfpYVg==} engines: {node: '>= 0.10.0'} @@ -9777,12 +9774,12 @@ snapshots: dependencies: '@unocss/core': 66.10.5 - '@unocss/inspector@66.10.5(cac@7.0.0)(srvx@1.0.5)': + '@unocss/inspector@66.10.5(cac@7.0.0)(srvx@1.0.6)': dependencies: '@unocss/config': 66.10.5 '@unocss/core': 66.10.5 '@unocss/rule-utils': 66.10.5 - devframe: 1.1.0(cac@7.0.0)(srvx@1.0.5) + devframe: 1.1.0(cac@7.0.0)(srvx@1.0.6) tinyglobby: 0.2.17 unplugin-utils: 0.3.2 transitivePeerDependencies: @@ -9882,12 +9879,12 @@ snapshots: dependencies: '@unocss/core': 66.10.5 - '@unocss/vite@66.10.5(cac@7.0.0)(srvx@1.0.5)(vite@8.2.2(@types/node@26.1.1)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.15)(yaml@2.9.0))': + '@unocss/vite@66.10.5(cac@7.0.0)(srvx@1.0.6)(vite@8.2.2(@types/node@26.1.1)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.15)(yaml@2.9.0))': dependencies: '@jridgewell/remapping': 2.3.5 '@unocss/config': 66.10.5 '@unocss/core': 66.10.5 - '@unocss/inspector': 66.10.5(cac@7.0.0)(srvx@1.0.5) + '@unocss/inspector': 66.10.5(cac@7.0.0)(srvx@1.0.6) chokidar: 5.0.0 magic-string: 1.4.2 pathe: 2.0.3 @@ -10398,9 +10395,9 @@ snapshots: dependencies: uncrypto: 0.1.3 - crossws@0.4.12(srvx@1.0.5): + crossws@0.4.12(srvx@1.0.6): optionalDependencies: - srvx: 1.0.5 + srvx: 1.0.6 css-select@5.2.2: dependencies: @@ -10498,10 +10495,10 @@ snapshots: devalue@5.9.2: {} - devframe@1.1.0(cac@7.0.0)(srvx@1.0.5): + devframe@1.1.0(cac@7.0.0)(srvx@1.0.6): dependencies: - crossws: 0.4.12(srvx@1.0.5) - h3: 2.0.1-rc.32(crossws@0.4.12(srvx@1.0.5)) + crossws: 0.4.12(srvx@1.0.6) + h3: 2.0.1-rc.32(crossws@0.4.12(srvx@1.0.6)) nostics: 1.3.0 pathe: 2.0.3 optionalDependencies: @@ -10907,12 +10904,12 @@ snapshots: ufo: 1.6.4 uncrypto: 0.1.3 - h3@2.0.1-rc.32(crossws@0.4.12(srvx@1.0.5)): + h3@2.0.1-rc.32(crossws@0.4.12(srvx@1.0.6)): dependencies: rou3: 0.9.2 - srvx: 1.0.5 + srvx: 1.0.6 optionalDependencies: - crossws: 0.4.12(srvx@1.0.5) + crossws: 0.4.12(srvx@1.0.6) has-flag@4.0.0: {} @@ -12223,7 +12220,7 @@ snapshots: sprintf-js@1.1.3: {} - srvx@1.0.5: {} + srvx@1.0.6: {} ssrfcheck@1.4.0: {} @@ -12511,7 +12508,7 @@ snapshots: unist-util-is: 6.0.1 unist-util-visit-parents: 6.0.2 - unocss@66.10.5(cac@7.0.0)(rolldown@1.2.0)(rollup@4.62.2)(srvx@1.0.5)(vite@8.2.2(@types/node@26.1.1)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.15)(yaml@2.9.0)): + unocss@66.10.5(cac@7.0.0)(rolldown@1.2.0)(rollup@4.62.2)(srvx@1.0.6)(vite@8.2.2(@types/node@26.1.1)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.15)(yaml@2.9.0)): dependencies: '@unocss/cli': 66.10.5 '@unocss/core': 66.10.5 @@ -12530,7 +12527,7 @@ snapshots: '@unocss/transformer-compile-class': 66.10.5 '@unocss/transformer-directives': 66.10.5 '@unocss/transformer-variant-group': 66.10.5 - '@unocss/vite': 66.10.5(cac@7.0.0)(srvx@1.0.5)(vite@8.2.2(@types/node@26.1.1)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.15)(yaml@2.9.0)) + '@unocss/vite': 66.10.5(cac@7.0.0)(srvx@1.0.6)(vite@8.2.2(@types/node@26.1.1)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.15)(yaml@2.9.0)) transitivePeerDependencies: - '@devframes/agentic' - cac @@ -12792,8 +12789,6 @@ snapshots: string-width: 7.2.0 strip-ansi: 7.2.0 - x-forwarded-fetch@0.2.0: {} - xss@1.0.15: dependencies: commander: 2.20.3 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 992049ed..29044110 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -13,6 +13,7 @@ overrides: minimumReleaseAgeExclude: - '@fedify/*' +- srvx@1.0.6 permissions: '@fedify/fedify': diff --git a/src/http-server.db.test.ts b/src/http-server.db.test.ts new file mode 100644 index 00000000..9043468d --- /dev/null +++ b/src/http-server.db.test.ts @@ -0,0 +1,72 @@ +import { request as httpRequest } from "node:http"; + +import { Hono } from "hono"; +import postgres from "postgres"; +import { expect, it } from "vitest"; + +import { + isPostgresQueryCancelError, + postgresQueryCancellationMiddleware, + wrapPostgresClient, +} from "./db-cancel"; +import { createHttpServer } from "./http-server"; + +it("cancels an active SQL query when the HTTP client disconnects", async () => { + const applicationName = `hollo-http-cancel-${crypto.randomUUID()}`; + const client = wrapPostgresClient( + postgres(process.env.DATABASE_URL!, { + max: 1, + connection: { application_name: applicationName }, + }), + ); + const observer = postgres(process.env.DATABASE_URL!, { max: 1 }); + const cancelled = Promise.withResolvers(); + const app = new Hono(); + // Match Hollo: let the cancellation middleware normalize a query error. + app.onError((error) => { + throw error; + }); + app.use(postgresQueryCancellationMiddleware()); + app.get("/", async (c) => { + try { + await client.unsafe("select pg_sleep(30)"); + return c.text("finished"); + } catch (error) { + cancelled.resolve(error); + throw error; + } + }); + const server = createHttpServer(app.fetch, { + port: 0, + hostname: "127.0.0.1", + behindProxy: true, + }); + await server.ready(); + const request = httpRequest(server.url!, { + headers: { "X-Forwarded-Proto": "https" }, + agent: false, + }); + request.on("error", () => {}); + try { + request.end(); + // Observe execution, rather than racing disconnect against lazy SQL dispatch. + await expect + .poll(async () => { + const rows = await observer` + select 1 from pg_stat_activity + where application_name = ${applicationName} and state = 'active' + `; + return rows.length; + }) + .toBe(1); + request.destroy(); + expect(isPostgresQueryCancelError(await cancelled.promise)).toBe(true); + } finally { + request.destroy(); + await server.close(true); + await Promise.all([ + client.end({ timeout: 1 }), + observer.end({ timeout: 1 }), + ]); + } +}); diff --git a/src/http-server.test.ts b/src/http-server.test.ts new file mode 100644 index 00000000..44f4864d --- /dev/null +++ b/src/http-server.test.ts @@ -0,0 +1,484 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { Agent } from "node:http"; +import { request as httpRequest, type IncomingHttpHeaders } from "node:http"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { text } from "node:stream/consumers"; + +import { + generateCryptoKeyPair, + signRequest, + verifyRequest, +} from "@fedify/fedify/sig"; +import { CryptographicKey } from "@fedify/vocab"; +import { serveStatic } from "@hono/node-server/serve-static"; +import { Hono } from "hono"; +import type { Server } from "srvx"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { createHttpServer } from "./http-server"; + +const servers: Server[] = []; + +async function start( + handler: (request: Request) => Response | Promise, + behindProxy = false, + hostname: string | null = "127.0.0.1", +) { + const server = createHttpServer(handler, { + port: 0, + hostname: hostname ?? undefined, + behindProxy, + }); + servers.push(server); + await server.ready(); + return server; +} + +function send( + server: Server, + { + method = "GET", + headers = {}, + body, + path = "/", + }: { + method?: string; + headers?: Record; + body?: string; + path?: string; + } = {}, +): Promise<{ + status: number; + headers: IncomingHttpHeaders; + body: string; +}> { + return new Promise((resolve, reject) => { + const request = httpRequest( + server.url!, + { method, path, headers, agent: false }, + (response) => { + text(response).then( + (body) => + resolve({ + status: response.statusCode!, + headers: response.headers, + body, + }), + reject, + ); + }, + ); + request.on("error", reject); + request.end(body); + }); +} + +afterEach(async () => { + await Promise.all(servers.splice(0).map((server) => server.close(true))); + vi.unstubAllEnvs(); + vi.restoreAllMocks(); +}); + +describe("HTTP server proxy boundary", () => { + it("ignores forwarded origins unless BEHIND_PROXY is enabled", async () => { + const server = await start((request) => + Response.json({ + url: request.url, + host: request.headers.get("Host"), + forwarded: request.headers.get("X-Forwarded-Host"), + }), + ); + const response = await send(server, { + headers: { + Host: "internal.example:3000", + "X-Forwarded-Host": "public.example", + "X-Forwarded-Proto": "https", + }, + }); + expect(JSON.parse(response.body)).toEqual({ + url: "http://internal.example:3000/", + host: "internal.example:3000", + forwarded: "public.example", + }); + }); + + it.each([ + ["Public.Example:443", "Public.Example:443"], + ["public.example:8443", "public.example:8443"], + ["[2001:db8::1]:443", "[2001:db8::1]:443"], + ["public.example, internal.example", "public.example"], + [", public.example", "public.example"], + [" , , public.example, ", "public.example"], + ["public.example,", "public.example"], + ["evil.example/path", "internal.example:3000"], + ["user@evil.example", "internal.example:3000"], + ["evil.example:999999", "internal.example:3000"], + ["evil.example?x", "internal.example:3000"], + ["evil.example#x", "internal.example:3000"], + ])("keeps Host consistent with srvx for %s", async (forwarded, host) => { + const server = await start( + (request) => + Response.json({ + url: request.url, + host: request.headers.get("Host"), + proto: request.headers.get("X-Forwarded-Proto"), + forwarded: request.headers.get("X-Forwarded-Host"), + for: request.headers.get("X-Forwarded-For"), + }), + true, + ); + // Dot segments force native URL normalization inside srvx's FastURL. + const response = await send(server, { + path: "/a/../inbox?x=1", + headers: { + Host: "internal.example:3000", + "X-Forwarded-Host": forwarded, + "X-Forwarded-Proto": ", https, http", + "X-Forwarded-For": "192.0.2.1", + }, + }); + const result = JSON.parse(response.body); + expect(result.host).toBe(host); + expect(new URL(result.url).href).toBe( + new URL(`https://${host}/inbox?x=1`).href, + ); + expect(result.proto).toBeNull(); + expect(result.forwarded).toBeNull(); + expect(result.for).toBe("192.0.2.1"); + }); + + it("preserves Host when the proxy only supplies the public scheme", async () => { + const server = await start( + (request) => + Response.json({ url: request.url, host: request.headers.get("Host") }), + true, + ); + const response = await send(server, { + headers: { Host: "Public.Example:443", "X-Forwarded-Proto": "https" }, + }); + expect(JSON.parse(response.body)).toEqual({ + url: "https://Public.Example:443/", + host: "Public.Example:443", + }); + }); + + it.each(["draft-cavage-http-signatures-12", "rfc9421"] as const)( + "verifies %s signed bodies through a trusted proxy", + async (spec) => { + const pair = await generateCryptoKeyPair(); + const keyId = new URL("https://remote.example/actor#key"); + const key = new CryptographicKey({ + id: keyId, + owner: new URL("https://remote.example/actor"), + publicKey: pair.publicKey, + }); + const body = '{"content":"한글 🐻","type":"Create"}'; + const signed = await signRequest( + new Request("https://public.example/inbox", { + method: "POST", + body, + headers: { + "Content-Type": "application/activity+json", + Host: "Public.Example:443", + }, + }), + pair.privateKey, + keyId, + { spec }, + ); + const server = await start(async (request) => { + const verified = await verifyRequest(request, { + keyCache: { get: async () => key, set: async () => {} }, + documentLoader: async () => { + throw new Error("Signature test must not use the network"); + }, + }); + return Response.json({ + verified: verified != null, + clone: await request.clone().text(), + body: await request.text(), + }); + }, true); + const response = await send(server, { + path: "/inbox", + method: "POST", + body: await signed.text(), + headers: { + ...Object.fromEntries(signed.headers), + host: "internal.example:3000", + "x-forwarded-host": "Public.Example:443", + "x-forwarded-proto": "https", + }, + }); + expect(response.status).toBe(200); + expect(JSON.parse(response.body)).toEqual({ + verified: true, + clone: body, + body, + }); + }, + ); +}); + +describe("HTTP transport", () => { + it("passes a chunk to the application before the upload completes", async () => { + const received = Promise.withResolvers(); + const server = await start(async (request) => { + const reader = request.body!.getReader(); + const first = await reader.read(); + received.resolve(new TextDecoder().decode(first.value)); + let rest = ""; + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + rest += new TextDecoder().decode(value); + } + return new Response(rest); + }, true); + const request = httpRequest(server.url!, { + method: "POST", + headers: { "X-Forwarded-Proto": "https" }, + agent: false, + }); + const response = new Promise((resolve, reject) => { + request.on("error", reject); + request.on("response", (response) => + text(response).then(resolve, reject), + ); + }); + try { + request.flushHeaders(); + request.write("first"); + expect(await received.promise).toBe("first"); + request.end("last"); + expect(await response).toBe("last"); + } finally { + request.destroy(); + } + }); + + it("parses multipart uploads through the Hono application", async () => { + const app = new Hono(); + app.post("/", async (c) => { + const data = await c.req.formData(); + const file = data.get("file") as File; + return c.json({ title: data.get("title"), file: await file.text() }); + }); + const server = await start(app.fetch, true); + const data = new FormData(); + data.set("title", "hello"); + data.set("file", new File(["한글"], "file.txt", { type: "text/plain" })); + const response = await fetch(server.url!, { + method: "POST", + body: data, + headers: { "X-Forwarded-Proto": "https" }, + }); + expect(await response.json()).toEqual({ title: "hello", file: "한글" }); + }); + + it("aborts the application signal when an incomplete client disconnects", async () => { + const entered = Promise.withResolvers(); + const aborted = Promise.withResolvers(); + const server = await start(async (request) => { + const signal = request.signal; + signal.addEventListener("abort", () => aborted.resolve(signal.aborted)); + entered.resolve(); + await aborted.promise; + return new Response(null, { status: 499 }); + }, true); + const request = httpRequest(server.url!, { + method: "POST", + headers: { "Content-Length": "1000000", "X-Forwarded-Proto": "https" }, + }); + request.on("error", () => {}); + try { + request.flushHeaders(); + request.write("partial"); + await entered.promise; + request.destroy(); + expect(await aborted.promise).toBe(true); + } finally { + request.destroy(); + } + }); + + it("responds to an unread incomplete body without waiting for its end", async () => { + const server = await start( + () => new Response("rejected", { status: 403 }), + true, + ); + const agent = new Agent({ keepAlive: true }); + const request = httpRequest(server.url!, { + method: "POST", + headers: { "Content-Length": "1000000" }, + agent, + }); + const closed = new Promise((resolve) => { + request.on("socket", (socket) => socket.once("close", resolve)); + }); + const response = new Promise((resolve, reject) => { + request.on("error", reject); + request.on("response", (response) => { + response.resume(); + resolve(response.statusCode); + }); + }); + try { + request.flushHeaders(); + request.write("partial"); + expect(await response).toBe(403); + await closed; + expect(request.socket?.destroyed).toBe(true); + } finally { + request.destroy(); + agent.destroy(); + } + expect((await send(server)).status).toBe(403); + }); + + it("preserves native constructors, cookies, redirects and response streams", async () => { + const NativeRequest = globalThis.Request; + const NativeResponse = globalThis.Response; + const app = new Hono(); + app.get("/cookies", (c) => { + c.header("Set-Cookie", "a=1", { append: true }); + c.header("Set-Cookie", "b=2", { append: true }); + return c.redirect("/stream"); + }); + app.get( + "/stream", + () => + new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("한글")); + controller.close(); + }, + }), + ), + ); + const server = await start(app.fetch); + const response = await send(server, { path: "/cookies" }); + expect(response.status).toBe(302); + expect(response.headers.location).toBe("/stream"); + expect(response.headers["set-cookie"]).toEqual(["a=1", "b=2"]); + expect((await send(server, { path: "/stream" })).body).toBe("한글"); + expect(globalThis.Request).toBe(NativeRequest); + expect(globalThis.Response).toBe(NativeResponse); + }); + + it("retains the existing static file helper's HEAD and range behavior", async () => { + const dir = await mkdtemp(join(tmpdir(), "hollo-http-static-")); + try { + await writeFile(join(dir, "hello.txt"), "hello world"); + const app = new Hono(); + app.use("/*", serveStatic({ root: dir })); + const server = await start(app.fetch); + const get = await send(server, { path: "/hello.txt" }); + expect(get.body).toBe("hello world"); + const head = await send(server, { path: "/hello.txt", method: "HEAD" }); + expect(head.body).toBe(""); + expect(head.headers["content-length"]).toBe("11"); + const range = await send(server, { + path: "/hello.txt", + headers: { Range: "bytes=1-3" }, + }); + expect(range.status).toBe(206); + expect(range.body).toBe("ell"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it("does not expose thrown errors to HTTP clients", async () => { + vi.spyOn(console, "error").mockImplementation(() => {}); + const server = await start(() => { + throw new Error("private credentials"); + }); + const response = await send(server); + expect(response.status).toBe(500); + expect(response.body).toBe("Internal Server Error"); + }); + + it.each([undefined, "127.0.0.2"])( + "ignores HOST=%s when BIND is unset", + async (host) => { + vi.stubEnv("HOST", host); + const server = await start(() => new Response("ok"), false, null); + const address = server.node!.server!.address(); + expect(address && typeof address !== "string" && address.address).toMatch( + /^(::|0\.0\.0\.0)$/, + ); + }, + ); + + it("rejects readiness on a port conflict", async () => { + const listening = await start(() => new Response("ok")); + const server = createHttpServer(() => new Response("unreachable"), { + hostname: "127.0.0.1", + port: Number(new URL(listening.url!).port), + behindProxy: false, + }); + await expect(server.ready()).rejects.toHaveProperty("code", "EADDRINUSE"); + }); + + it("waits for an active response before close completes", async () => { + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); + const server = await start(async () => { + entered.resolve(); + await release.promise; + return new Response("finished"); + }); + const response = send(server); + await entered.promise; + let closed = false; + const close = server.close().then(() => { + closed = true; + }); + expect(closed).toBe(false); + release.resolve(); + expect((await response).body).toBe("finished"); + await close; + expect(closed).toBe(true); + }); + + it("finishes shutdown while a proxy tries to reuse a keep-alive connection", async () => { + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); + let handled = 0; + const server = await start(async () => { + handled++; + entered.resolve(); + await release.promise; + return new Response("finished"); + }); + const url = server.url!; + const agent = new Agent({ keepAlive: true, maxSockets: 1 }); + const get = () => + new Promise((resolve, reject) => { + const request = httpRequest(url, { agent }, (response) => { + text(response).then(resolve, reject); + }); + request.on("error", reject); + request.end(); + }); + const response = get(); + await entered.promise; + const close = server.close(); + // A busy proxy must not keep this connection alive until the 30s deadline. + const reuse = setInterval(() => { + void get().catch(() => {}); + }, 100); + try { + release.resolve(); + expect(await response).toBe("finished"); + await close; + expect(handled).toBe(1); + } finally { + clearInterval(reuse); + agent.destroy(); + } + }); +}); diff --git a/src/http-server.ts b/src/http-server.ts new file mode 100644 index 00000000..c5b2e66c --- /dev/null +++ b/src/http-server.ts @@ -0,0 +1,67 @@ +import type { Server, ServerRequest } from "srvx"; +import { serve } from "srvx/node"; + +// Match srvx's authority validation so a rejected forwarded host cannot be +// restored into the header used by HTTP signature verification. +// https://github.com/h3js/srvx/blob/v1.0.6/src/_trust-proxy.ts +const forwardedHostPattern = + /^(\[(?:[A-Fa-f0-9:.]+)\]|(?:[A-Za-z0-9_-]+\.)*[A-Za-z0-9_-]+|(?:\d{1,3}\.){3}\d{1,3})(:\d{1,5})?$/; + +/** Run a Fetch application without buffering or replacing incoming requests. */ +export function createHttpServer( + fetch: (request: Request) => Response | Promise, + { + port, + hostname, + behindProxy, + }: { port: number; hostname?: string; behindProxy: boolean }, +): Server { + const server = serve({ + port, + hostname, + // Override srvx's HOST fallback even when BIND is undefined. + node: { host: hostname }, + trustProxy: behindProxy, + gracefulShutdown: false, + silent: true, + error(error) { + console.error("Error handling HTTP request:", error); + return new Response("Internal Server Error", { status: 500 }); + }, + fetch(request: ServerRequest) { + const node = request.runtime?.node; + if (node?.res) { + // An early response must not retain a slow, incomplete upload socket. + // Wait until the response has been sent before closing the connection. + node.res.once("finish", () => { + if (!node.req.complete) node.req.destroy(); + // close() only closes connections that were idle at its call time. + // Also close those becoming idle as in-flight responses finish. + const http = server.node?.server; + if (http && !http.listening && "closeIdleConnections" in http) { + http.closeIdleConnections(); + } + }); + } + if (behindProxy) { + // srvx caches its URL lazily. Resolve it before removing the headers + // used for its public origin, without materializing the request body. + void request.url; + const host = request.headers + .get("X-Forwarded-Host") + ?.split(",") + .map((value) => value.trim()) + .find((value) => value !== ""); + if (host && forwardedHostPattern.test(host)) { + // Keep case and explicit default ports: URL.host normalizes them, + // but the Host header can be part of an HTTP signature. + request.headers.set("Host", host); + } + request.headers.delete("X-Forwarded-Host"); + request.headers.delete("X-Forwarded-Proto"); + } + return fetch(request); + }, + }); + return server; +} diff --git a/src/server-lifecycle.test.ts b/src/server-lifecycle.test.ts new file mode 100644 index 00000000..6a6f6011 --- /dev/null +++ b/src/server-lifecycle.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it, vi } from "vitest"; + +import { createServerLifecycle } from "./server-lifecycle"; + +describe("server lifecycle", () => { + it("starts sequentially and stops services concurrently exactly once", async () => { + const draining = Promise.withResolvers(); + const closing = Promise.withResolvers(); + const stopHttp = vi.fn(() => { + closing.resolve(); + return draining.promise; + }); + const stopWorkers = vi.fn(); + const lifecycle = createServerLifecycle([ + async (register) => register(stopHttp), + async (register) => register(stopWorkers), + ]); + await lifecycle.start(); + const stop = lifecycle.stop(); + expect(lifecycle.stop()).toBe(stop); + await closing.promise; + expect(stopWorkers).toHaveBeenCalledOnce(); + draining.resolve(); + await stop; + expect(stopHttp).toHaveBeenCalledOnce(); + }); + + it("releases an acquired resource after stop interrupts startup", async () => { + const entered = Promise.withResolvers(); + const acquired = Promise.withResolvers(); + const close = vi.fn(); + const startWorkers = vi.fn(async () => {}); + const lifecycle = createServerLifecycle([ + async (register) => { + entered.resolve(); + await acquired.promise; + register(close); + }, + startWorkers, + ]); + const start = lifecycle.start(); + await entered.promise; + const stop = lifecycle.stop(); + expect(lifecycle.stopping).toBe(true); + acquired.resolve(); + await Promise.all([start, stop]); + expect(close).toHaveBeenCalledOnce(); + expect(startWorkers).not.toHaveBeenCalled(); + }); + + it("cleans up partial startup after a failure", async () => { + const close = vi.fn(); + const lifecycle = createServerLifecycle([ + async (register) => { + register(close); + throw new Error("startup failed"); + }, + ]); + await expect(lifecycle.start()).rejects.toThrow("startup failed"); + await lifecycle.stop(); + expect(close).toHaveBeenCalledOnce(); + }); + + it("waits for other services even if a close callback throws", async () => { + const drained = Promise.withResolvers(); + const closing = Promise.withResolvers(); + let stopped = false; + const lifecycle = createServerLifecycle([ + async (register) => { + register(() => { + throw new Error("HTTP close failed"); + }); + register(async () => { + closing.resolve(); + await drained.promise; + stopped = true; + }); + }, + ]); + await lifecycle.start(); + const stop = lifecycle.stop(); + const failure = stop.catch((error: unknown) => error); + await closing.promise; + expect(stopped).toBe(false); + drained.resolve(); + expect(await failure).toBeInstanceOf(AggregateError); + expect(stopped).toBe(true); + }); + + it("does not start services after an early stop", async () => { + const start = vi.fn(async () => {}); + const lifecycle = createServerLifecycle([start]); + await lifecycle.stop(); + await lifecycle.start(); + expect(start).not.toHaveBeenCalled(); + }); +}); diff --git a/src/server-lifecycle.ts b/src/server-lifecycle.ts new file mode 100644 index 00000000..6be9df8c --- /dev/null +++ b/src/server-lifecycle.ts @@ -0,0 +1,41 @@ +type Stop = () => void | Promise; +type Start = (registerStop: (stop: Stop) => void) => Promise; + +/** Coordinate startup and shutdown of the HTTP server and background workers. */ +export function createServerLifecycle(starters: Start[]) { + const stops: Stop[] = []; + let stopping = false; + let startup: Promise | undefined; + let shutdown: Promise | undefined; + + return { + get stopping() { + return stopping; + }, + start(): Promise { + return (startup ??= (async () => { + for (const start of starters) { + if (stopping) break; + await start((stop) => stops.push(stop)); + } + })()); + }, + stop(): Promise { + stopping = true; + return (shutdown ??= (async () => { + // A stop during an import/listen must also release resources acquired + // by that pending startup. The caller reports any startup failure. + await startup?.catch(() => {}); + const results = await Promise.allSettled( + stops.map((stop) => Promise.resolve().then(stop)), + ); + const failures = results + .filter((result) => result.status === "rejected") + .map((result) => result.reason); + if (failures.length > 0) { + throw new AggregateError(failures, "Error stopping server"); + } + })()); + }, + }; +} From 22df856ab8b8d6cc4e04d8504fd7eea34c75f686 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 18 Oct 2026 01:22:52 +0900 Subject: [PATCH 2/4] Use FastResponse for web application responses Install srvx's lazy response implementation after Sentry initialization and before loading the web application. Worker-only processes keep the native response constructor. Snapshot Response-valued initializers before srvx materializes them so Hono's header merging and compression preserve non-200 status codes. Cover native interoperability, API middleware, cookies and streaming. https://github.com/fedify-dev/hollo/issues/672 Assisted-by: OpenCode:deepseek-flash Assisted-by: Codex:gpt-6.1-sol --- CHANGES.md | 3 + bin/server.ts | 4 +- changes.d/streaming-http-server.md | 3 + src/fast-response.test.ts | 132 +++++++++++++++++++++++++++++ src/http-server.ts | 22 ++++- 5 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 src/fast-response.test.ts diff --git a/CHANGES.md b/CHANGES.md index d1545522..ea5ef15a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -151,6 +151,9 @@ To be released. for active HTTP requests alongside background workers within the existing 30-second deadline. + Responses now avoid creating a web stream for strings and byte buffers + until one is needed. + Proxy mode now uses the first nonempty forwarded host and protocol in a comma-separated list, and ignores invalid forwarded hosts in favor of the original `Host` header. Trusted proxies must overwrite client-supplied diff --git a/bin/server.ts b/bin/server.ts index 64142e22..685460ef 100644 --- a/bin/server.ts +++ b/bin/server.ts @@ -47,7 +47,9 @@ const lifecycle = createServerLifecycle([ async (registerStop) => { if (NODE_TYPE === "worker") return; // Initialize Sentry before loading the HTTP adapter and application. - const { createHttpServer } = await import("../src/http-server"); + const { createHttpServer, enableFastResponse } = + await import("../src/http-server"); + enableFastResponse(); const { default: app } = await import("../src/index"); if (lifecycle.stopping) return; const server = createHttpServer(app.fetch.bind(app), { diff --git a/changes.d/streaming-http-server.md b/changes.d/streaming-http-server.md index 9939322b..b0467567 100644 --- a/changes.d/streaming-http-server.md +++ b/changes.d/streaming-http-server.md @@ -8,6 +8,9 @@ links: for active HTTP requests alongside background workers within the existing 30-second deadline. + Responses now avoid creating a web stream for strings and byte buffers + until one is needed. + Proxy mode now uses the first nonempty forwarded host and protocol in a comma-separated list, and ignores invalid forwarded hosts in favor of the original `Host` header. Trusted proxies must overwrite client-supplied diff --git a/src/fast-response.test.ts b/src/fast-response.test.ts new file mode 100644 index 00000000..858bc6a3 --- /dev/null +++ b/src/fast-response.test.ts @@ -0,0 +1,132 @@ +import { Hono } from "hono"; +import { compress } from "hono/compress"; +import { cors } from "hono/cors"; +import type { Server } from "srvx"; +import { FastResponse } from "srvx/node"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { createHttpServer, enableFastResponse } from "./http-server"; + +const nativeRequest = globalThis.Request; +const nativeResponse = globalThis.Response; +let server: Server; + +beforeAll(async () => { + enableFastResponse(); + const app = new Hono(); + app.use("/status/*", compress()); + app.use("/status/*", cors()); + for (const code of [201, 401, 403, 404] as const) { + app.get(`/status/${code}`, (c) => c.json({ code }, code)); + } + app.get("/json", (c) => c.json({ message: "안녕하세요" })); + app.get("/cookies", (c) => { + c.header("Set-Cookie", "a=1", { append: true }); + c.header("Set-Cookie", "b=2", { append: true }); + return c.text("cookies"); + }); + app.get("/redirect", (c) => c.redirect("/json", 302)); + app.get("/native", () => new nativeResponse("native")); + app.get( + "/stream", + () => + new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("first")); + setTimeout(() => { + controller.enqueue(new TextEncoder().encode("second")); + controller.close(); + }, 25); + }, + }), + ), + ); + app.get("/headers", async (c) => { + c.res = c.text("body"); + c.header("Cache-Control", "private, no-store"); + return c.res; + }); + server = createHttpServer(app.fetch.bind(app), { + port: 0, + hostname: "127.0.0.1", + behindProxy: false, + }); + await server.ready(); +}); + +afterAll(async () => { + await server?.close(true); + globalThis.Response = nativeResponse; +}); + +describe("FastResponse", () => { + it.each([201, 401, 403, 404])( + "preserves status %i through API middleware", + async (code) => { + const response = await fetch(new URL(`/status/${code}`, server.url)); + expect(response.status).toBe(code); + expect(await response.json()).toEqual({ code }); + expect(response.headers.get("Access-Control-Allow-Origin")).toBe("*"); + }, + ); + + it("preserves response-valued initializers when materializing the body", async () => { + const original = new nativeResponse("denied", { + status: 401, + statusText: "Denied", + }); + const copy = new Response(original.body, original); + copy.headers.set("X-Test", "yes"); + expect(await copy.text()).toBe("denied"); + expect(copy.status).toBe(401); + expect(copy.statusText).toBe("Denied"); + expect(copy.headers.get("X-Test")).toBe("yes"); + }); + + it("replaces only Response and preserves native response interoperability", async () => { + expect(Object.getPrototypeOf(globalThis.Response)).toBe(FastResponse); + expect(globalThis.Request).toBe(nativeRequest); + expect(Response.error().type).toBe("error"); + expect( + Response.redirect("https://example.com/").headers.get("Location"), + ).toBe("https://example.com/"); + const response = new Response("hello"); + expect(response).toBeInstanceOf(nativeResponse); + expect(await response.clone().text()).toBe("hello"); + expect(await response.text()).toBe("hello"); + expect(await (await fetch(new URL("/native", server.url))).text()).toBe( + "native", + ); + }); + + it("sends Hono JSON with the UTF-8 byte length and supports HEAD", async () => { + const response = await fetch(new URL("/json", server.url)); + const body = await response.text(); + expect(JSON.parse(body)).toEqual({ message: "안녕하세요" }); + expect(response.headers.get("Content-Length")).toBe( + String(Buffer.byteLength(body)), + ); + const head = await fetch(new URL("/json", server.url), { method: "HEAD" }); + expect(head.status).toBe(200); + expect(await head.text()).toBe(""); + }); + + it("keeps multiple cookies and redirects", async () => { + const cookies = await fetch(new URL("/cookies", server.url)); + expect(cookies.headers.getSetCookie()).toEqual(["a=1", "b=2"]); + const redirect = await fetch(new URL("/redirect", server.url), { + redirect: "manual", + }); + expect(redirect.status).toBe(302); + expect(redirect.headers.get("Location")).toBe("/json"); + }); + + it("streams responses and preserves Hono header changes", async () => { + const response = await fetch(new URL("/stream", server.url)); + expect(await response.text()).toBe("firstsecond"); + const headers = await fetch(new URL("/headers", server.url)); + expect(headers.headers.get("Cache-Control")).toBe("private, no-store"); + expect(await headers.text()).toBe("body"); + }); +}); diff --git a/src/http-server.ts b/src/http-server.ts index c5b2e66c..7f32c6d1 100644 --- a/src/http-server.ts +++ b/src/http-server.ts @@ -1,6 +1,26 @@ -import type { Server, ServerRequest } from "srvx"; +import { FastResponse, type Server, type ServerRequest } from "srvx"; import { serve } from "srvx/node"; +/** Install before loading the web application, after configuring Sentry. */ +export function enableFastResponse(): void { + globalThis.Response = class Response extends FastResponse { + constructor(body?: BodyInit | null, init?: ResponseInit) { + // Hono passes a Response as its initializer when merging headers. srvx + // 1.0.6 later spreads that initializer, losing inherited status getters. + super( + body, + init instanceof FastResponse + ? { + status: init.status, + statusText: init.statusText, + headers: init.headers, + } + : init, + ); + } + }; +} + // Match srvx's authority validation so a rejected forwarded host cannot be // restored into the header used by HTTP signature verification. // https://github.com/h3js/srvx/blob/v1.0.6/src/_trust-proxy.ts From 42bd8b435dd17413b6b29dd1b358698f26183f6c Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 18 Oct 2026 01:54:45 +0900 Subject: [PATCH 3/4] Serve static files with srvx Replace the last @hono/node-server helper with srvx/static for bundled files and filesystem media. Preserve exact-file and directory-index lookup, MIME types and legacy methods. Keep compression and cache lifetimes unchanged, add conditional responses, and reject hidden files and escaping symlinks before srvx can select an adjacent HTML alias. Correct the empty OPTIONS response length. https://github.com/fedify-dev/hollo/issues/672 Assisted-by: OpenCode:deepseek-flash Assisted-by: Codex:gpt-6.1-sol --- CHANGES.md | 4 + changes.d/streaming-http-server.md | 4 + package.json | 1 - pnpm-lock.yaml | 13 -- src/http-server.test.ts | 4 +- src/index.tsx | 13 +- src/serve-static.test.ts | 294 +++++++++++++++++++++++++++++ src/serve-static.ts | 102 ++++++++++ 8 files changed, 412 insertions(+), 23 deletions(-) create mode 100644 src/serve-static.test.ts create mode 100644 src/serve-static.ts diff --git a/CHANGES.md b/CHANGES.md index ea5ef15a..2f2d29ea 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -154,6 +154,10 @@ To be released. Responses now avoid creating a web stream for strings and byte buffers until one is needed. + Static files now support conditional requests with `ETag` and + `Last-Modified` validators. Hidden files and symlinks pointing outside the + configured storage directory are no longer served. + Proxy mode now uses the first nonempty forwarded host and protocol in a comma-separated list, and ignores invalid forwarded hosts in favor of the original `Host` header. Trusted proxies must overwrite client-supplied diff --git a/changes.d/streaming-http-server.md b/changes.d/streaming-http-server.md index b0467567..38eacc49 100644 --- a/changes.d/streaming-http-server.md +++ b/changes.d/streaming-http-server.md @@ -11,6 +11,10 @@ links: Responses now avoid creating a web stream for strings and byte buffers until one is needed. + Static files now support conditional requests with `ETag` and + `Last-Modified` validators. Hidden files and symlinks pointing outside the + configured storage directory are no longer served. + Proxy mode now uses the first nonempty forwarded host and protocol in a comma-separated list, and ignores invalid forwarded hosts in favor of the original `Host` header. Trusted proxies must overwrite client-supplied diff --git a/package.json b/package.json index ad1cccec..ec17d4ec 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,6 @@ "@gukhanmun/opendict-fst": "^0.2.0", "@gukhanmun/stdict-fst": "^0.2.0", "@hexagon/base64": "^2.0.4", - "@hono/node-server": "^2.1.4", "@hono/zod-validator": "^0.9.1", "@js-temporal/polyfill": "^0.5.0", "@logtape/drizzle-orm": "~2.3.10", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 048f2481..62f08ced 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -218,9 +218,6 @@ importers: '@hexagon/base64': specifier: ^2.0.4 version: 2.0.4 - '@hono/node-server': - specifier: ^2.1.4 - version: 2.1.4(hono@4.13.12) '@hono/zod-validator': specifier: ^0.9.1 version: 0.9.1(hono@4.13.12)(zod@4.6.5) @@ -1469,12 +1466,6 @@ packages: '@hexagon/base64@2.0.4': resolution: {integrity: sha512-H/ZY6rGyaEuk0mwQgZ3BVi9hMjFTYpBNFbmtOuec/pPibuGhCMXd8fGtwBaO0h44FkWMurysMsDrpkJsBRmoWQ==} - '@hono/node-server@2.1.4': - resolution: {integrity: sha512-2g1qeHl4BduwDn6L8nkAvDrPUWPMlkQ//a5NhP/zIyaXKMjZNRV+cG9Csu+BiJFf6+68pqzjf4Hi5sJGKHuS1g==} - engines: {node: '>=20'} - peerDependencies: - hono: ^4 || ^5.0.0-0 - '@hono/zod-validator@0.9.1': resolution: {integrity: sha512-iiv6w0qrIc0arfvCtUqBWsvl4fXjzaTcQcCJTTtCnkawF9HHGE+KjEl0ox3gQJ6rKZEgE3mLExlQCF72M+mNuw==} peerDependencies: @@ -7955,10 +7946,6 @@ snapshots: '@hexagon/base64@2.0.4': {} - '@hono/node-server@2.1.4(hono@4.13.12)': - dependencies: - hono: 4.13.12 - '@hono/zod-validator@0.9.1(hono@4.13.12)(zod@4.6.5)': dependencies: hono: 4.13.12 diff --git a/src/http-server.test.ts b/src/http-server.test.ts index 44f4864d..de964241 100644 --- a/src/http-server.test.ts +++ b/src/http-server.test.ts @@ -11,12 +11,12 @@ import { verifyRequest, } from "@fedify/fedify/sig"; import { CryptographicKey } from "@fedify/vocab"; -import { serveStatic } from "@hono/node-server/serve-static"; import { Hono } from "hono"; import type { Server } from "srvx"; import { afterEach, describe, expect, it, vi } from "vitest"; import { createHttpServer } from "./http-server"; +import { serveStatic } from "./serve-static"; const servers: Server[] = []; @@ -373,7 +373,7 @@ describe("HTTP transport", () => { try { await writeFile(join(dir, "hello.txt"), "hello world"); const app = new Hono(); - app.use("/*", serveStatic({ root: dir })); + app.use("/*", serveStatic({ root: dir, prefix: "" })); const server = await start(app.fetch); const get = await send(server, { path: "/hello.txt" }); expect(get.body).toBe("hello world"); diff --git a/src/index.tsx b/src/index.tsx index 885c762a..1b3c0c1a 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -1,9 +1,7 @@ -import { join, relative } from "node:path"; +import { join } from "node:path"; import "./logging"; - import { federation } from "@fedify/hono"; -import { serveStatic } from "@hono/node-server/serve-static"; import { captureException } from "@sentry/core"; import { type Context, Hono, type Next } from "hono"; import { cors } from "hono/cors"; @@ -17,6 +15,7 @@ import oauth from "./oauth"; import oauthMetadataEndpoint from "./oauth/endpoints/metadata"; import pages from "./pages"; import proxy from "./proxy"; +import { serveStatic } from "./serve-static"; import { DRIVE_DISK, FS_STORAGE_PATH } from "./storage-config"; const app = new Hono(); @@ -36,8 +35,8 @@ if (DRIVE_DISK === "fs") { app.use( "/assets/*", serveStatic({ - root: relative(process.cwd(), FS_STORAGE_PATH!), - rewriteRequestPath: (path) => path.substring("/assets".length), + root: FS_STORAGE_PATH!, + prefix: "/assets", }), ); } @@ -45,8 +44,8 @@ if (DRIVE_DISK === "fs") { app.use( "/public/*", serveStatic({ - root: relative(process.cwd(), join(import.meta.dirname, "public")), - rewriteRequestPath: (path) => path.substring("/public".length), + root: join(import.meta.dirname, "public"), + prefix: "/public", }), ); diff --git a/src/serve-static.test.ts b/src/serve-static.test.ts new file mode 100644 index 00000000..86205059 --- /dev/null +++ b/src/serve-static.test.ts @@ -0,0 +1,294 @@ +import { + chmod, + mkdir, + mkdtemp, + readFile, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import { request as httpRequest, type IncomingHttpHeaders } from "node:http"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { text } from "node:stream/consumers"; + +import { Hono } from "hono"; +import type { Server } from "srvx"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { createHttpServer, enableFastResponse } from "./http-server"; +import { serveStatic } from "./serve-static"; + +let fixture: string; +let server: Server; +let exactFileReadable: boolean; +const nativeResponse = globalThis.Response; + +function send( + path: string, + { + method = "GET", + headers = {}, + }: { method?: string; headers?: Record } = {}, +): Promise<{ status: number; headers: IncomingHttpHeaders; body: string }> { + return new Promise((resolve, reject) => { + const request = httpRequest( + server.url!, + { path, method, headers, agent: false }, + (response) => { + text(response).then( + (body) => + resolve({ + status: response.statusCode!, + headers: response.headers, + body, + }), + reject, + ); + }, + ); + request.on("error", reject); + request.end(); + }); +} + +beforeAll(async () => { + enableFastResponse(); + fixture = await mkdtemp(join(tmpdir(), "hollo-static-")); + const root = join(fixture, "root"); + await mkdir(join(root, "nested"), { recursive: true }); + await writeFile(join(root, "hello.txt"), "0123456789"); + await writeFile(join(root, "index.html"), "

root

"); + await writeFile(join(root, "nested", "index.html"), "

nested

"); + await writeFile(join(root, ".secret"), "hidden"); + await writeFile(join(fixture, "outside.txt"), "outside secret"); + await symlink(join(fixture, "outside.txt"), join(root, "escape.txt")); + await symlink(join(root, "hello.txt"), join(root, "alias.txt")); + for (const name of [ + "movie.ogv", + "sound.oga", + "song.aac", + "font.ttf", + "style.css", + ]) { + await writeFile(join(root, name), "fixture"); + } + await mkdir(join(root, "collision")); + await writeFile(join(root, "collision.html"), "wrong alias"); + await writeFile(join(root, "collision", "index.html"), "directory index"); + await writeFile(join(root, "alias-only.html"), "unrequested alias"); + await mkdir(join(root, "v1.2.3")); + await writeFile(join(root, "v1.2.3", "index.html"), "dotted directory"); + await mkdir(join(root, "theme.css")); + await writeFile(join(root, "theme.css", "index.html"), "HTML index"); + await symlink(join(fixture, "outside.txt"), join(root, "escape")); + await writeFile(join(root, "escape.html"), "unwanted outside-link fallback"); + await symlink(join(root, ".secret"), join(root, "hidden-alias")); + await writeFile( + join(root, "hidden-alias.html"), + "unwanted hidden-link fallback", + ); + await writeFile(join(root, "large.txt"), "x".repeat(2048)); + await writeFile(join(root, "unreadable"), "exact file"); + await chmod(join(root, "unreadable"), 0); + exactFileReadable = await readFile(join(root, "unreadable")).then( + () => true, + () => false, + ); + await writeFile(join(root, "unreadable.html"), "wrong permission fallback"); + const app = new Hono(); + app.use("*", serveStatic({ root, prefix: "/files" })); + app.all("*", (c) => c.text("fallback", 404)); + server = createHttpServer(app.fetch.bind(app), { + port: 0, + hostname: "127.0.0.1", + behindProxy: false, + }); + await server.ready(); +}); + +afterAll(async () => { + await server?.close(true); + globalThis.Response = nativeResponse; + if (fixture) await rm(fixture, { recursive: true, force: true }); +}); + +describe("static files", () => { + it("prefers a directory index over an adjacent HTML file", async () => { + const response = await send("/files/collision"); + expect(response.status).toBe(200); + expect(response.body).toBe("directory index"); + }); + + it("does not add an HTML extension to a missing path", async () => { + const response = await send("/files/alias-only"); + expect(response.status).toBe(404); + expect(response.body).toBe("fallback"); + }); + + it("serves an index for dotted directory names without a trailing slash", async () => { + const response = await send("/files/v1.2.3"); + expect(response.status).toBe(200); + expect(response.body).toBe("dotted directory"); + }); + + it("uses the index file MIME type for directories named like files", async () => { + const response = await send("/files/theme.css"); + expect(response.status).toBe(200); + expect(response.body).toBe("HTML index"); + expect(response.headers["content-type"]).toBe("text/html; charset=utf-8"); + }); + + it.each(["escape", "hidden-alias"])( + "does not serve an HTML alias after rejecting %s", + async (name) => { + const response = await send(`/files/${name}`); + expect(response.status).toBe(404); + expect(response.body).toBe("fallback"); + }, + ); + + it("does not serve an HTML alias when the exact file cannot be read", async () => { + const response = await send("/files/unreadable"); + // Root and some platforms can read a mode-000 file; neither may select its alias. + expect(response.status).toBe(exactFileReadable ? 200 : 404); + expect(response.body).toBe(exactFileReadable ? "exact file" : "fallback"); + }); + + it("serves files without enabling compression or a cache lifetime", async () => { + const response = await send("/files/large.txt?download=true", { + headers: { "Accept-Encoding": "gzip, br" }, + }); + expect(response.status).toBe(200); + expect(response.body).toBe("x".repeat(2048)); + expect(response.headers["content-type"]).toBe("text/plain; charset=utf-8"); + expect(response.headers["content-length"]).toBe("2048"); + expect(response.headers["cache-control"]).toBeUndefined(); + expect(response.headers["content-encoding"]).toBeUndefined(); + }); + + it.each(["HEAD", "OPTIONS"])( + "preserves empty %s responses", + async (method) => { + const response = await send("/files/hello.txt", { method }); + expect(response.status).toBe(200); + expect(response.body).toBe(""); + expect(response.headers["content-length"]).toBe( + method === "HEAD" ? "10" : "0", + ); + }, + ); + + it("keeps serving files for POST as the previous helper did", async () => { + const response = await send("/files/hello.txt", { method: "POST" }); + expect(response.status).toBe(200); + expect(response.body).toBe("0123456789"); + }); + + it.each([ + ["bytes=2-4", "234", "bytes 2-4/10"], + ["bytes=-3", "789", "bytes 7-9/10"], + ["bytes=8-", "89", "bytes 8-9/10"], + ])("serves %s", async (range, body, contentRange) => { + const response = await send("/files/hello.txt", { + headers: { Range: range }, + }); + expect(response.status).toBe(206); + expect(response.body).toBe(body); + expect(response.headers["content-range"]).toBe(contentRange); + expect(response.headers["content-length"]).toBe(String(body.length)); + }); + + it("does not apply GET revalidation to legacy POST or OPTIONS", async () => { + const initial = await send("/files/hello.txt"); + for (const method of ["POST", "OPTIONS"]) { + const response = await send("/files/hello.txt", { + method, + headers: { "If-None-Match": initial.headers.etag! }, + }); + expect(response.status).toBe(200); + expect(response.body).toBe(method === "POST" ? "0123456789" : ""); + } + }); + + it("rejects an unsatisfiable range", async () => { + const response = await send("/files/hello.txt", { + headers: { Range: "bytes=20-" }, + }); + expect(response.status).toBe(416); + expect(response.headers["content-range"]).toBe("bytes */10"); + expect(response.body).toBe(""); + }); + + it("revalidates with ETag and Last-Modified", async () => { + const initial = await send("/files/hello.txt"); + expect(initial.headers.etag).toBeTypeOf("string"); + expect(initial.headers["last-modified"]).toBeTypeOf("string"); + const validators: Record[] = [ + { "If-None-Match": initial.headers.etag! }, + { "If-Modified-Since": initial.headers["last-modified"]! }, + ]; + for (const headers of validators) { + const cached = await send("/files/hello.txt", { headers }); + expect(cached.status).toBe(304); + expect(cached.body).toBe(""); + } + const stale = await send("/files/hello.txt", { + headers: { + Range: "bytes=2-4", + "If-Range": "Thu, 01 Jan 1970 00:00:00 GMT", + }, + }); + expect(stale.status).toBe(200); + expect(stale.body).toBe("0123456789"); + }); + + it.each([ + ["movie.ogv", "video/ogg"], + ["sound.oga", "audio/ogg"], + ["song.aac", "audio/aac"], + ["font.ttf", "font/ttf"], + ["style.css", "text/css; charset=utf-8"], + ])("preserves the MIME type of %s", async (name, type) => { + const response = await send(`/files/${name}`); + expect(response.status).toBe(200); + expect(response.headers["content-type"]).toBe(type); + expect(response.body).toBe("fixture"); + }); + + it.each(["/files", "/files/", "/files/nested/"])( + "serves an index at %s", + async (path) => { + const response = await send(path); + expect(response.status).toBe(200); + expect(response.headers["content-type"]).toBe("text/html; charset=utf-8"); + expect(response.body).toContain( + path.includes("nested") ? "nested" : "root", + ); + }, + ); + + it.each([ + "/files/missing.txt", + "/files/missing.oga", + "/files-other/hello.txt", + "/hello.txt", + "/files/../outside.txt", + "/files/%2e%2e/outside.txt", + "/files/%2F..%2Foutside.txt", + "/files/%", + "/files//hello.txt", + "/files/.secret", + "/files/escape.txt", + ])("falls through without exposing %s", async (path) => { + const response = await send(path); + expect(response.status).toBe(404); + expect(response.body).toBe("fallback"); + }); + + it("allows symlinks whose targets remain inside the root", async () => { + const response = await send("/files/alias.txt"); + expect(response.status).toBe(200); + expect(response.body).toBe("0123456789"); + }); +}); diff --git a/src/serve-static.ts b/src/serve-static.ts new file mode 100644 index 00000000..eddadedf --- /dev/null +++ b/src/serve-static.ts @@ -0,0 +1,102 @@ +import { constants } from "node:fs"; +import { access, realpath, stat } from "node:fs/promises"; +import { isAbsolute, join, relative, sep } from "node:path"; + +import type { MiddlewareHandler } from "hono"; +import { getMimeType } from "hono/utils/mime"; +import { staticMiddleware } from "srvx/static"; + +/** Mount srvx's asynchronous file server in Hono. */ +export function serveStatic({ + root, + prefix, +}: { + root: string; + prefix: string; +}): MiddlewareHandler { + const serve = staticMiddleware({ + dir: root, + compress: false, + encodings: false, + dotfiles: false, + }); + let realRoot: string | undefined; + return async (c, next) => { + if (c.finalized) return next(); + const path = c.req.path; + // Preserve the previous helper's rejection of encoded/ambiguous paths. + if ( + (path !== prefix && !path.startsWith(`${prefix}/`)) || + path.includes("%") || + /(?:^|[/\\])\.{1,2}(?:$|[/\\])|[/\\]{2,}|\\/.test(path) + ) { + return next(); + } + const url = new URL(c.req.url); + let filename = path.slice(prefix.length) || "/"; + // Preserve exact-file and directory-index lookup. srvx also tries .html + // aliases, which can select a different resource for extensionless paths. + const info = await stat(join(root, filename)).catch(() => undefined); + if (!info) return next(); + if (info.isDirectory()) { + filename = `${filename.replace(/\/+$/, "")}/index.html`; + } else if (!info.isFile()) { + return next(); + } + // Reject an unservable exact path before srvx can try an adjacent .html + // alias. srvx still validates the opened file against its root and inode. + realRoot ??= await realpath(root).catch(() => undefined); + if (!realRoot) return next(); + const realFile = await realpath(join(root, filename)).catch( + () => undefined, + ); + if (!realFile) return next(); + const relativeFile = relative(realRoot, realFile); + if ( + !relativeFile || + isAbsolute(relativeFile) || + relativeFile.split(sep).some((part) => part.startsWith(".")) + ) { + return next(); + } + if ( + !(await access(realFile, constants.R_OK).then( + () => true, + () => false, + )) + ) { + return next(); + } + url.pathname = filename; + // The previous helper served files for all methods, with an empty body + // for HEAD and OPTIONS. Never clone or consume an incoming upload body. + const request = new Request(url, { + method: + c.req.method === "HEAD" || c.req.method === "OPTIONS" ? "HEAD" : "GET", + headers: c.req.raw.headers, + signal: c.req.raw.signal, + }); + if (c.req.method !== "GET" && c.req.method !== "HEAD") { + // Conditional retrieval applies to GET/HEAD, not legacy file POSTs or OPTIONS. + request.headers.delete("If-None-Match"); + request.headers.delete("If-Modified-Since"); + request.headers.delete("If-Range"); + } + let found = true; + const response = await serve(request, async () => { + found = false; + await next(); + return c.res; + }); + if (found && response.headers.has("Content-Type")) { + // srvx has a small MIME table. Retain Hono's full media/font mappings. + const type = getMimeType(filename); + if (type) response.headers.set("Content-Type", type); + } + if (found && c.req.method === "OPTIONS" && response.status === 200) { + // OPTIONS has an empty response body, rather than HEAD's representation length. + response.headers.set("Content-Length", "0"); + } + return response; + }; +} From 783460fe70a5763833cd6732321d9f7b323578dd Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 18 Oct 2026 02:24:25 +0900 Subject: [PATCH 4/4] Document HTTP lifecycle and test helper contracts Explain startup sequencing, shutdown idempotency and concurrent resource cleanup. Describe why transport tests preserve raw request paths rather than normalizing them through the URL API. https://github.com/fedify-dev/hollo/pull/673#issuecomment-6237316047 Assisted-by: Codex:gpt-6.1-sol Assisted-by: Claude Code:claude-fable-5-1 Changelog: none --- bin/server.ts | 1 + src/http-server.test.ts | 2 ++ src/serve-static.test.ts | 1 + src/server-lifecycle.ts | 6 ++++++ 4 files changed, 10 insertions(+) diff --git a/bin/server.ts b/bin/server.ts index 685460ef..50e2e268 100644 --- a/bin/server.ts +++ b/bin/server.ts @@ -114,6 +114,7 @@ const lifecycle = createServerLifecycle([ let exitCode = 0; let shutdownPromise: Promise | undefined; +/** Stop acquired resources once, forcing exit if cleanup exceeds 30 seconds. */ function shutdown(code = 0): Promise { exitCode = Math.max(exitCode, code); if (shutdownPromise) return shutdownPromise; diff --git a/src/http-server.test.ts b/src/http-server.test.ts index de964241..01295553 100644 --- a/src/http-server.test.ts +++ b/src/http-server.test.ts @@ -20,6 +20,7 @@ import { serveStatic } from "./serve-static"; const servers: Server[] = []; +/** Start a tracked server on an ephemeral port for transport tests. */ async function start( handler: (request: Request) => Response | Promise, behindProxy = false, @@ -35,6 +36,7 @@ async function start( return server; } +/** Send a raw HTTP request without URL normalization or connection pooling. */ function send( server: Server, { diff --git a/src/serve-static.test.ts b/src/serve-static.test.ts index 86205059..76b8d040 100644 --- a/src/serve-static.test.ts +++ b/src/serve-static.test.ts @@ -24,6 +24,7 @@ let server: Server; let exactFileReadable: boolean; const nativeResponse = globalThis.Response; +/** Preserve the raw URL path for traversal and encoding tests at the server boundary. */ function send( path: string, { diff --git a/src/server-lifecycle.ts b/src/server-lifecycle.ts index 6be9df8c..b50282fc 100644 --- a/src/server-lifecycle.ts +++ b/src/server-lifecycle.ts @@ -9,9 +9,11 @@ export function createServerLifecycle(starters: Start[]) { let shutdown: Promise | undefined; return { + /** Whether shutdown has been requested, including during startup. */ get stopping() { return stopping; }, + /** Run startup phases once in order, skipping remaining phases after `stop()`. */ start(): Promise { return (startup ??= (async () => { for (const start of starters) { @@ -20,6 +22,10 @@ export function createServerLifecycle(starters: Start[]) { } })()); }, + /** + * Wait for pending startup, then run every registered cleanup concurrently. + * Repeated calls share a promise; cleanup failures reject with `AggregateError`. + */ stop(): Promise { stopping = true; return (shutdown ??= (async () => {