Merge pull request #503 from AikidoSec/fix-hono-body-locked

Fix Hono response body locked error

Author: hansott

library/sources/Hono.test.ts

@@ -94,6 +94,28 @@ function getApp() {
     return c.json(getContext());
   });
 
+  app.post("/json", async (c) => {
+    try {
+      const json = await c.req.json();
+    } catch (e) {
+      if (e instanceof SyntaxError) {
+        return c.text("Invalid JSON", 400);
+      }
+      throw e;
+    }
+    return c.json(getContext());
+  });
+
+  app.post("/text", async (c) => {
+    const text = await c.req.text();
+    return c.json(getContext());
+  });
+
+  app.post("/form", async (c) => {
+    const form = await c.req.parseBody();
+    return c.json(getContext());
+  });
+
   app.on(["GET"], ["/user", "/user/blocked"], (c) => {
     return c.json(getContext());
   });
@@ -140,7 +162,7 @@ t.test("it adds context from request for GET", opts, async (t) => {
 });
 
 t.test("it adds JSON body to context", opts, async (t) => {
-  const response = await getApp().request("/", {
+  const response = await getApp().request("/json", {
     method: "POST",
     headers: {
       "content-type": "application/json",
@@ -153,12 +175,12 @@ t.test("it adds JSON body to context", opts, async (t) => {
     method: "POST",
     body: { title: "test" },
     source: "hono",
-    route: "/",
+    route: "/json",
   });
 });
 
 t.test("it adds form body to context", opts, async (t) => {
-  const response = await getApp().request("/", {
+  const response = await getApp().request("/form", {
     method: "POST",
     headers: {
       "content-type": "application/x-www-form-urlencoded",
@@ -171,12 +193,12 @@ t.test("it adds form body to context", opts, async (t) => {
     method: "POST",
     body: { title: "test" },
     source: "hono",
-    route: "/",
+    route: "/form",
   });
 });
 
 t.test("it adds text body to context", opts, async (t) => {
-  const response = await getApp().request("/", {
+  const response = await getApp().request("/text", {
     method: "POST",
     headers: {
       "content-type": "text/plain",
@@ -189,12 +211,12 @@ t.test("it adds text body to context", opts, async (t) => {
     method: "POST",
     body: "test",
     source: "hono",
-    route: "/",
+    route: "/text",
   });
 });
 
 t.test("it adds xml body to context", opts, async (t) => {
-  const response = await getApp().request("/", {
+  const response = await getApp().request("/text", {
     method: "POST",
     headers: {
       "content-type": "application/xml",
@@ -207,7 +229,7 @@ t.test("it adds xml body to context", opts, async (t) => {
     method: "POST",
     body: "<test>test</test>",
     source: "hono",
-    route: "/",
+    route: "/text",
   });
 });
 
@@ -379,3 +401,108 @@ t.test("The hono async context still works", opts, async (t) => {
   const body = await response.text();
   t.equal(body, "test-value");
 });
+
+t.test("Proxy request", opts, async (t) => {
+  const { Hono } = require("hono") as typeof import("hono");
+  const { serve } =
+    require("@hono/node-server") as typeof import("@hono/node-server");
+
+  const app = new Hono();
+
+  app.on(["GET", "POST"], "/proxy", async (c) => {
+    const response = await globalThis.fetch(
+      new Request("http://127.0.0.1:8768/body", {
+        method: c.req.method,
+        headers: c.req.raw.headers,
+        body: c.req.raw.body,
+        // @ts-expect-error wrong types
+        duplex: "half",
+        redirect: "manual",
+      })
+    );
+    // clone the response to return a response with modifiable headers
+    return new Response(response.body, response);
+  });
+
+  app.post("/body", async (c) => {
+    return await c.req.json();
+  });
+
+  const server = serve({
+    fetch: app.fetch,
+    port: 8767,
+    hostname: "127.0.0.1",
+  });
+
+  const app2 = new Hono();
+  app2.all("/*", async (c) => {
+    return c.text(await c.req.text());
+  });
+
+  const server2 = serve({
+    fetch: app2.fetch,
+    port: 8768,
+    hostname: "127.0.0.1",
+  });
+
+  const response = await fetch.fetch({
+    url: new URL("http://127.0.0.1:8767/proxy"),
+    method: "POST",
+    headers: {
+      "Content-Type": "application/json",
+    },
+    body: JSON.stringify({ a: 1 }),
+  });
+  t.equal(response.statusCode, 200);
+  t.equal(response.body, JSON.stringify({ a: 1 }));
+
+  // Cleanup servers
+  server.close();
+  server2.close();
+});
+
+t.test("Body parsing in middleware", opts, async (t) => {
+  const { Hono } = require("hono") as typeof import("hono");
+
+  const app = new Hono<{ Variables: { body: any } }>();
+
+  app.use(async (c, next) => {
+    c.set("body", await c.req.json());
+    return next();
+  });
+
+  app.post("/", async (c) => {
+    return c.json(getContext());
+  });
+
+  const response = await app.request("/", {
+    method: "POST",
+    headers: {
+      "Content-Type": "application/json",
+    },
+    body: JSON.stringify({ x: 42 }),
+  });
+
+  const body = await response.json();
+  t.match(body, {
+    method: "POST",
+    body: { x: 42 },
+    source: "hono",
+    route: "/",
+  });
+});
+
+t.test("invalid json body", opts, async (t) => {
+  const app = getApp();
+
+  const response = await app.request("/json", {
+    method: "POST",
+    headers: {
+      "Content-Type": "application/json",
+    },
+    body: "invalid",
+  });
+
+  t.same(response.status, 400);
+  t.same(await response.text(), "Invalid JSON");
+});

library/sources/hono/contextFromRequest.ts

@@ -1,50 +1,27 @@
 import type { Context as HonoContext } from "hono";
-import { Context } from "../../agent/Context";
+import { Context, getContext } from "../../agent/Context";
 import { buildRouteFromURL } from "../../helpers/buildRouteFromURL";
 import { getIPAddressFromRequest } from "../../helpers/getIPAddressFromRequest";
-import { isJsonContentType } from "../../helpers/isJsonContentType";
 import { parse } from "../../helpers/parseCookies";
 import { getRemoteAddress } from "./getRemoteAddress";
 
 export async function contextFromRequest(c: HonoContext): Promise<Context> {
   const { req } = c;
 
-  let body = undefined;
-  const contentType = req.header("content-type");
-  if (contentType) {
-    if (isJsonContentType(contentType)) {
-      try {
-        body = await req.json();
-      } catch {
-        // Ignore
-      }
-    } else if (contentType.startsWith("application/x-www-form-urlencoded")) {
-      try {
-        body = await req.parseBody();
-      } catch {
-        // Ignore
-      }
-    } else if (
-      contentType.includes("text/plain") ||
-      contentType.includes("xml")
-    ) {
-      try {
-        body = await req.text();
-      } catch {
-        // Ignore
-      }
-    }
-  }
-
   const cookieHeader = req.header("cookie");
+  const existingContext = getContext();
 
   return {
     method: c.req.method,
     remoteAddress: getIPAddressFromRequest({
       headers: req.header(),
       remoteAddress: getRemoteAddress(c),
     }),
-    body: body,
+    // Pass the body from the existing context if it's already set, otherwise the body is set in wrapRequestBodyParsing
+    body:
+      existingContext && existingContext.source === "hono"
+        ? existingContext.body
+        : undefined,
     url: req.url,
     headers: req.header(),
     routeParams: req.param(),

library/sources/hono/wrapRequestBodyParsing.ts

@@ -0,0 +1,33 @@
+import type { Context } from "hono";
+import { getContext, updateContext } from "../../agent/Context";
+import { createWrappedFunction, isWrapped } from "../../helpers/wrap";
+
+// Wrap the request body parsing functions to update the context with the parsed body, if any of the functions are called.
+export function wrapRequestBodyParsing(req: Context["req"]) {
+  req.parseBody = wrapBodyParsingFunction(req.parseBody);
+  req.json = wrapBodyParsingFunction(req.json);
+  req.text = wrapBodyParsingFunction(req.text);
+}
+
+function wrapBodyParsingFunction<T extends Function>(func: T) {
+  if (isWrapped(func)) {
+    return func;
+  }
+
+  return createWrappedFunction(func, function parse(parser) {
+    return async function wrap() {
+      // @ts-expect-error No type for arguments
+      // eslint-disable-next-line prefer-rest-params
+      const returnValue = await parser.apply(this, arguments);
+
+      if (returnValue) {
+        const context = getContext();
+        if (context) {
+          updateContext(context, "body", returnValue);
+        }
+      }
+
+      return returnValue;
+    };
+  }) as T;
+}

library/sources/hono/wrapRequestHandler.ts

@@ -1,6 +1,7 @@
 import type { Handler, MiddlewareHandler } from "hono";
 import { runWithContext } from "../../agent/Context";
 import { contextFromRequest } from "./contextFromRequest";
+import { wrapRequestBodyParsing } from "./wrapRequestBodyParsing";
 
 export function wrapRequestHandler(
   handler: Handler | MiddlewareHandler
@@ -9,6 +10,8 @@ export function wrapRequestHandler(
     const context = await contextFromRequest(c);
 
     return await runWithContext(context, async () => {
+      wrapRequestBodyParsing(c.req);
+
       return await handler(c, next);
     });
   };