fix: prevent splitting multibyte characters in getPayload() (#939)

If you expect that the payload is using utf8 or utf16 or any other multybyte encoding, then you have to concat the buffers first and in the final step you can serialize the buffer to a string with the desired encoding.

The issue will arise, when with some "luck" a multibyte character is split between two chunks.

Author: Uzlopak

src/middleware/node/get-payload.ts

@@ -18,13 +18,19 @@ export function getPayload(request: IncomingMessage): Promise<string> {
   if (request.body) return Promise.resolve(request.body);
 
   return new Promise((resolve, reject) => {
-    let data = "";
+    let data: Buffer[] = [];
 
-    request.setEncoding("utf8");
-
-    // istanbul ignore next
     request.on("error", (error: Error) => reject(new AggregateError([error])));
-    request.on("data", (chunk: string) => (data += chunk));
-    request.on("end", () => resolve(data));
+    request.on("data", (chunk: Buffer) => data.push(chunk));
+    request.on("end", () =>
+      // setImmediate improves the throughput by reducing the pressure from
+      // the event loop
+      setImmediate(
+        resolve,
+        data.length === 1
+          ? data[0].toString("utf8")
+          : Buffer.concat(data).toString("utf8"),
+      ),
+    );
   });
 }

test/integration/get-payload.test.ts

@@ -0,0 +1,77 @@
+import EventEmitter from "node:events";
+import { getPayload } from "../../src/middleware/node/get-payload.ts";
+
+describe("getPayload", () => {
+  it("returns a promise", () => {
+    const request = new EventEmitter();
+    const promise = getPayload(request);
+
+    expect(promise).toBeInstanceOf(Promise);
+  });
+
+  it("resolves with a string when only receiving no chunk", async () => {
+    const request = new EventEmitter();
+    const promise = getPayload(request);
+
+    request.emit("end");
+
+    expect(await promise).toEqual("");
+  });
+
+  it("resolves with a string when only receiving one chunk", async () => {
+    const request = new EventEmitter();
+    const promise = getPayload(request);
+
+    request.emit("data", Buffer.from("foobar"));
+    request.emit("end");
+
+    expect(await promise).toEqual("foobar");
+  });
+
+  it("resolves with a string when receiving multiple chunks", async () => {
+    const request = new EventEmitter();
+    const promise = getPayload(request);
+
+    request.emit("data", Buffer.from("foo"));
+    request.emit("data", Buffer.from("bar"));
+    request.emit("end");
+
+    expect(await promise).toEqual("foobar");
+  });
+
+  it("rejects with an error", async () => {
+    const request = new EventEmitter();
+    const promise = getPayload(request);
+
+    request.emit("error", new Error("test"));
+
+    await expect(promise).rejects.toThrow("test");
+  });
+
+  it("resolves with a string with respecting the utf-8 encoding", async () => {
+    const request = new EventEmitter();
+    const promise = getPayload(request);
+
+    const doubleByteBuffer = Buffer.from("ݔ");
+    request.emit("data", doubleByteBuffer.subarray(0, 1));
+    request.emit("data", doubleByteBuffer.subarray(1, 2));
+    request.emit("end");
+
+    expect(await promise).toEqual("ݔ");
+  });
+
+  it("resolves with the body, if passed via the request", async () => {
+    const request = new EventEmitter();
+    // @ts-ignore body is not part of EventEmitter, which we are using
+    // to mock the request object
+    request.body = "foo";
+
+    const promise = getPayload(request);
+
+    // we emit data, to ensure that the body attribute is preferred
+    request.emit("data", "bar");
+    request.emit("end");
+
+    expect(await promise).toEqual("foo");
+  });
+});

test/integration/node-middleware.test.ts

@@ -372,7 +372,7 @@ describe("createNodeMiddleware(webhooks)", () => {
   });
 
   test("Handles timeout", async () => {
-    jest.useFakeTimers();
+    jest.useFakeTimers({ doNotFake: ["setImmediate"] });
 
     const webhooks = new Webhooks({
       secret: "mySecret",
@@ -407,7 +407,7 @@ describe("createNodeMiddleware(webhooks)", () => {
   });
 
   test("Handles timeout with error", async () => {
-    jest.useFakeTimers();
+    jest.useFakeTimers({ doNotFake: ["setImmediate"] });
 
     const webhooks = new Webhooks({
       secret: "mySecret",