Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/middleware/create-middleware.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { WebhookEventName } from "../generated/webhook-identifiers.ts";

import type { Webhooks } from "../index.ts";
import { normalizeTrailingSlashes } from "../normalize-trailing-slashes.ts";
import type { WebhookEventHandlerError } from "../types.ts";
import type { MiddlewareOptions } from "./types.ts";

Expand Down Expand Up @@ -33,14 +34,19 @@ export function createMiddleware(options: CreateMiddlewareOptions) {
webhooks: Webhooks,
options: Required<MiddlewareOptions>,
) {
const middlewarePath = normalizeTrailingSlashes(options.path);

return async function octokitWebhooksMiddleware(
request: IncomingMessage,
response?: ServerResponse,
next?: Function,
) {
let pathname: string;
try {
pathname = new URL(request.url as string, "http://localhost").pathname;
pathname = new URL(
normalizeTrailingSlashes(request.url) as string,
"http://localhost",
).pathname;
} catch (error) {
return handleResponse(
JSON.stringify({
Expand All @@ -54,7 +60,7 @@ export function createMiddleware(options: CreateMiddlewareOptions) {
);
}

if (pathname !== options.path) {
if (pathname !== middlewarePath) {
next?.();
return handleResponse(null);
} else if (request.method !== "POST") {
Expand Down
19 changes: 19 additions & 0 deletions src/normalize-trailing-slashes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export function normalizeTrailingSlashes(path: string) {
let i = path.length;

if (i === 0) {
return "/";
}

while (i > 0) {
if (path.charCodeAt(--i) !== 47 /* '/' */) {
break;
}
}

if (i === -1) {
return "/";
}

return path.slice(0, i + 1);
}
87 changes: 40 additions & 47 deletions test/integration/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,53 +454,6 @@ runtimes.forEach((runtimeCase) => {
assert((await response.text()) === "still processing\n");
});

it("Handles invalid URL", async () => {
const webhooks = new Webhooks({
secret: "mySecret",
});

let middlewareWasRan: () => void;
const untilMiddlewareIsRan = new Promise<void>(function (resolve) {
middlewareWasRan = resolve;
});
const actualMiddleware = createNodeMiddleware(webhooks);
const mockedMiddleware = async function (
...[req, ...rest]: Parameters<typeof actualMiddleware>
) {
req.url = "//";
await actualMiddleware(req, ...rest);
middlewareWasRan();
};

const server = createServer(mockedMiddleware).listen(await getPort());

const { port } = server.address() as AddressInfo;

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,
},
);

await untilMiddlewareIsRan;

assert(response.status === 422);
assert(
(await response.text()) ===
'{"error":"Request URL could not be parsed: //"}',
);

server.close();
});

it("Handles invalid signature", async () => {
const webhooks = new Webhooks({
secret: "mySecret",
Expand Down Expand Up @@ -591,5 +544,45 @@ runtimes.forEach((runtimeCase) => {

closeTestServer();
});

["/", "/api/github/webhooks"].forEach((webhooksPath) => {
["", "/", "//", "///"].forEach((trailing) => {
it(`Handles trailing slashes - webhooksPath "${webhooksPath}" with trailing "${trailing}"`, async () => {
const webhooks = new Webhooks({
secret: "mySecret",
});

webhooks.on("push", (event) => {
assert(event.id === "123e4567-e89b-12d3-a456-426655440000");
});

const { port, closeTestServer } = await instantiateTestServer(
runtime,
target,
webhooks,
{ path: webhooksPath },
);

const response = await fetch(
`http://localhost:${port}${webhooksPath}${trailing}`,
{
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,
},
);

assert(response.status === 200);
assert((await response.text()) === "ok\n");

closeTestServer();
});
});
});
});
});
21 changes: 21 additions & 0 deletions test/integration/normalize-trailing-slashes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { describe, it, assert } from "../testrunner.ts";
import { normalizeTrailingSlashes } from "../../src/normalize-trailing-slashes.ts";

describe("normalizeTrailingSlashes", () => {
[
["", "/"],
["/", "/"],
["//", "/"],
["///", "/"],
["/a", "/a"],
["/a/", "/a"],
["/a//", "/a"],
].forEach(([actual, expected]) => {
it(`should normalize trailing slashes from ${actual} to ${expected}`, () => {
assert(
normalizeTrailingSlashes(actual) === expected,
`Expected ${actual} to be stripped to ${expected}, but got ${normalizeTrailingSlashes(actual)}`,
);
});
});
});