From f8f1629828f4c5be2516d52b3b84c2a180389cfe Mon Sep 17 00:00:00 2001 From: Leonardo Ortiz Date: Thu, 23 Apr 2026 18:29:23 -0300 Subject: [PATCH 1/2] fix(frameworks): prepend body-parser shim so strict parse failures reach the app Fixes #10404. Malformed application/json could return 500 on Hosting framework SSR while the same route returned 400 under next start; the shim only clears entity.parse.failed so the framework can still handle the request. - Add patchBodyParser, installBodyParserTolerance, and getBodyParserToleranceShim in utils; document PARSE_FAILED_TYPE against body-parser read.js - Prepend the shim to generated server.js for CJS and ESM; ESM path adds createRequire(import.meta.url) before the snapshot so require and require.cache match the shim - Use non-enumerable __bodyParserPassthroughApplied on body-parser exports for idempotency - Add express/supertest tests for #10404-style JSON failures, other parsers, limits, urlencoded depth, and shim string checks without eval under nyc --- src/frameworks/index.ts | 35 +++-- src/frameworks/utils.spec.ts | 262 ++++++++++++++++++++++++++++++++++- src/frameworks/utils.ts | 103 ++++++++++++++ 3 files changed, 387 insertions(+), 13 deletions(-) diff --git a/src/frameworks/index.ts b/src/frameworks/index.ts index d9fca0fe123..3558b9673c2 100644 --- a/src/frameworks/index.ts +++ b/src/frameworks/index.ts @@ -25,6 +25,7 @@ import { findDependency, conjoinOptions, frameworksCallToAction, + getBodyParserToleranceShim, getFrameworksBuildTarget, } from "./utils"; import { @@ -539,25 +540,35 @@ ${ // TODO move to templates + // Prepends getBodyParserToleranceShim() before framework imports; see + // https://github.com/firebase/firebase-tools/issues/10404 + // ESM needs createRequire first: the shim uses require() and require.cache. Same shim string + // for both branches; only import/export wrapping differs. + const bodyParserToleranceShim = getBodyParserToleranceShim(); + if (packageJson.type === "module") { await writeFile( join(functionsDist, "server.js"), - `import { onRequest } from 'firebase-functions/v2/https'; - const server = import('firebase-frameworks'); - export const ${functionId} = onRequest(${JSON.stringify( - frameworksBackend || {}, - )}, (req, res) => server.then(it => it.handle(req, res))); - `, + `import { createRequire } from 'module'; +const require = createRequire(import.meta.url); +${bodyParserToleranceShim} +const { onRequest } = require('firebase-functions/v2/https'); +const server = import('firebase-frameworks'); +export const ${functionId} = onRequest(${JSON.stringify( + frameworksBackend || {}, + )}, (req, res) => server.then(it => it.handle(req, res))); +`, ); } else { await writeFile( join(functionsDist, "server.js"), - `const { onRequest } = require('firebase-functions/v2/https'); - const server = import('firebase-frameworks'); - exports.${functionId} = onRequest(${JSON.stringify( - frameworksBackend || {}, - )}, (req, res) => server.then(it => it.handle(req, res))); - `, + `${bodyParserToleranceShim} +const { onRequest } = require('firebase-functions/v2/https'); +const server = import('firebase-frameworks'); +exports.${functionId} = onRequest(${JSON.stringify( + frameworksBackend || {}, + )}, (req, res) => server.then(it => it.handle(req, res))); +`, ); } } else { diff --git a/src/frameworks/utils.spec.ts b/src/frameworks/utils.spec.ts index ab424dfcb6f..f6cb1a75f2b 100644 --- a/src/frameworks/utils.spec.ts +++ b/src/frameworks/utils.spec.ts @@ -1,9 +1,20 @@ import { expect } from "chai"; import * as sinon from "sinon"; import * as fs from "fs"; +import * as express from "express"; +import * as supertest from "supertest"; import { resolve, join } from "path"; -import { warnIfCustomBuildScript, isUrl, getNodeModuleBin, conjoinOptions } from "./utils"; +import { + BodyParserExports, + conjoinOptions, + getBodyParserToleranceShim, + getNodeModuleBin, + installBodyParserTolerance, + isUrl, + patchBodyParser, + warnIfCustomBuildScript, +} from "./utils"; describe("Frameworks utils", () => { describe("getNodeModuleBin", () => { @@ -135,4 +146,253 @@ describe("Frameworks utils", () => { ); }); }); + + describe("body-parser tolerance", () => { + function loadFreshBodyParserModuleExports(): BodyParserExports { + for (const requireCacheKey of Object.keys(require.cache)) { + if (requireCacheKey.includes(`/node_modules/body-parser/`)) { + delete require.cache[requireCacheKey]; + } + } + return require("body-parser") as BodyParserExports; + } + + type RequestWithRawJsonBody = express.Request & { rawJsonBody?: Buffer }; + + function readRawJsonBody(req: express.Request): Buffer | undefined { + const candidate = (req as RequestWithRawJsonBody).rawJsonBody; + return Buffer.isBuffer(candidate) ? candidate : undefined; + } + + /** + * `json()` plus a POST handler shaped like hardened Next.js API routes in + * https://github.com/firebase/firebase-tools/issues/10404: `verify` keeps the wire bytes so the + * route can `JSON.parse` after tolerant `body-parser` skips strict failures. + */ + function buildApp(bodyParserExports: BodyParserExports): express.Express { + const app = express(); + app.use( + bodyParserExports.json!({ + verify: (req: express.Request, _res: express.Response, buf: Buffer) => { + (req as RequestWithRawJsonBody).rawJsonBody = Buffer.from(buf); + }, + }) as express.RequestHandler, + ); + app.post("/", (req, res) => { + const raw = readRawJsonBody(req); + if (raw !== undefined && raw.length > 0) { + try { + JSON.parse(raw.toString("utf8")); + } catch { + return res.status(400).type("text").send("Invalid JSON body"); + } + } + res.status(200).json({ body: req.body ?? null }); + }); + return app; + } + + let bodyParserExports: BodyParserExports; + + beforeEach(() => { + // `patchBodyParser` mutates exports in place; flush `require.cache` and reload so each test + // starts from stock `body-parser` getters. + bodyParserExports = loadFreshBodyParserModuleExports(); + }); + + describe("patchBodyParser", () => { + // https://github.com/firebase/firebase-tools/issues/10404 — malformed `application/json` (for + // example a truncated body `{` after `POST` + `Content-Type: application/json`) could surface + // as 500 on Hosting framework SSR while the same app returned 400 under plain `next start`. + // The tolerance shim only clears `err.type === "entity.parse.failed"` so the downstream + // framework can read the raw body and respond with a client error instead of failing in + // Express body-parser middleware first. + it("lets truncated JSON bodies reach the app route (#10404: POST + application/json + `{`)", async () => { + patchBodyParser(bodyParserExports); + + const httpResponse = await supertest(buildApp(bodyParserExports)) + .post("/") + .set("Content-Type", "application/json") + .send("{"); + + expect(httpResponse.status).to.equal(400); + expect(httpResponse.text).to.equal("Invalid JSON body"); + }); + + it("lets invalid JSON that fails strict first-token checks reach the app route (#10404)", async () => { + patchBodyParser(bodyParserExports); + + const httpResponse = await supertest(buildApp(bodyParserExports)) + .post("/") + .set("Content-Type", "application/json") + .send("not-json"); + + expect(httpResponse.status).to.equal(400); + expect(httpResponse.text).to.equal("Invalid JSON body"); + }); + + it("parses valid JSON bodies normally", async () => { + patchBodyParser(bodyParserExports); + + const httpResponse = await supertest(buildApp(bodyParserExports)) + .post("/") + .set("Content-Type", "application/json") + .send(JSON.stringify({ foo: "bar" })); + + expect(httpResponse.status).to.equal(200); + expect(httpResponse.body.body).to.deep.equal({ foo: "bar" }); + }); + + it("still propagates non-parse errors (e.g. entity.too.large)", async () => { + patchBodyParser(bodyParserExports); + + const app = express(); + app.use(bodyParserExports.json!({ limit: "1b" }) as express.RequestHandler); + app.post("/", (req, res) => res.status(200).send("ok")); + + const httpResponse = await supertest(app) + .post("/") + .set("Content-Type", "application/json") + .send(JSON.stringify({ a: 1 })); + + expect(httpResponse.status).to.equal(413); + }); + + it("leaves non-matching content-types untouched", async () => { + patchBodyParser(bodyParserExports); + + const httpResponse = await supertest(buildApp(bodyParserExports)) + .post("/") + .set("Content-Type", "text/plain") + .send("hello"); + + expect(httpResponse.status).to.equal(200); + expect(httpResponse.body.body).to.deep.equal({}); + }); + + // Stock `text` / `raw` parsers do not throw from their parse step, so they do not emit + // `entity.parse.failed` the way `json` does (#10404). This still guards that wrapping those + // factories does not break normal parsing. + it("still parses valid text, urlencoded, and raw bodies after the patch", async () => { + patchBodyParser(bodyParserExports); + + const textApp = express(); + textApp.use(bodyParserExports.text!({ type: "*/*" }) as express.RequestHandler); + textApp.post("/", (req, res) => res.json({ body: req.body })); + + const urlApp = express(); + urlApp.use(bodyParserExports.urlencoded!({ extended: false }) as express.RequestHandler); + urlApp.post("/", (req, res) => res.json({ body: req.body })); + + const rawApp = express(); + rawApp.use(bodyParserExports.raw!({ type: "*/*" }) as express.RequestHandler); + rawApp.post("/", (req, res) => res.json({ length: (req.body as Buffer).length })); + + const [textResponse, urlResponse, rawResponse] = await Promise.all([ + supertest(textApp).post("/").set("Content-Type", "application/custom").send("hello"), + supertest(urlApp) + .post("/") + .set("Content-Type", "application/x-www-form-urlencoded") + .send("a=1&b=2"), + supertest(rawApp).post("/").set("Content-Type", "application/octet-stream").send("hi"), + ]); + + expect(textResponse.body.body).to.equal("hello"); + expect(urlResponse.body.body).to.deep.equal({ a: "1", b: "2" }); + expect(rawResponse.body.length).to.equal(2); + }); + + it("still propagates extended urlencoded depth failures (not entity.parse.failed)", async () => { + patchBodyParser(bodyParserExports); + + const app = express(); + app.use( + bodyParserExports.urlencoded!({ extended: true, depth: 5 }) as express.RequestHandler, + ); + app.post("/", (req, res) => res.status(200).json({ body: req.body })); + + const httpResponse = await supertest(app) + .post("/") + .type("form") + .send("a[b][c][d][e][f][g][h][i]=1"); + + expect(httpResponse.status).to.equal(400); + }); + + it("is idempotent - re-patching does not re-wrap", () => { + patchBodyParser(bodyParserExports); + const jsonFactoryAfterFirstPatch = bodyParserExports.json; + + patchBodyParser(bodyParserExports); + expect(bodyParserExports.json).to.equal(jsonFactoryAfterFirstPatch); + expect(bodyParserExports.__bodyParserPassthroughApplied).to.equal(true); + }); + + it("hides the patch marker from Object.keys / for..in", () => { + patchBodyParser(bodyParserExports); + expect(Object.keys(bodyParserExports)).to.not.include("__bodyParserPassthroughApplied"); + }); + + it("handles null and undefined without throwing", () => { + expect(() => patchBodyParser(null)).to.not.throw(); + expect(() => patchBodyParser(undefined)).to.not.throw(); + }); + + it("skips properties whose factory is not a function", () => { + const partialExports: BodyParserExports = { json: undefined }; + expect(() => patchBodyParser(partialExports)).to.not.throw(); + expect(partialExports.json).to.equal(undefined); + }); + }); + + describe("installBodyParserTolerance", () => { + it("patches body-parser reachable through require.cache", async () => { + expect(bodyParserExports.__bodyParserPassthroughApplied).to.be.undefined; + + installBodyParserTolerance(); + + expect(bodyParserExports.__bodyParserPassthroughApplied).to.equal(true); + + const httpResponse = await supertest(buildApp(bodyParserExports)) + .post("/") + .set("Content-Type", "application/json") + // Same class of failure as #10404 (`entity.parse.failed`), distinct from the truncated `{` probe. + .send("not-json"); + + expect(httpResponse.status).to.equal(400); + expect(httpResponse.text).to.equal("Invalid JSON body"); + }); + + it("is safe to call when no body-parser is loaded or resolvable", () => { + for (const requireCacheKey of Object.keys(require.cache)) { + if (requireCacheKey.includes(`/node_modules/body-parser/`)) { + delete require.cache[requireCacheKey]; + } + } + expect(() => installBodyParserTolerance()).to.not.throw(); + }); + }); + + describe("getBodyParserToleranceShim", () => { + const bodyParserToleranceShimSource = getBodyParserToleranceShim(); + + it("does not reference module.exports (would clobber host bindings)", () => { + expect(/\bmodule\.exports\b/.test(bodyParserToleranceShimSource)).to.equal(false); + }); + + it("invokes installBodyParserTolerance at the end", () => { + expect( + bodyParserToleranceShimSource.trimEnd().endsWith("installBodyParserTolerance();"), + ).to.equal(true); + }); + + it("concatenates patchBodyParser and installBodyParserTolerance sources for generated server.js", () => { + // Do not eval this string under `nyc`: instrumented `Function#toString()` embeds `cov_*` + // that is out of scope in `new Function` (https://github.com/istanbuljs/nyc/issues/1327). + // Patches applied at runtime are covered by installBodyParserTolerance / patchBodyParser tests above. + expect(bodyParserToleranceShimSource).to.include("function patchBodyParser"); + expect(bodyParserToleranceShimSource).to.include("function installBodyParserTolerance"); + }); + }); + }); }); diff --git a/src/frameworks/utils.ts b/src/frameworks/utils.ts index 2ee5339099c..52cd788d55e 100644 --- a/src/frameworks/utils.ts +++ b/src/frameworks/utils.ts @@ -474,3 +474,106 @@ export function getFrameworksBuildTarget(purpose: BUILD_TARGET_PURPOSE, validOpt ); } } + +type NextFn = (err?: unknown) => void; +type Middleware = (req: unknown, res: unknown, next: NextFn) => void; +type ParserFactory = (opts?: unknown) => Middleware; + +export interface BodyParserExports { + json?: ParserFactory; + text?: ParserFactory; + urlencoded?: ParserFactory; + raw?: ParserFactory; + __bodyParserPassthroughApplied?: boolean; +} + +/** + * Wraps `body-parser` parser factories so strict parse failures call `next()` without an error. + * Serialized into generated `server.js`; must not reference other modules from this file. + */ +export function patchBodyParser(bodyParserExports: BodyParserExports | null | undefined): void { + if (!bodyParserExports || bodyParserExports.__bodyParserPassthroughApplied) return; + // Idempotent; non-enumerable so `Object.keys(body-parser)` still looks like stock exports. + Object.defineProperty(bodyParserExports, "__bodyParserPassthroughApplied", { value: true, enumerable: false }); + + const PARSER_NAMES: Exclude[] = [ + "json", + "text", + "urlencoded", + "raw", + ]; + // Same default as body-parser's parse catch: https://github.com/expressjs/body-parser/blob/1.20.3/lib/read.js#L129-L132 + const PARSE_FAILED_TYPE = "entity.parse.failed"; + + for (const parserName of PARSER_NAMES) { + const factory = bodyParserExports[parserName]; + if (typeof factory !== "function") continue; + // Stock `body-parser` exposes these as getters without setters, so `exports.json = …` throws. + Object.defineProperty(bodyParserExports, parserName, { + configurable: true, + enumerable: true, + writable: true, + value: function tolerantFactory(this: unknown, opts: unknown): Middleware { + // Preserve original `this` and options so behavior matches stock body-parser. + const parse = factory.call(this, opts); + return function tolerantMiddleware(req, res, next) { + parse(req, res, (err) => { + // Only the strict parse-failure path: `verify` has already run, so `req.rawBody` is + // still populated when configured. Other errors (size limits, aborted reads, etc.) pass through. + if ( + typeof err === "object" && + err !== null && + (err as Record).type === PARSE_FAILED_TYPE + ) { + return next(); + } + next(err); + }); + }; + }, + }); + } +} + +/** + * Calls {@link patchBodyParser} for every loaded `body-parser` module and for `require("body-parser")`. + * Serialized next to {@link patchBodyParser} in {@link getBodyParserToleranceShim}. + */ +export function installBodyParserTolerance(): void { + const BODY_PARSER_INDEX_REGEX = /[\\/]node_modules[\\/]body-parser[\\/]index\.js$/; + + // Same semver, multiple disk locations: e.g. user `node_modules` and buildpack + // `.../functions-framework/.../node_modules/body-parser` each get their own `require.cache` entry. + for (const requireCacheKey of Object.keys(require.cache)) { + if (!BODY_PARSER_INDEX_REGEX.test(requireCacheKey)) continue; + const cachedModule = require.cache[requireCacheKey]; + if (cachedModule && cachedModule.exports) { + patchBodyParser(cachedModule.exports as BodyParserExports); + } + } + + try { + // Also patch the instance this process would get from a fresh `require()`. + patchBodyParser(require("body-parser") as BodyParserExports); + } catch { + // body-parser is not resolvable from the caller; nothing to do. + } +} + +/** + * Returns JavaScript prepended to generated framework `server.js` so {@link installBodyParserTolerance} + * runs before `firebase-functions` / `firebase-frameworks` load (`frameworks/index.ts`). + * + * Related issue: https://github.com/firebase/firebase-tools/issues/10404 + * + * Invalid JSON with a JSON `Content-Type` was returning 500 from Hosting while the same route + * returned 400 when run locally (for example `next start`). The emitted snippet patches `body-parser` + * so the framework receives the request. + * + * Built from `toString()` on {@link patchBodyParser} and {@link installBodyParserTolerance}, then + * `installBodyParserTolerance();` + */ +export function getBodyParserToleranceShim(): string { + // Function declarations hoist so installBodyParserTolerance can call patchBodyParser without imports. + return `${patchBodyParser.toString()}\n${installBodyParserTolerance.toString()}\ninstallBodyParserTolerance();\n`; +} From c8071d927fbf79417084edc7c4bc2d3928d1aa88 Mon Sep 17 00:00:00 2001 From: Leonardo Ortiz Date: Thu, 23 Apr 2026 18:58:02 -0300 Subject: [PATCH 2/2] format --- src/frameworks/utils.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/frameworks/utils.ts b/src/frameworks/utils.ts index 52cd788d55e..f092ffeeb1d 100644 --- a/src/frameworks/utils.ts +++ b/src/frameworks/utils.ts @@ -494,7 +494,10 @@ export interface BodyParserExports { export function patchBodyParser(bodyParserExports: BodyParserExports | null | undefined): void { if (!bodyParserExports || bodyParserExports.__bodyParserPassthroughApplied) return; // Idempotent; non-enumerable so `Object.keys(body-parser)` still looks like stock exports. - Object.defineProperty(bodyParserExports, "__bodyParserPassthroughApplied", { value: true, enumerable: false }); + Object.defineProperty(bodyParserExports, "__bodyParserPassthroughApplied", { + value: true, + enumerable: false, + }); const PARSER_NAMES: Exclude[] = [ "json",