Skip to content

Commit 99f265a

Browse files
committed
fix: handle gcf, verify already provided request.body
1 parent 8423803 commit 99f265a

File tree

5 files changed

+90
-19
lines changed

5 files changed

+90
-19
lines changed

src/middleware/node/get-payload.ts

+4-9
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,15 @@ import AggregateError from "aggregate-error";
1111
// }
1212
type IncomingMessage = any;
1313

14-
export function getPayload(request: IncomingMessage): Promise<string> {
14+
export function getPayload(request: IncomingMessage): Promise<Buffer> {
1515
// If request.body already exists we can stop here
1616
// See https://github.com/octokit/webhooks.js/pull/23
17-
18-
if (request.body) return Promise.resolve(request.body);
19-
2017
return new Promise((resolve, reject) => {
21-
let data = "";
22-
23-
request.setEncoding("utf8");
18+
let data: Buffer[] = [];
2419

2520
// istanbul ignore next
2621
request.on("error", (error: Error) => reject(new AggregateError([error])));
27-
request.on("data", (chunk: string) => (data += chunk));
28-
request.on("end", () => resolve(data));
22+
request.on("data", (chunk: Buffer) => data.push(chunk));
23+
request.on("end", () => resolve(Buffer.concat(data)));
2924
});
3025
}

src/middleware/node/middleware.ts

+17-1
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,28 @@ export async function middleware(
9393
}, 9000).unref();
9494

9595
try {
96-
const payload = await getPayload(request);
96+
let payload: Buffer;
97+
let body: { [key: string]: any } | undefined;
98+
99+
const bodyType = typeof request.body;
100+
101+
// The body is a String (e.g. Lambda)
102+
if (bodyType === "string") {
103+
payload = request.body;
104+
// The body is already an Object and rawBody is a Buffer (e.g. GCF)
105+
} else if (bodyType === "object" && request.rawBody instanceof Buffer) {
106+
body = request.body;
107+
payload = request.rawBody;
108+
// We need to load the payload from the request (normal case of Node.js server)
109+
} else {
110+
payload = await getPayload(request);
111+
}
97112

98113
await webhooks.verifyAndReceive({
99114
id: id,
100115
name: eventName as any,
101116
payload,
117+
body,
102118
signature: signatureSHA256,
103119
});
104120
clearTimeout(timeout);

src/types.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ export type EmitterWebhookEvent<
1818
export type EmitterWebhookEventWithStringPayloadAndSignature = {
1919
id: string;
2020
name: EmitterWebhookEventName;
21-
payload: string;
21+
body?: { [key: string]: any };
22+
payload: string | Buffer;
2223
signature: string;
2324
};
2425

src/verify-and-receive.ts

+20-8
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export async function verifyAndReceive(
1515
// verify will validate that the secret is not undefined
1616
const matchesSignature = await verify(
1717
state.secret,
18+
// @ts-expect-error verify uses createHmac, which can take Strings and Buffers
1819
event.payload,
1920
event.signature,
2021
).catch(() => false);
@@ -29,18 +30,29 @@ export async function verifyAndReceive(
2930
);
3031
}
3132

32-
let payload: EmitterWebhookEvent["payload"];
33+
// The body is already an Object (e.g. GCF) and can be passed directly to the EventHandler
34+
if (event.body) {
35+
return state.eventHandler.receive({
36+
id: event.id,
37+
name: event.name,
38+
payload: event.body as EmitterWebhookEvent["payload"],
39+
});
40+
}
41+
42+
const payload =
43+
typeof event.payload === "string"
44+
? event.payload
45+
: event.payload.toString("utf8");
46+
3347
try {
34-
payload = JSON.parse(event.payload);
48+
return state.eventHandler.receive({
49+
id: event.id,
50+
name: event.name,
51+
payload: JSON.parse(payload) as EmitterWebhookEvent["payload"],
52+
});
3553
} catch (error: any) {
3654
error.message = "Invalid JSON";
3755
error.status = 400;
3856
throw new AggregateError([error]);
3957
}
40-
41-
return state.eventHandler.receive({
42-
id: event.id,
43-
name: event.name,
44-
payload,
45-
});
4658
}

test/integration/node-middleware.test.ts

+47
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,53 @@ describe("createNodeMiddleware(webhooks)", () => {
107107
server.close();
108108
});
109109

110+
test("request.body is already an Object and has request.rawBody as Buffer (e.g. GCF)", async () => {
111+
expect.assertions(3);
112+
113+
const webhooks = new Webhooks({
114+
secret: "mySecret",
115+
});
116+
const dataChunks: any[] = [];
117+
const middleware = createNodeMiddleware(webhooks);
118+
119+
const server = createServer((req, res) => {
120+
req.once("data", (chunk) => dataChunks.push(chunk));
121+
req.once("end", () => {
122+
// @ts-expect-error - TS2339: Property 'rawBody' does not exist on type 'IncomingMessage'.
123+
req.rawBody = Buffer.concat(dataChunks);
124+
// @ts-expect-error - TS2339: Property 'body' does not exist on type 'IncomingMessage'.
125+
req.body = JSON.parse(req.rawBody);
126+
middleware(req, res);
127+
});
128+
}).listen();
129+
130+
webhooks.on("push", (event) => {
131+
expect(event.id).toBe("123e4567-e89b-12d3-a456-426655440000");
132+
});
133+
134+
// @ts-expect-error complains about { port } although it's included in returned AddressInfo interface
135+
const { port } = server.address();
136+
137+
const response = await fetch(
138+
`http://localhost:${port}/api/github/webhooks`,
139+
{
140+
method: "POST",
141+
headers: {
142+
"Content-Type": "application/json",
143+
"X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000",
144+
"X-GitHub-Event": "push",
145+
"X-Hub-Signature-256": signatureSha256,
146+
},
147+
body: pushEventPayload,
148+
},
149+
);
150+
151+
expect(response.status).toEqual(200);
152+
expect(await response.text()).toEqual("ok\n");
153+
154+
server.close();
155+
});
156+
110157
test("Handles invalid Content-Type", async () => {
111158
const webhooks = new Webhooks({
112159
secret: "mySecret",

0 commit comments

Comments
 (0)