From afab8175f9177ec731f972b9f676b8ef8619a9a9 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 10 Apr 2026 10:06:19 +0200 Subject: [PATCH] feat(24-01): refactor rateLimit to factory pattern with createRateLimit - Add createRateLimit(maxAttempts, windowMs) factory function - Rewrite rateLimit export to delegate to factory (backward compatible) - Keep shared store, getClientIp, cleanup, and _resetForTesting unchanged - Add createRateLimit factory test suite with 5 test cases - All existing rateLimit middleware tests still pass --- src/server/middleware/rateLimit.ts | 44 +++++++------- tests/middleware/rateLimit.test.ts | 95 ++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 24 deletions(-) diff --git a/src/server/middleware/rateLimit.ts b/src/server/middleware/rateLimit.ts index 6c861fe..70b2b89 100644 --- a/src/server/middleware/rateLimit.ts +++ b/src/server/middleware/rateLimit.ts @@ -7,9 +7,6 @@ interface RateLimitEntry { const store = new Map(); -const MAX_ATTEMPTS = 5; -const WINDOW_MS = 15 * 60 * 1000; // 15 minutes - function getClientIp(c: Context): string { return c.req.header("x-forwarded-for")?.split(",")[0]?.trim() || "unknown"; } @@ -23,30 +20,29 @@ function cleanup() { } } -export async function rateLimit(c: Context, next: Next) { - cleanup(); - - const ip = getClientIp(c); - const key = `${ip}:${c.req.path}`; - const now = Date.now(); - - const entry = store.get(key); - - if (!entry || now >= entry.resetAt) { - store.set(key, { count: 1, resetAt: now + WINDOW_MS }); +export function createRateLimit(maxAttempts: number, windowMs: number) { + return async function rateLimitMiddleware(c: Context, next: Next) { + cleanup(); + const ip = getClientIp(c); + const key = `${ip}:${c.req.path}`; + const now = Date.now(); + const entry = store.get(key); + if (!entry || now >= entry.resetAt) { + store.set(key, { count: 1, resetAt: now + windowMs }); + return next(); + } + if (entry.count >= maxAttempts) { + const retryAfter = Math.ceil((entry.resetAt - now) / 1000); + c.header("Retry-After", String(retryAfter)); + return c.json({ error: "Too many attempts. Try again later." }, 429); + } + entry.count++; return next(); - } - - if (entry.count >= MAX_ATTEMPTS) { - const retryAfter = Math.ceil((entry.resetAt - now) / 1000); - c.header("Retry-After", String(retryAfter)); - return c.json({ error: "Too many attempts. Try again later." }, 429); - } - - entry.count++; - return next(); + }; } +export const rateLimit = createRateLimit(5, 15 * 60 * 1000); + /** @internal — only for testing */ export function _resetForTesting() { store.clear(); diff --git a/tests/middleware/rateLimit.test.ts b/tests/middleware/rateLimit.test.ts index bec6348..07884a0 100644 --- a/tests/middleware/rateLimit.test.ts +++ b/tests/middleware/rateLimit.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it } from "bun:test"; import { Hono } from "hono"; import { _resetForTesting, + createRateLimit, rateLimit, } from "../../src/server/middleware/rateLimit"; @@ -19,6 +20,13 @@ function makeRequest(app: Hono, path: string, ip = "127.0.0.1") { }); } +function makeGetRequest(app: Hono, path: string, ip = "127.0.0.1") { + return app.request(path, { + method: "GET", + headers: { "x-forwarded-for": ip }, + }); +} + describe("rateLimit middleware", () => { beforeEach(() => { _resetForTesting(); @@ -83,3 +91,90 @@ describe("rateLimit middleware", () => { expect(allowedSetup.status).toBe(200); }); }); + +describe("createRateLimit factory", () => { + beforeEach(() => { + _resetForTesting(); + }); + + it("blocks on 4th request when limit is 3", async () => { + const limit3 = createRateLimit(3, 60_000); + const app = new Hono(); + app.get("/items", limit3, (c) => c.json({ ok: true })); + + for (let i = 0; i < 3; i++) { + const res = await makeGetRequest(app, "/items"); + expect(res.status).toBe(200); + } + const res = await makeGetRequest(app, "/items"); + expect(res.status).toBe(429); + }); + + it("allows exactly 10 requests then blocks on 11th", async () => { + const limit10 = createRateLimit(10, 60_000); + const app = new Hono(); + app.get("/catalog", limit10, (c) => c.json({ ok: true })); + + for (let i = 0; i < 10; i++) { + const res = await makeGetRequest(app, "/catalog"); + expect(res.status).toBe(200); + } + const res = await makeGetRequest(app, "/catalog"); + expect(res.status).toBe(429); + }); + + it("tracks different IPs independently", async () => { + const limit3 = createRateLimit(3, 60_000); + const app = new Hono(); + app.get("/items", limit3, (c) => c.json({ ok: true })); + + for (let i = 0; i < 3; i++) { + await makeGetRequest(app, "/items", "192.168.1.1"); + } + const blocked = await makeGetRequest(app, "/items", "192.168.1.1"); + expect(blocked.status).toBe(429); + + // Different IP should still be allowed + const allowed = await makeGetRequest(app, "/items", "192.168.1.2"); + expect(allowed.status).toBe(200); + }); + + it("includes Retry-After header on 429 response", async () => { + const limit2 = createRateLimit(2, 60_000); + const app = new Hono(); + app.get("/tags", limit2, (c) => c.json({ ok: true })); + + await makeGetRequest(app, "/tags"); + await makeGetRequest(app, "/tags"); + const res = await makeGetRequest(app, "/tags"); + + expect(res.status).toBe(429); + const retryAfter = res.headers.get("Retry-After"); + expect(retryAfter).toBeTruthy(); + expect(Number(retryAfter)).toBeGreaterThan(0); + }); + + it("two instances with different limits operate independently", async () => { + const limit3 = createRateLimit(3, 60_000); + const limit5 = createRateLimit(5, 60_000); + + const app = new Hono(); + app.get("/browse", limit3, (c) => c.json({ ok: true })); + app.get("/detail", limit5, (c) => c.json({ ok: true })); + + // Exhaust browse limit (3) + for (let i = 0; i < 3; i++) { + await makeGetRequest(app, "/browse"); + } + const blockedBrowse = await makeGetRequest(app, "/browse"); + expect(blockedBrowse.status).toBe(429); + + // Detail endpoint still has its own limit (5) and should work + for (let i = 0; i < 5; i++) { + const res = await makeGetRequest(app, "/detail"); + expect(res.status).toBe(200); + } + const blockedDetail = await makeGetRequest(app, "/detail"); + expect(blockedDetail.status).toBe(429); + }); +});