diff --git a/src/middleware/node/get-payload.ts b/src/middleware/node/get-payload.ts index 30c924ad..36e67a7f 100644 --- a/src/middleware/node/get-payload.ts +++ b/src/middleware/node/get-payload.ts @@ -11,20 +11,18 @@ import AggregateError from "aggregate-error"; // } type IncomingMessage = any; -export function getPayload(request: IncomingMessage): Promise { - // If request.body already exists we can stop here - // See https://github.com/octokit/webhooks.js/pull/23 - - if (request.body) return Promise.resolve(request.body); - +export function getPayload(request: IncomingMessage): Promise { return new Promise((resolve, reject) => { - let data = ""; - - request.setEncoding("utf8"); + let data: Buffer[] = []; // 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)); + // istanbul ignore next + request.on("end", () => + // setImmediate improves the throughput by reducing the pressure from + // the event loop + setImmediate(resolve, data.length === 1 ? data[0] : Buffer.concat(data)), + ); }); } diff --git a/src/middleware/node/middleware.ts b/src/middleware/node/middleware.ts index d80246e3..7781457c 100644 --- a/src/middleware/node/middleware.ts +++ b/src/middleware/node/middleware.ts @@ -77,11 +77,10 @@ export async function middleware( return true; } - const eventName = request.headers["x-github-event"] as WebhookEventName; - const signatureSHA256 = request.headers["x-hub-signature-256"] as string; + const name = request.headers["x-github-event"] as WebhookEventName; + const signature = request.headers["x-hub-signature-256"] as string; const id = request.headers["x-github-delivery"] as string; - - options.log.debug(`${eventName} event received (id: ${id})`); + options.log.debug(`${name} event received (id: ${id})`); // GitHub will abort the request if it does not receive a response within 10s // See https://github.com/octokit/webhooks.js/issues/185 @@ -93,13 +92,30 @@ export async function middleware( }, 9000).unref(); try { - const payload = await getPayload(request); + let payload: Buffer; + + if ("body" in request) { + if ( + typeof request.body === "object" && + "rawBody" in request && + request.rawBody instanceof Buffer + ) { + // The body is already an Object and rawBody is a Buffer (e.g. GCF) + payload = request.rawBody; + } else { + // The body is a String (e.g. Lambda) + payload = request.body; + } + } else { + // We need to load the payload from the request (normal case of Node.js server) + payload = await getPayload(request); + } await webhooks.verifyAndReceive({ - id: id, - name: eventName as any, + id, + name, payload, - signature: signatureSHA256, + signature, }); clearTimeout(timeout); diff --git a/src/types.ts b/src/types.ts index e64b07ea..8ca25da3 100644 --- a/src/types.ts +++ b/src/types.ts @@ -18,7 +18,7 @@ export type EmitterWebhookEvent< export type EmitterWebhookEventWithStringPayloadAndSignature = { id: string; name: EmitterWebhookEventName; - payload: string; + payload: string | Buffer; signature: string; }; diff --git a/src/verify-and-receive.ts b/src/verify-and-receive.ts index 88f28083..10adfcd5 100644 --- a/src/verify-and-receive.ts +++ b/src/verify-and-receive.ts @@ -15,6 +15,7 @@ export async function verifyAndReceive( // verify will validate that the secret is not undefined const matchesSignature = await verify( state.secret, + // @ts-expect-error verify uses createHmac, which can take Strings and Buffers event.payload, event.signature, ).catch(() => false); @@ -30,8 +31,12 @@ export async function verifyAndReceive( } let payload: EmitterWebhookEvent["payload"]; + try { - payload = JSON.parse(event.payload); + payload = + typeof event.payload === "string" + ? JSON.parse(event.payload) + : JSON.parse(event.payload.toString("utf8")); } catch (error: any) { error.message = "Invalid JSON"; error.status = 400; diff --git a/test/integration/node-middleware.test.ts b/test/integration/node-middleware.test.ts index a5568a42..1e7d7708 100644 --- a/test/integration/node-middleware.test.ts +++ b/test/integration/node-middleware.test.ts @@ -107,6 +107,53 @@ describe("createNodeMiddleware(webhooks)", () => { server.close(); }); + test("request.body is already an Object and has request.rawBody as Buffer (e.g. GCF)", async () => { + expect.assertions(3); + + const webhooks = new Webhooks({ + secret: "mySecret", + }); + const dataChunks: any[] = []; + const middleware = createNodeMiddleware(webhooks); + + const server = createServer((req, res) => { + req.once("data", (chunk) => dataChunks.push(chunk)); + req.once("end", () => { + // @ts-expect-error - TS2339: Property 'rawBody' does not exist on type 'IncomingMessage'. + req.rawBody = Buffer.concat(dataChunks); + // @ts-expect-error - TS2339: Property 'body' does not exist on type 'IncomingMessage'. + req.body = JSON.parse(req.rawBody); + middleware(req, res); + }); + }).listen(); + + webhooks.on("push", (event) => { + expect(event.id).toBe("123e4567-e89b-12d3-a456-426655440000"); + }); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }, + ); + + expect(response.status).toEqual(200); + expect(await response.text()).toEqual("ok\n"); + + server.close(); + }); + test("Handles invalid Content-Type", async () => { const webhooks = new Webhooks({ secret: "mySecret", @@ -372,7 +419,7 @@ describe("createNodeMiddleware(webhooks)", () => { }); test("Handles timeout", async () => { - jest.useFakeTimers(); + jest.useFakeTimers({ doNotFake: ["setImmediate"] }); const webhooks = new Webhooks({ secret: "mySecret", @@ -407,7 +454,7 @@ describe("createNodeMiddleware(webhooks)", () => { }); test("Handles timeout with error", async () => { - jest.useFakeTimers(); + jest.useFakeTimers({ doNotFake: ["setImmediate"] }); const webhooks = new Webhooks({ secret: "mySecret",