fix: normalize trailing slashes (#1167)

* fix: strip-trailing-slashes

* improve

* no regex

* make it a normalize function

* fix prettier linting issue

Author: Uzlopak

src/middleware/create-middleware.ts

@@ -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";
 
@@ -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({
@@ -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") {

src/normalize-trailing-slashes.ts

@@ -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);
+}

test/integration/middleware.test.ts

@@ -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",
@@ -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();
+        });
+      });
+    });
   });
 });

test/integration/normalize-trailing-slashes.test.ts

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