From f95b974c7ec948c0288897f59205ed0e39817e73 Mon Sep 17 00:00:00 2001 From: Dmytro Stanchiev Date: Wed, 29 Apr 2026 21:09:10 -0400 Subject: [PATCH] fix: harden shared http helper --- packages/core/src/utils/http.ts | 70 ++++++++++++++++++++++++++------- packages/core/test/http.test.ts | 64 ++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 14 deletions(-) diff --git a/packages/core/src/utils/http.ts b/packages/core/src/utils/http.ts index 54d4f24..ad4d600 100644 --- a/packages/core/src/utils/http.ts +++ b/packages/core/src/utils/http.ts @@ -71,6 +71,43 @@ function calculateBackoffDelay( return Math.min(exponentialDelay + jitterDelay, 30000); // Cap at 30 seconds } +const MAX_RATE_LIMIT_WAIT_MS = 30_000; +const MAX_DELTA_RESET_SECONDS = 86_400; + +function mergeHeaders( + defaultHeaders: Record, + customHeaders?: Record, +): Record { + const merged: Record = {}; + + for (const [key, value] of Object.entries(defaultHeaders)) { + merged[key.toLowerCase()] = value; + } + + for (const [key, value] of Object.entries(customHeaders ?? {})) { + merged[key.toLowerCase()] = value; + } + + return merged; +} + +function calculateRateLimitWaitMs( + resetHeader: string | null, + fallbackWaitMs: number, +): number { + if (!resetHeader) return fallbackWaitMs; + + const resetValue = Number(resetHeader); + if (!Number.isFinite(resetValue)) return fallbackWaitMs; + + const waitMs = + resetValue <= MAX_DELTA_RESET_SECONDS + ? resetValue * 1000 + : resetValue * 1000 - Date.now(); + + return Math.min(Math.max(0, waitMs), MAX_RATE_LIMIT_WAIT_MS); +} + /** Result type when includeResponseUrl is true */ export interface FetchHtmlResult { html: HTMLString; @@ -141,13 +178,17 @@ export async function fetchHtml( const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), timeoutMs); - const res = await fetch(url, { - method: "GET", - headers: { ...defaultHeaders, ...opts?.headers }, - signal: controller.signal, - }); - - clearTimeout(timeoutId); + const res = await (async () => { + try { + return await fetch(url, { + method: "GET", + headers: mergeHeaders(defaultHeaders, opts?.headers), + signal: controller.signal, + }); + } finally { + clearTimeout(timeoutId); + } + })(); const rateLimitRemaining = res.headers.get("X-RateLimit-Remaining"); const rateLimitReset = res.headers.get("X-RateLimit-Reset"); @@ -159,13 +200,14 @@ export async function fetchHtml( const resetSeconds = rateLimitReset ? Number(rateLimitReset) : Number.NaN; - const waitMs = Number.isFinite(resetSeconds) - ? Math.max(0, resetSeconds * 1000) - : calculateBackoffDelay( - attempt, - retryBaseMs, - opts?.jitter ?? Math.random, - ); + const waitMs = calculateRateLimitWaitMs( + rateLimitReset, + calculateBackoffDelay( + attempt, + retryBaseMs, + opts?.jitter ?? Math.random, + ), + ); if (attempt < maxRetries) { await delay(waitMs); diff --git a/packages/core/test/http.test.ts b/packages/core/test/http.test.ts index 828c01b..f385ebc 100644 --- a/packages/core/test/http.test.ts +++ b/packages/core/test/http.test.ts @@ -57,4 +57,68 @@ describe("fetchHtml", () => { expect(result.html).toBe(""); expect(result.responseUrl).toBe("https://example.test/final"); }); + + test("rate limit epoch reset uses bounded wait", async () => { + process.env.NODE_ENV = "production"; + const scheduledDelays: number[] = []; + const farFutureEpochSeconds = Math.floor(Date.now() / 1000) + 315_360_000; + let calls = 0; + + global.fetch = mock(() => { + calls += 1; + return Promise.resolve({ + ok: calls > 1, + status: calls > 1 ? 200 : 429, + url: "https://example.test", + headers: { + get: (name: string) => + name === "X-RateLimit-Reset" ? String(farFutureEpochSeconds) : null, + }, + text: () => Promise.resolve(""), + }); + }) as unknown as typeof fetch; + globalThis.setTimeout = mock((handler: TimerHandler, timeout?: number) => { + scheduledDelays.push(Number(timeout)); + if (timeout !== 1_234_567 && typeof handler === "function") { + handler(); + } + return 0 as unknown as ReturnType; + }) as unknown as typeof setTimeout; + globalThis.clearTimeout = mock(() => {}) as unknown as typeof clearTimeout; + + await fetchHtml("https://example.test", 0, { + maxRetries: 1, + timeoutMs: 1_234_567, + }); + + expect(scheduledDelays).toContain(30_000); + expect(scheduledDelays).not.toContain(farFutureEpochSeconds * 1000); + }); + + test("custom Accept header overrides default accept without duplicate casing", async () => { + process.env.NODE_ENV = "test"; + const customAccept = "text/plain"; + let requestHeaders: HeadersInit | undefined; + + global.fetch = mock((_url: string | URL | Request, init?: RequestInit) => { + requestHeaders = init?.headers; + return Promise.resolve({ + ok: true, + status: 200, + url: "https://example.test", + headers: { get: () => null }, + text: () => Promise.resolve(""), + }); + }) as unknown as typeof fetch; + + await fetchHtml("https://example.test", 0, { + headers: { Accept: customAccept }, + }); + + expect(requestHeaders).toBeDefined(); + expect((requestHeaders as Record).accept).toBe( + customAccept, + ); + expect((requestHeaders as Record).Accept).toBeUndefined(); + }); });