fix: body-parser shim to allow strict parse failures to reach the app

src/frameworks/index.ts

@@ -25,6 +25,7 @@
   findDependency,
   conjoinOptions,
   frameworksCallToAction,
+  getBodyParserToleranceShim,
   getFrameworksBuildTarget,
 } from "./utils";
 import {
@@ -61,7 +62,7 @@
 /**
  *
  */
 export async function discover(dir: string, warn = true) {
   const allFrameworkTypes = [
     ...new Set(Object.values(WebFrameworks).map(({ type }) => type)),
   ].sort();
@@ -91,18 +92,18 @@
 function memoizeBuild(
   dir: string,
   build: Framework["build"],
   deps: any[],
   target: string,
   context: FrameworkContext,
 ): ReturnType<Framework["build"]> {
   const key = [dir, ...deps];
   for (const existingKey of BUILD_MEMO.keys()) {
     if (isDeepStrictEqual(existingKey, key)) {
       return BUILD_MEMO.get(existingKey) as ReturnType<Framework["build"]>;
     }
   }
   const value = build(dir, target, context);
   BUILD_MEMO.set(key, value);
   return value;
 }
 
@@ -110,7 +111,7 @@
  * Use a function to ensure the same codebase name is used here and
  * during hosting deploy.
  */
 export function generateSSRCodebaseId(site: string) {
   return `firebase-frameworks-${site}`;
 }
 
@@ -180,7 +181,7 @@
         `Hosting config for site ${site} places server-side content in region ${ssrRegion} which is not known. Valid regions are ${validRegions}`,
       );
     }
     const getProjectPath = (...args: string[]) => join(projectRoot, source, ...args);
     // Combined traffic tag (19 chars) and functionId cannot exceed 46 characters.
     const functionId = `ssr${site.toLowerCase().replace(/-/g, "").substring(0, 20)}`;
     const usesFirebaseAdminSdk = !!findDependency("firebase-admin", { cwd: getProjectPath() });
@@ -229,7 +230,7 @@
   You can link a Web app to a Hosting site here https://console.firebase.google.com/project/${project}/settings/general/web`,
             );
             firebaseDefaults ||= {};
             firebaseDefaults.config = JSON.parse(defaultConfig.json);
           } else {
             // N.B. None of us know when this can ever happen and the deploy would
             // still succeed. Maaaaybe if someone tried calling firebase serve
@@ -364,7 +365,7 @@
 
       const codebase = generateSSRCodebaseId(site);
       const existingFunctionsConfig = options.config.get("functions")
         ? [].concat(options.config.get("functions"))
         : [];
       options.config.set("functions", [
         ...existingFunctionsConfig,
@@ -405,7 +406,7 @@
       }
 
       const {
         packageJson,
         bootstrapScript,
         frameworksEntry = framework,
         dotEnv = {},
@@ -438,7 +439,7 @@
       // Set the framework entry in the env variables to handle generation of the functions.yaml
       process.env.__FIREBASE_FRAMEWORKS_ENTRY__ = frameworksEntry;
 
       packageJson.main = "server.js";
       packageJson.dependencies ||= {};
       packageJson.dependencies["firebase-frameworks"] ||= FIREBASE_FRAMEWORKS_VERSION;
       packageJson.dependencies["firebase-functions"] ||= FIREBASE_FUNCTIONS_VERSION;
@@ -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 {

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("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");
+      });
+    });
+  });
 });