Skip to content

Commit

Permalink
fix: only parse payload once
Browse files Browse the repository at this point in the history
  • Loading branch information
Uzlopak committed Nov 16, 2023
1 parent b38eeb3 commit f3b3d27
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 17 deletions.
10 changes: 1 addition & 9 deletions src/middleware/node/get-payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,7 @@ export function getPayload(request: IncomingMessage): Promise<string> {
request.on("error", (error: Error) => reject(new AggregateError([error])));
request.on("data", (chunk: string) => (data += chunk));
request.on("end", () => {
try {
// Call JSON.parse() only to check if the payload is valid JSON
JSON.parse(data);
resolve(data);
} catch (error: any) {
error.message = "Invalid JSON";
error.status = 400;
reject(new AggregateError([error]));
}
resolve(data);
});
});
}
24 changes: 18 additions & 6 deletions src/verify-and-receive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import type {
EmitterWebhookEventWithStringPayloadAndSignature,
State,
} from "./types";
import AggregateError from "aggregate-error";

export async function verifyAndReceive(
state: State & { secret: string },
event: EmitterWebhookEventWithStringPayloadAndSignature,
): Promise<any> {
): Promise<void> {
// verify will validate that the secret is not undefined
const matchesSignature = await verify(
state.secret,
Expand All @@ -26,9 +27,20 @@ export async function verifyAndReceive(
);
}

return state.eventHandler.receive({
id: event.id,
name: event.name,
payload: JSON.parse(event.payload),
});
try {
return state.eventHandler.receive({
id: event.id,
name: event.name,
payload: JSON.parse(event.payload),
});
} catch (error: any) {
error.message = "Invalid JSON";
error.status = 400;
let tmpStackTraceLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
const aggregateError = new AggregateError([error]);
Error.stackTraceLimit = tmpStackTraceLimit;
aggregateError.stack = error.stack;
throw aggregateError;
}
}
6 changes: 4 additions & 2 deletions test/integration/node-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ describe("createNodeMiddleware(webhooks)", () => {
// @ts-expect-error complains about { port } although it's included in returned AddressInfo interface
const { port } = server.address();

const payload = '{"name":"invalid"';

const response = await fetch(
`http://localhost:${port}/api/github/webhooks`,
{
Expand All @@ -186,9 +188,9 @@ describe("createNodeMiddleware(webhooks)", () => {
"Content-Type": "application/json",
"X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000",
"X-GitHub-Event": "push",
"X-Hub-Signature-256": signatureSha256,
"X-Hub-Signature-256": await sign("mySecret", payload),
},
body: "invalid",
body: payload,
},
);

Expand Down

0 comments on commit f3b3d27

Please sign in to comment.