Skip to content
13 changes: 4 additions & 9 deletions src/middleware/node/get-payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,15 @@ import AggregateError from "aggregate-error";
// }
type IncomingMessage = any;

export function getPayload(request: IncomingMessage): Promise<string> {
export function getPayload(request: IncomingMessage): Promise<Buffer> {
// 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);

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));
request.on("end", () => resolve(Buffer.concat(data)));
});
}
18 changes: 17 additions & 1 deletion src/middleware/node/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,28 @@ export async function middleware(
}, 9000).unref();

try {
const payload = await getPayload(request);
let payload: Buffer;
let body: { [key: string]: any } | undefined;

const bodyType = typeof request.body;

// The body is a String (e.g. Lambda)
if (bodyType === "string") {
payload = request.body;
// The body is already an Object and rawBody is a Buffer (e.g. GCF)
} else if (bodyType === "object" && request.rawBody instanceof Buffer) {
body = request.body;
payload = request.rawBody;
// We need to load the payload from the request (normal case of Node.js server)
} else {
payload = await getPayload(request);
}

await webhooks.verifyAndReceive({
id: id,
name: eventName as any,
payload,
body,
signature: signatureSHA256,
});
clearTimeout(timeout);
Expand Down
3 changes: 2 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ export type EmitterWebhookEvent<
export type EmitterWebhookEventWithStringPayloadAndSignature = {
id: string;
name: EmitterWebhookEventName;
payload: string;
body?: { [key: string]: any };
payload: string | Buffer;
signature: string;
};

Expand Down
28 changes: 20 additions & 8 deletions src/verify-and-receive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not. It's better to fix this at the source

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

event.payload,
event.signature,
).catch(() => false);
Expand All @@ -29,18 +30,29 @@ export async function verifyAndReceive(
);
}

let payload: EmitterWebhookEvent["payload"];
// The body is already an Object (e.g. GCF) and can be passed directly to the EventHandler
if (event.body) {
return state.eventHandler.receive({
id: event.id,
name: event.name,
payload: event.body as EmitterWebhookEvent["payload"],
});
}

const payload =
typeof event.payload === "string"
? event.payload
: event.payload.toString("utf8");

try {
payload = JSON.parse(event.payload);
return state.eventHandler.receive({
id: event.id,
name: event.name,
payload: JSON.parse(payload) as EmitterWebhookEvent["payload"],
});
} catch (error: any) {
error.message = "Invalid JSON";
error.status = 400;
throw new AggregateError([error]);
}

return state.eventHandler.receive({
id: event.id,
name: event.name,
payload,
});
}
47 changes: 47 additions & 0 deletions test/integration/node-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down