Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion image-cdn/src/handler/image.ts
Original file line number Diff line number Diff line change
@@ -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<Response> => {
const pathRegex = new RegExp(/^\/images\/(google_drive)\/(small|large|full)\/(.+)\.jpg$/);
Expand Down Expand Up @@ -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}`);
}
Expand Down
30 changes: 30 additions & 0 deletions image-cdn/src/utils.ts
Original file line number Diff line number Diff line change
@@ -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<Response> => {
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`);
};
18 changes: 18 additions & 0 deletions image-cdn/tests/handler/image.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof setTimeout>;
}) 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(
Expand Down
70 changes: 70 additions & 0 deletions image-cdn/tests/utils.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof setTimeout>;
}) 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<typeof setTimeout>;
}) 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<typeof setTimeout>;
}) 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);
});
});
1 change: 1 addition & 0 deletions image-cdn/worker-configuration.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 14 additions & 0 deletions image-cdn/wrangler.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Loading