fix(BUG-101): enforce route-specific body size limits
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 12m40s
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 12m40s
Remove global express.json({ limit: '2mb' }) that preempted route-specific
parsers. Each route group now has its own express.json() with correct limit:
- Demo: 50KB, Convert: 500KB, Others: 2MB, Stripe webhook: unchanged
This commit is contained in:
parent
d2f819de94
commit
c03f217690
2 changed files with 111 additions and 5 deletions
103
src/__tests__/body-limits.test.ts
Normal file
103
src/__tests__/body-limits.test.ts
Normal file
|
|
@ -0,0 +1,103 @@
|
||||||
|
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||||
|
import express from "express";
|
||||||
|
import request from "supertest";
|
||||||
|
|
||||||
|
vi.mock("../services/browser.js", () => ({
|
||||||
|
renderPdf: vi.fn(),
|
||||||
|
renderUrlPdf: vi.fn(),
|
||||||
|
initBrowser: vi.fn(),
|
||||||
|
closeBrowser: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../services/keys.js", () => ({
|
||||||
|
loadKeys: vi.fn(),
|
||||||
|
getAllKeys: vi.fn().mockReturnValue([]),
|
||||||
|
keyStore: new Map(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../services/db.js", () => ({
|
||||||
|
initDatabase: vi.fn(),
|
||||||
|
pool: { query: vi.fn(), end: vi.fn() },
|
||||||
|
cleanupStaleData: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../services/verification.js", () => ({
|
||||||
|
verifyToken: vi.fn(),
|
||||||
|
loadVerifications: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../middleware/usage.js", () => ({
|
||||||
|
usageMiddleware: (_req: any, _res: any, next: any) => next(),
|
||||||
|
loadUsageData: vi.fn(),
|
||||||
|
getUsageStats: vi.fn().mockReturnValue({}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../middleware/pdfRateLimit.js", () => ({
|
||||||
|
pdfRateLimitMiddleware: (_req: any, _res: any, next: any) => next(),
|
||||||
|
getConcurrencyStats: vi.fn().mockReturnValue({}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../middleware/auth.js", () => ({
|
||||||
|
authMiddleware: (req: any, _res: any, next: any) => {
|
||||||
|
req.apiKeyInfo = { key: "test-key", tier: "pro" };
|
||||||
|
next();
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe("Body size limits", () => {
|
||||||
|
let app: express.Express;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
vi.resetModules();
|
||||||
|
|
||||||
|
const { renderPdf } = await import("../services/browser.js");
|
||||||
|
vi.mocked(renderPdf).mockResolvedValue(Buffer.from("%PDF-1.4 mock"));
|
||||||
|
|
||||||
|
const { demoRouter } = await import("../routes/demo.js");
|
||||||
|
const { convertRouter } = await import("../routes/convert.js");
|
||||||
|
|
||||||
|
app = express();
|
||||||
|
|
||||||
|
// Simulate the production middleware setup:
|
||||||
|
// Route-specific parsers BEFORE global parser
|
||||||
|
app.use("/v1/demo", express.json({ limit: "50kb" }), demoRouter);
|
||||||
|
app.use("/v1/convert", express.json({ limit: "500kb" }), convertRouter);
|
||||||
|
// No global express.json() — that's the fix
|
||||||
|
});
|
||||||
|
|
||||||
|
it("demo rejects payloads > 50KB with 413", async () => {
|
||||||
|
const bigHtml = "x".repeat(51 * 1024); // ~51KB
|
||||||
|
const res = await request(app)
|
||||||
|
.post("/v1/demo/html")
|
||||||
|
.set("content-type", "application/json")
|
||||||
|
.send(JSON.stringify({ html: bigHtml }));
|
||||||
|
expect(res.status).toBe(413);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("demo accepts payloads < 50KB", async () => {
|
||||||
|
const res = await request(app)
|
||||||
|
.post("/v1/demo/html")
|
||||||
|
.set("content-type", "application/json")
|
||||||
|
.send({ html: "<h1>Hello</h1>" });
|
||||||
|
expect([200, 400]).not.toContain(413);
|
||||||
|
expect(res.status).not.toBe(413);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("convert rejects payloads > 500KB with 413", async () => {
|
||||||
|
const bigHtml = "x".repeat(501 * 1024);
|
||||||
|
const res = await request(app)
|
||||||
|
.post("/v1/convert/html")
|
||||||
|
.set("content-type", "application/json")
|
||||||
|
.send(JSON.stringify({ html: bigHtml }));
|
||||||
|
expect(res.status).toBe(413);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("convert accepts payloads < 500KB", async () => {
|
||||||
|
const res = await request(app)
|
||||||
|
.post("/v1/convert/html")
|
||||||
|
.set("content-type", "application/json")
|
||||||
|
.send({ html: "<h1>Hello</h1>" });
|
||||||
|
expect(res.status).not.toBe(413);
|
||||||
|
});
|
||||||
|
});
|
||||||
13
src/index.ts
13
src/index.ts
|
|
@ -82,7 +82,8 @@ app.use((req, res, next) => {
|
||||||
|
|
||||||
// Raw body for Stripe webhook signature verification
|
// Raw body for Stripe webhook signature verification
|
||||||
app.use("/v1/billing/webhook", express.raw({ type: "application/json" }));
|
app.use("/v1/billing/webhook", express.raw({ type: "application/json" }));
|
||||||
app.use(express.json({ limit: "2mb" }));
|
// NOTE: No global express.json() here — route-specific parsers are applied
|
||||||
|
// per-route below to enforce correct body size limits (BUG-101 fix).
|
||||||
app.use(express.text({ limit: "2mb", type: "text/*" }));
|
app.use(express.text({ limit: "2mb", type: "text/*" }));
|
||||||
|
|
||||||
// Trust nginx proxy
|
// Trust nginx proxy
|
||||||
|
|
@ -130,14 +131,16 @@ app.use("/v1/signup", (_req, res) => {
|
||||||
pro_url: "https://docfast.dev/#pricing"
|
pro_url: "https://docfast.dev/#pricing"
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
app.use("/v1/recover", recoverRouter);
|
// Default 2MB JSON parser for standard routes
|
||||||
app.use("/v1/email-change", emailChangeRouter);
|
const defaultJsonParser = express.json({ limit: "2mb" });
|
||||||
app.use("/v1/billing", billingRouter);
|
app.use("/v1/recover", defaultJsonParser, recoverRouter);
|
||||||
|
app.use("/v1/email-change", defaultJsonParser, emailChangeRouter);
|
||||||
|
app.use("/v1/billing", defaultJsonParser, billingRouter);
|
||||||
|
|
||||||
// Authenticated routes — conversion routes get tighter body limits (500KB)
|
// Authenticated routes — conversion routes get tighter body limits (500KB)
|
||||||
const convertBodyLimit = express.json({ limit: "500kb" });
|
const convertBodyLimit = express.json({ limit: "500kb" });
|
||||||
app.use("/v1/convert", convertBodyLimit, authMiddleware, usageMiddleware, pdfRateLimitMiddleware, convertRouter);
|
app.use("/v1/convert", convertBodyLimit, authMiddleware, usageMiddleware, pdfRateLimitMiddleware, convertRouter);
|
||||||
app.use("/v1/templates", authMiddleware, usageMiddleware, templatesRouter);
|
app.use("/v1/templates", defaultJsonParser, authMiddleware, usageMiddleware, templatesRouter);
|
||||||
|
|
||||||
// Admin: usage stats (admin key required)
|
// Admin: usage stats (admin key required)
|
||||||
const adminAuth = (req: any, res: any, next: any) => {
|
const adminAuth = (req: any, res: any, next: any) => {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue