Skip to content

Commit

Permalink
improve performance of headers check
Browse files Browse the repository at this point in the history
  • Loading branch information
Uzlopak committed Jul 21, 2024
1 parent c9df94f commit 0b88ef7
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 34 deletions.
15 changes: 0 additions & 15 deletions src/middleware/node/get-missing-headers.ts

This file was deleted.

22 changes: 5 additions & 17 deletions src/middleware/node/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type { WebhookEventName } from "../../generated/webhook-identifiers.js";
import type { Webhooks } from "../../index.js";
import type { WebhookEventHandlerError } from "../../types.js";
import type { MiddlewareOptions } from "./types.js";
import { getMissingHeaders } from "./get-missing-headers.js";
import { validateHeaders } from "./validate-headers.js";
import { getPayload } from "./get-payload.js";

type Handler = (
Expand Down Expand Up @@ -44,18 +44,7 @@ export function createNodeHandler(
return true;
}

const missingHeaders = getMissingHeaders(request).join(", ");

if (missingHeaders) {
response.writeHead(400, {
"content-type": "application/json",
});
response.end(
JSON.stringify({
error: `Required headers missing: ${missingHeaders}`,
}),
);

if (validateHeaders(request, response)) {
return true;
}

Expand Down Expand Up @@ -98,14 +87,13 @@ export function createNodeHandler(
const errorMessage = err.message
? `${err.name}: ${err.message}`
: "Error: An Unspecified error occurred";

const statusCode =
typeof err.status !== "undefined" ? err.status : 500;

const statusCode = typeof err.status !== "undefined" ? err.status : 500;

logger.error(error);

response.writeHead(statusCode, {
"content-type": "application/json"
"content-type": "application/json",
});

response.end(
Expand Down
33 changes: 33 additions & 0 deletions src/middleware/node/validate-headers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// remove type imports from http for Deno compatibility
// see https://github.com/octokit/octokit.js/issues/24#issuecomment-817361886
// import type { IncomingMessage } from "node:http";
type IncomingMessage = any;
type OutgoingMessage = any;

function sendMissindHeaderResponse(
response: OutgoingMessage,
missingHeader: string,
) {
response.writeHead(400, {
"content-type": "application/json",
});
response.end(`{\"error\": \"Required header missing: ${missingHeader}\"}`);
}

// https://docs.github.com/en/developers/webhooks-and-events/webhook-events-and-payloads#delivery-headers
export function validateHeaders(
request: IncomingMessage,
response: OutgoingMessage,
) {
if ("x-github-event" in request.headers === false) {
sendMissindHeaderResponse(response, "x-github-event");
} else if ("x-hub-signature-256" in request.headers === false) {
sendMissindHeaderResponse(response, "x-hub-signature-256");
} else if ("x-github-delivery" in request.headers === false) {
sendMissindHeaderResponse(response, "x-github-delivery");
} else {
return false;
}

return true;
}
2 changes: 1 addition & 1 deletion test/integration/node-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ describe("createNodeHandler(webhooks)", () => {
expect(response.status).toEqual(400);

await expect(response.text()).resolves.toMatch(
/Required headers missing: x-github-event/,
/Required header missing: x-github-event/,
);

server.close();
Expand Down
2 changes: 1 addition & 1 deletion test/integration/node-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ describe("createNodeMiddleware(webhooks)", () => {
expect(response.status).toEqual(400);

await expect(response.text()).resolves.toMatch(
/Required headers missing: x-github-event/,
/Required header missing: x-github-event/,
);

server.close();
Expand Down

0 comments on commit 0b88ef7

Please sign in to comment.