Repository: unjs/h3
Pull request #207 (2022-10-19 11:42:53)

feat(readBody): validate requests with `application/json` content type
Follow-up from https://github.com/unjs/h3/pull/201. Changes are:
- Throw error if content-type says the request body is json but the body is not valid json; uses the newly introduced strict mode from https://github.com/unjs/destr/pull/11.
- Doesn't try to parse the body (as json) if content-type says its plain text
- Improve internal and external typing information. Fixes https://github.com/unjs/h3/issues/223.
- Streamline codeflow in `readBodyRaw`

tobiasdiez: @pi0 this is now ready for review
codecov[bot]: # [Codecov](https://codecov.io/gh/unjs/h3/pull/207?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs) Report
> Merging [#207](https://codecov.io/gh/unjs/h3/pull/207?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs) (8e99061) into [main](https://codecov.io/gh/unjs/h3/commit/501f0c6e623ea827d47691046f3c7319f5ac4651?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs) (501f0c6) will **increase** coverage by `0.58%`.
> The diff coverage is `93.61%`.

```diff
@@            Coverage Diff             @@
##             main     #207      +/-   ##
==========================================
+ Coverage   76.84%   77.43%   +0.58%     
==========================================
  Files          20       20              
  Lines        1218     1232      +14     
  Branches      228      229       +1     
==========================================
+ Hits          936      954      +18     
+ Misses        282      278       -4     
```


| [Impacted Files](https://codecov.io/gh/unjs/h3/pull/207?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs) | Coverage Δ | |
|---|---|---|
| [src/utils/body.ts](https://codecov.io/gh/unjs/h3/pull/207/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs#diff-c3JjL3V0aWxzL2JvZHkudHM=) | `91.48% <93.61%> (+6.48%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs)

nopeless: @tobiasdiez Nice pr; I ended up fixing the encoding signature too just to realize that you had a fix haha
tobiasdiez: @pi0 merged in the latest version from main, so this PR is ready for review again
tobiasdiez: @pi0 would appreciate if you could have a look at this PR. Thanks!
Hebilicious: @pi0 I think this is overall a good change, would like to know what you think before approving.
pi0: Hi, dear @tobiasdiez, and sorry for checking this late.

Before merging, I made a few changes to fix the behavior matching PR description (we strictly validate json body only when `application/json` is passing otherwise using default `defu` behavior with a try-parse strategy which also ensures this is a non-breaking change). 

I will be merging the type fix issue for `readRawBody` in a separate commit into the main (on your behalf). In the future please consider splitting fixes to allow iterating faster and having a clear changelog 🙏🏼 

Cheers!

Patch:
diff --git a/src/utils/body.ts b/src/utils/body.ts
index 382d7b9..c53b90a 100644
--- a/src/utils/body.ts
+++ b/src/utils/body.ts
@@ -1,6 +1,8 @@
+import type { IncomingMessage } from "node:http";
 import destr from "destr";
 import type { Encoding, HTTPMethod } from "../types";
 import type { H3Event } from "../event";
+import { createError } from "../error";
 import { parse as parseMultipartData } from "./internal/multipart";
 import { assertMethod, getRequestHeader } from "./request";
 
@@ -8,11 +10,16 @@ export type { MultiPartData } from "./internal/multipart";
 
 const RawBodySymbol = Symbol.for("h3RawBody");
 const ParsedBodySymbol = Symbol.for("h3ParsedBody");
+type InternalRequest<T = any> = IncomingMessage & {
+  [RawBodySymbol]?: Promise<Buffer | undefined>;
+  [ParsedBodySymbol]?: T;
+  body?: string | undefined;
+};
 
 const PayloadMethods: HTTPMethod[] = ["PATCH", "POST", "PUT", "DELETE"];
 
 /**
- * Reads body of the request and returns encoded raw string (default) or `Buffer` if encoding if falsy.
+ * Reads body of the request and returns encoded raw string (default), or `Buffer` if encoding is falsy.
  * @param event {H3Event} H3 event or req passed by h3 handler
  * @param encoding {Encoding} encoding="utf-8" - The character encoding to use.
  *
@@ -73,45 +80,42 @@ export function readRawBody<E extends Encoding = "utf8">(
 }
 
 /**
- * Reads request body and try to safely parse using [destr](https://github.com/unjs/destr)
- * @param event {H3Event} H3 event or req passed by h3 handler
+ * Reads request body and tries to safely parse using [destr](https://github.com/unjs/destr).
+ * @param event {H3Event} H3 event passed by h3 handler
  * @param encoding {Encoding} encoding="utf-8" - The character encoding to use.
  *
  * @return {*} The `Object`, `Array`, `String`, `Number`, `Boolean`, or `null` value corresponding to the request JSON body
  *
  * ```ts
- * const body = await readBody(req)
+ * const body = await readBody(event)
  * ```
  */
-export async function readBody<T = any>(event: H3Event): Promise<T> {
-  if (ParsedBodySymbol in event.node.req) {
-    return (event.node.req as any)[ParsedBodySymbol];
+export async function readBody<T = any>(
+  event: H3Event,
+  options: { strict?: boolean } = {}
+): Promise<T | undefined | string> {
+  const request = event.node.req as InternalRequest<T>;
+  if (ParsedBodySymbol in request) {
+    return request[ParsedBodySymbol];
   }
 
-  const body = await readRawBody(event, "utf8");
-
-  if (
-    event.node.req.headers["content-type"] ===
-    "application/x-www-form-urlencoded"
-  ) {
-    const form = new URLSearchParams(body);
-    const parsedForm: Record<string, any> = Object.create(null);
-    for (const [key, value] of form.entries()) {
-      if (key in parsedForm) {
-        if (!Array.isArray(parsedForm[key])) {
-          parsedForm[key] = [parsedForm[key]];
-        }
-        parsedForm[key].push(value);
-      } else {
-        parsedForm[key] = value;
-      }
-    }
-    return parsedForm as unknown as T;
+  const contentType = request.headers["content-type"] || "";
+  const body = await readRawBody(event);
+
+  let parsed: T;
+
+  if (contentType === "application/json") {
+    parsed = _parseJSON(body, options.strict ?? true) as T;
+  } else if (contentType === "application/x-www-form-urlencoded") {
+    parsed = _parseURLEncodedBody(body!) as T;
+  } else if (contentType.startsWith("text/")) {
+    parsed = body as T;
+  } else {
+    parsed = _parseJSON(body, options.strict ?? false) as T;
   }
 
-  const json = destr(body) as T;
-  (event.node.req as any)[ParsedBodySymbol] = json;
-  return json;
+  request[ParsedBodySymbol] = parsed;
+  return parsed;
 }
 
 export async function readMultipartFormData(event: H3Event) {
@@ -129,3 +133,36 @@ export async function readMultipartFormData(event: H3Event) {
   }
   return parseMultipartData(body, boundary);
 }
+
+// --- Internal ---
+
+function _parseJSON(body = "", strict: boolean) {
+  if (!body) {
+    return undefined;
+  }
+  try {
+    return destr(body, { strict });
+  } catch {
+    throw createError({
+      statusCode: 400,
+      statusMessage: "Bad Request",
+      message: "Invalid JSON body",
+    });
+  }
+}
+
+function _parseURLEncodedBody(body: string) {
+  const form = new URLSearchParams(body);
+  const parsedForm: Record<string, any> = Object.create(null);
+  for (const [key, value] of form.entries()) {
+    if (key in parsedForm) {
+      if (!Array.isArray(parsedForm[key])) {
+        parsedForm[key] = [parsedForm[key]];
+      }
+      parsedForm[key].push(value);
+    } else {
+      parsedForm[key] = value;
+    }
+  }
+  return parsedForm as unknown;
+}


Test patch:
diff --git a/test/body.test.ts b/test/body.test.ts
index 24e0b87..98696af 100644
--- a/test/body.test.ts
+++ b/test/body.test.ts
@@ -41,7 +41,7 @@ describe("", () => {
     });
 
     it("returns undefined if body is not present", async () => {
-      let body: string | undefined = "initial";
+      let body = "initial";
       app.use(
         "/",
         eventHandler(async (request) => {
@@ -56,7 +56,7 @@ describe("", () => {
     });
 
     it("returns an empty string if body is empty", async () => {
-      let body: string | undefined = "initial";
+      let body = "initial";
       app.use(
         "/",
         eventHandler(async (request) => {
@@ -71,7 +71,7 @@ describe("", () => {
     });
 
     it("returns an empty object string if body is empty object", async () => {
-      let body: string | undefined = "initial";
+      let body = "initial";
       app.use(
         "/",
         eventHandler(async (request) => {
@@ -110,7 +110,7 @@ describe("", () => {
     });
 
     it("handles non-present body", async () => {
-      let _body = "initial";
+      let _body;
       app.use(
         "/",
         eventHandler(async (request) => {
@@ -136,7 +136,7 @@ describe("", () => {
         .post("/api/test")
         .set("Content-Type", "text/plain")
         .send('""');
-      expect(_body).toStrictEqual("");
+      expect(_body).toStrictEqual('""');
       expect(result.text).toBe("200");
     });
 
@@ -265,5 +265,92 @@ describe("", () => {
         ]
       `);
     });
+
+    it("returns undefined if body is not present with text/plain", async () => {
+      let body;
+      app.use(
+        "/",
+        eventHandler(async (request) => {
+          body = await readBody(request);
+          return "200";
+        })
+      );
+      const result = await request
+        .post("/api/test")
+        .set("Content-Type", "text/plain");
+
+      expect(body).toBeUndefined();
+      expect(result.text).toBe("200");
+    });
+
+    it("returns undefined if body is not present with json", async () => {
+      let body;
+      app.use(
+        "/",
+        eventHandler(async (request) => {
+          body = await readBody(request);
+          return "200";
+        })
+      );
+      const result = await request
+        .post("/api/test")
+        .set("Content-Type", "application/json");
+
+      expect(body).toBeUndefined();
+      expect(result.text).toBe("200");
+    });
+
+    it("returns the string if content type is text/*", async () => {
+      let body;
+      app.use(
+        "/",
+        eventHandler(async (request) => {
+          body = await readBody(request);
+          return "200";
+        })
+      );
+      const result = await request
+        .post("/api/test")
+        .set("Content-Type", "text/*")
+        .send('{ "hello": true }');
+
+      expect(body).toBe('{ "hello": true }');
+      expect(result.text).toBe("200");
+    });
+
+    it("returns string as is if cannot parse with unknown content type", async () => {
+      app.use(
+        "/",
+        eventHandler(async (request) => {
+          const _body = await readBody(request);
+          return _body;
+        })
+      );
+      const result = await request
+        .post("/api/test")
+        .set("Content-Type", "application/foobar")
+        .send("{ test: 123 }");
+
+      expect(result.statusCode).toBe(200);
+      expect(result.text).toBe("{ test: 123 }");
+    });
+
+    it("fails if json is invalid", async () => {
+      app.use(
+        "/",
+        eventHandler(async (request) => {
+          const _body = await readBody(request);
+          return _body;
+        })
+      );
+      const result = await request
+        .post("/api/test")
+        .set("Content-Type", "application/json")
+        .send('{ "hello": true');
+
+      expect(result.statusCode).toBe(400);
+      expect(result.body.statusMessage).toBe("Bad Request");
+      expect(result.body.stack[0]).toBe("Error: Invalid JSON body");
+    });
   });
 });