From 1ebfa066a501bd097d6d1d1a6e9b47fac4fa36aa Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 18 Jul 2026 08:32:25 +0100 Subject: [PATCH] fix(synology-chat): bound outgoing response deadlines (#110520) Co-authored-by: Pick-cat --- .../synology-chat/src/client.loopback.test.ts | 52 ++++++++++++++++++- extensions/synology-chat/src/client.ts | 38 +++++++++++--- 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/extensions/synology-chat/src/client.loopback.test.ts b/extensions/synology-chat/src/client.loopback.test.ts index bf3959825254..6449cb6cc4f4 100644 --- a/extensions/synology-chat/src/client.loopback.test.ts +++ b/extensions/synology-chat/src/client.loopback.test.ts @@ -1,7 +1,7 @@ import { once } from "node:events"; import * as http from "node:http"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { resolveLegacyWebhookNameToChatUserId } from "./client.js"; +import { resolveLegacyWebhookNameToChatUserId, sendMessage } from "./client.js"; const USER_LIST_RESPONSE_MAX_BYTES = 1 * 1024 * 1024; @@ -149,4 +149,54 @@ describe("Synology Chat user_list loopback", () => { expect(elapsedMs).toBeGreaterThanOrEqual(timeoutMs - 50); expect(elapsedMs).toBeLessThan(timeoutMs + 1_500); }); + + it("bounds a dripping chatbot response with a wall-clock deadline", async () => { + let requestCount = 0; + server = http.createServer((_req, res) => { + requestCount += 1; + res.on("error", () => {}); + res.writeHead(200, { + "Content-Type": "application/json", + "Transfer-Encoding": "chunked", + }); + const dripTimer = setInterval(() => { + if (!res.writableEnded && !res.destroyed) { + res.write("x"); + } + }, 20); + res.on("close", () => clearInterval(dripTimer)); + res.write("x"); + }); + server.on("clientError", (_err, socket) => socket.destroy()); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("expected loopback server address"); + } + const incomingUrl = `http://127.0.0.1:${address.port}/webapi/entry.cgi`; + const timeoutMs = 250; + const nativeSetTimeout = globalThis.setTimeout; + const timeoutSpy = vi.spyOn(globalThis, "setTimeout"); + timeoutSpy.mockImplementation((( + callback: (...args: unknown[]) => void, + delay?: number, + ...args: unknown[] + ) => + nativeSetTimeout( + callback, + delay === 30_000 ? timeoutMs : delay, + ...args, + )) as typeof setTimeout); + + const startedAt = performance.now(); + await expect(sendMessage(incomingUrl, "hello")).resolves.toBe(false); + const elapsedMs = performance.now() - startedAt; + + expect(requestCount).toBe(3); + expect(timeoutSpy).toHaveBeenCalledWith(expect.any(Function), 30_000); + expect(elapsedMs).toBeGreaterThanOrEqual(timeoutMs * 3 - 100); + expect(elapsedMs).toBeLessThan(3_500); + }); }); diff --git a/extensions/synology-chat/src/client.ts b/extensions/synology-chat/src/client.ts index 29bf0a39b4e8..ae9a49326a78 100644 --- a/extensions/synology-chat/src/client.ts +++ b/extensions/synology-chat/src/client.ts @@ -21,6 +21,8 @@ const MIN_SEND_INTERVAL_MS = 500; const USER_LIST_RESPONSE_MAX_BYTES = 1 * 1024 * 1024; /** Wall-clock budget for user_list fetch including response body. */ const USER_LIST_REQUEST_TIMEOUT_MS = 15_000; +/** Wall-clock budget for outgoing webhook requests including response body. */ +const POST_REQUEST_TIMEOUT_MS = 30_000; let lastSendTime = 0; let sendQueue: Promise = Promise.resolve(); @@ -335,6 +337,24 @@ function parseNumericUserId(userId?: string | number): number | undefined { function doPost(url: string, body: string, allowInsecureSsl = false): Promise { return new Promise((resolve, reject) => { + let settled = false; + let response: http.IncomingMessage | undefined; + let deadlineTimer: ReturnType | undefined; + const finish = (result: { ok?: boolean; error?: Error }) => { + if (settled) { + return; + } + settled = true; + if (deadlineTimer !== undefined) { + clearTimeout(deadlineTimer); + deadlineTimer = undefined; + } + if (result.error) { + reject(result.error); + return; + } + resolve(result.ok === true); + }; let parsedUrl: URL; try { parsedUrl = new URL(url); @@ -352,24 +372,30 @@ function doPost(url: string, body: string, allowInsecureSsl = false): Promise { + response = res; res.on("end", () => { - resolve(res.statusCode === 200); + finish({ ok: res.statusCode === 200 }); }); + res.on("error", (error) => finish({ error })); res.resume(); }, ); - req.on("error", reject); - req.on("timeout", () => { + req.on("error", (error) => finish({ error })); + // ClientRequest timeout is socket-idle based. Keep one absolute budget + // across connect, upload, and response drain so trickling bodies terminate. + deadlineTimer = setTimeout(() => { + const error = new Error("Request timeout"); + finish({ error }); + response?.destroy(); req.destroy(); - reject(new Error("Request timeout")); - }); + }, POST_REQUEST_TIMEOUT_MS); + deadlineTimer.unref?.(); req.write(body); req.end(); });