From ecd56dc32e860696da4974e43feab7ace48abf97 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:51:32 +0000 Subject: [PATCH] Add rate limiter for lh4.googleusercontent.com full-tier image fetches Shared by PDF export, bulk download, and the local OCR/phash pilot - previously unguarded (GOOGLE_DRIVE_RATE_LIMITER only covers the real Drive API, a different Google domain this path never touches). --- image-cdn/src/handler/image.ts | 11 ++++- image-cdn/src/utils.ts | 30 ++++++++++++ image-cdn/tests/handler/image.test.ts | 18 +++++++ image-cdn/tests/utils.test.ts | 70 +++++++++++++++++++++++++++ image-cdn/worker-configuration.d.ts | 1 + image-cdn/wrangler.toml | 14 ++++++ 6 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 image-cdn/tests/utils.test.ts diff --git a/image-cdn/src/handler/image.ts b/image-cdn/src/handler/image.ts index 4578939f6..48fdfaf64 100644 --- a/image-cdn/src/handler/image.ts +++ b/image-cdn/src/handler/image.ts @@ -1,6 +1,7 @@ import { R2Service } from "../service/R2Service"; import { ImageSize, ImageType } from "../types"; import { getImageURL } from "../url"; +import { fetchWithRateLimit } from "../utils"; export const handleImageRequest = async (url: URL, request: Request, env: Env, ctx: ExecutionContext): Promise => { const pathRegex = new RegExp(/^\/images\/(google_drive)\/(small|large|full)\/(.+)\.jpg$/); @@ -33,7 +34,15 @@ export const handleImageRequest = async (url: URL, request: Request, env: Env, c return R2Service.getThumbnail(env, ctx, getImageURL(imageType, imageSize, undefined, jpgQuality, imageIdentifier), imageKey); case "full": const url = getImageURL(imageType, imageSize, dpi, jpgQuality, imageIdentifier); - return fetch(url); + // full-tier bypasses R2 entirely (see R2Service - only small/large are cached), so + // EVERY request here hits lh4.googleusercontent.com directly. Shared by three + // callers: this handler, the PDF export path, and the bulk image download feature + // (both frontend/src/features/*) - none of which previously had any rate limiting on + // this specific Google endpoint (GOOGLE_DRIVE_RATE_LIMITER only guards the real Drive + // API in GoogleDriveService.executeCall, a different Google domain entirely). A + // sustained high-volume caller (e.g. an unattended backfill script) hammering this + // unguarded endpoint risked degrading it for live PDF export/download traffic too. + return fetchWithRateLimit(env.IMAGE_FULL_TIER_RATE_LIMITER, "global-image-full-tier-rate-limit", url); default: throw new Error(`Invalid image size ${imageSize}`); } diff --git a/image-cdn/src/utils.ts b/image-cdn/src/utils.ts index fef0b28a6..6dce4e330 100644 --- a/image-cdn/src/utils.ts +++ b/image-cdn/src/utils.ts @@ -1,3 +1,33 @@ export const assertUnreachable = (x: never): never => { throw new Error(`Didn't expect to get here with ${x}`); }; + +export const MAX_RATE_LIMIT_RETRIES = 5; + +export const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +// Same shape as GoogleDriveService.executeCall's retry/backoff (Cloudflare limiter check, plus a +// defensive retry on an upstream 429 - "ideally this case is caught by cloudflare rate limiting +// for us" per that method's own comment), extracted as a standalone helper because this call +// site is a plain unauthenticated GET to a DIFFERENT Google domain (lh4.googleusercontent.com, +// the image-serving CDN) - not the Drive API executeCall is built around (POST, OAuth, JSON/batch +// bodies) - so reusing that method directly would be a worse fit than a small shared primitive. +export const fetchWithRateLimit = async (limiter: RateLimit, key: string, url: string): Promise => { + for (let attempt = 0; attempt <= MAX_RATE_LIMIT_RETRIES; attempt++) { + const { success } = await limiter.limit({ key }); + const backoff = 2 ** attempt + 1000 * Math.random(); + + if (success) { + const response = await fetch(url); + if (response.status === 429 && attempt < MAX_RATE_LIMIT_RETRIES) { + await delay(backoff); + continue; + } + return response; + } + + await delay(backoff); + } + + throw new Error(`Rate limit exceeded for key "${key}" after ${MAX_RATE_LIMIT_RETRIES} retries`); +}; diff --git a/image-cdn/tests/handler/image.test.ts b/image-cdn/tests/handler/image.test.ts index 64673e404..1e5e6e91d 100644 --- a/image-cdn/tests/handler/image.test.ts +++ b/image-cdn/tests/handler/image.test.ts @@ -117,6 +117,24 @@ describe("worker image routing", () => { } ); + it("retries the full-tier fetch when the image rate limiter denies the first attempt", async () => { + const fetchMock = vi.fn(async () => new Response("full-image")); + vi.stubGlobal("fetch", fetchMock); + const limit = vi.fn().mockResolvedValueOnce({ success: false }).mockResolvedValueOnce({ success: true }); + vi.spyOn(global, "setTimeout").mockImplementation(((cb: () => void) => { + cb(); + return 0 as unknown as ReturnType; + }) as typeof setTimeout); + + const request = new IncomingRequest("http://example.com/images/google_drive/full/image-id.jpg"); + const ctx = createExecutionContext(); + const response = await worker.fetch(request, { ...env, IMAGE_FULL_TIER_RATE_LIMITER: { limit } as unknown as RateLimit }, ctx); + await waitOnExecutionContext(ctx); + + expect(await response.text()).toBe("full-image"); + expect(limit).toHaveBeenCalledTimes(2); + }); + it("throws for invalid dpi or JPG quality query parameters", async () => { await expect(fetchWorker("http://example.com/images/google_drive/full/image-id.jpg?dpi=0")).rejects.toThrow("invalid DPI 0"); await expect(fetchWorker("http://example.com/images/google_drive/full/image-id.jpg?jpgQuality=101")).rejects.toThrow( diff --git a/image-cdn/tests/utils.test.ts b/image-cdn/tests/utils.test.ts new file mode 100644 index 000000000..70dccf494 --- /dev/null +++ b/image-cdn/tests/utils.test.ts @@ -0,0 +1,70 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { fetchWithRateLimit } from "../src/utils"; + +beforeEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe("fetchWithRateLimit", () => { + it("fetches immediately when the limiter allows the first attempt", async () => { + const limit = vi.fn(async () => ({ success: true })); + const fetchMock = vi.fn(async () => new Response("ok")); + vi.stubGlobal("fetch", fetchMock); + + const response = await fetchWithRateLimit({ limit } as unknown as RateLimit, "test-key", "https://example.com/image.jpg"); + + expect(await response.text()).toBe("ok"); + expect(limit).toHaveBeenCalledTimes(1); + expect(limit).toHaveBeenCalledWith({ key: "test-key" }); + expect(fetchMock).toHaveBeenCalledWith("https://example.com/image.jpg"); + }); + + it("backs off and retries when the limiter denies an attempt", async () => { + const limit = vi.fn().mockResolvedValueOnce({ success: false }).mockResolvedValueOnce({ success: true }); + const fetchMock = vi.fn(async () => new Response("ok")); + vi.stubGlobal("fetch", fetchMock); + vi.spyOn(global, "setTimeout").mockImplementation(((cb: () => void) => { + cb(); + return 0 as unknown as ReturnType; + }) as typeof setTimeout); + + const response = await fetchWithRateLimit({ limit } as unknown as RateLimit, "test-key", "https://example.com/image.jpg"); + + expect(await response.text()).toBe("ok"); + expect(limit).toHaveBeenCalledTimes(2); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("retries an upstream 429 before returning a successful response", async () => { + const limit = vi.fn(async () => ({ success: true })); + const fetchMock = vi + .fn() + .mockResolvedValueOnce(new Response("too many", { status: 429 })) + .mockResolvedValueOnce(new Response("ok")); + vi.stubGlobal("fetch", fetchMock); + vi.spyOn(global, "setTimeout").mockImplementation(((cb: () => void) => { + cb(); + return 0 as unknown as ReturnType; + }) as typeof setTimeout); + + const response = await fetchWithRateLimit({ limit } as unknown as RateLimit, "test-key", "https://example.com/image.jpg"); + + expect(await response.text()).toBe("ok"); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("throws after every attempt is rate limited", async () => { + const limit = vi.fn(async () => ({ success: false })); + vi.spyOn(global, "setTimeout").mockImplementation(((cb: () => void) => { + cb(); + return 0 as unknown as ReturnType; + }) as typeof setTimeout); + + await expect(fetchWithRateLimit({ limit } as unknown as RateLimit, "test-key", "https://example.com/image.jpg")).rejects.toThrow( + 'Rate limit exceeded for key "test-key" after 5 retries' + ); + expect(limit).toHaveBeenCalledTimes(6); + }); +}); diff --git a/image-cdn/worker-configuration.d.ts b/image-cdn/worker-configuration.d.ts index c6e34336f..27755603c 100644 --- a/image-cdn/worker-configuration.d.ts +++ b/image-cdn/worker-configuration.d.ts @@ -4,6 +4,7 @@ interface __BaseEnv_Env { thumbnails: R2Bucket; GOOGLE_DRIVE_RATE_LIMITER: RateLimit; + IMAGE_FULL_TIER_RATE_LIMITER: RateLimit; GOOGLE_CLIENT_ID: ""; GOOGLE_CLIENT_SECRET: string; GOOGLE_REFRESH_TOKEN: string; diff --git a/image-cdn/wrangler.toml b/image-cdn/wrangler.toml index af00c9836..b11c4992a 100644 --- a/image-cdn/wrangler.toml +++ b/image-cdn/wrangler.toml @@ -27,3 +27,17 @@ enabled = true name = "GOOGLE_DRIVE_RATE_LIMITER" namespace_id = "1001" simple = { limit = 12000, period = 10 } + +# Protects lh4.googleusercontent.com (the "full" tier's image-serving fetch, image.ts) - a +# DIFFERENT Google domain/endpoint than GOOGLE_DRIVE_RATE_LIMITER above (the real Drive API), +# with no published quota to size this against (unlike the Drive API's documented ~200 req/s - +# see cardpicker/sources/update_database.py's comment on that machine). Deliberately +# conservative (3 req/s sustained) since we're guessing at Google's real ceiling here: this is +# shared with live PDF export and bulk image download traffic (frontend/src/features/pdf/, +# frontend/src/features/download/), not just the local OCR/phash pilot that motivated adding it +# - a burst of "tens of cards" from either live feature clears in well under a second at this +# rate, while a sustained high-volume caller gets backed off instead of hammering the endpoint. +[[ratelimits]] +name = "IMAGE_FULL_TIER_RATE_LIMITER" +namespace_id = "1002" +simple = { limit = 30, period = 10 }