Remove trie deduplication (#431)

Author: blakeembrey

package.json

@@ -50,7 +50,7 @@
   "size-limit": [
     {
       "path": "dist/index.js",
-      "limit": "2.15 kB"
+      "limit": "2 kB"
     }
   ],
   "ts-scripts": {

src/cases.spec.ts

@@ -1504,6 +1504,13 @@ export const MATCH_TESTS: MatchTestSet[] = [
           params: { path: ["test"], ext: "html" },
         },
       },
+      {
+        input: "/test/nested.html",
+        expected: {
+          path: "/test/nested.html",
+          params: { path: ["test", "nested"], ext: "html" },
+        },
+      },
       {
         input: "/test.html/nested",
         expected: {
@@ -1550,6 +1557,128 @@ export const MATCH_TESTS: MatchTestSet[] = [
       },
     ],
   },
+  {
+    path: "{*path}",
+    tests: [
+      {
+        input: "",
+        expected: { path: "", params: {} },
+      },
+      {
+        input: "/",
+        expected: { path: "/", params: { path: ["", ""] } },
+      },
+      {
+        input: "/test",
+        expected: { path: "/test", params: { path: ["", "test"] } },
+      },
+      {
+        input: "/test/nested",
+        expected: {
+          path: "/test/nested",
+          params: { path: ["", "test", "nested"] },
+        },
+      },
+    ],
+  },
+  {
+    path: "{*path}",
+    options: { end: false },
+    tests: [
+      {
+        input: "",
+        expected: { path: "", params: {} },
+      },
+      {
+        input: "/",
+        expected: { path: "/", params: { path: ["", ""] } },
+      },
+      {
+        input: "/test",
+        expected: { path: "/test", params: { path: ["", "test"] } },
+      },
+      {
+        input: "/test/nested",
+        expected: {
+          path: "/test/nested",
+          params: { path: ["", "test", "nested"] },
+        },
+      },
+    ],
+  },
+  {
+    path: "/*path",
+    options: { end: false },
+    tests: [
+      {
+        input: "",
+        expected: false,
+      },
+      {
+        input: "/",
+        expected: false,
+      },
+      {
+        input: "/test",
+        expected: { path: "/test", params: { path: ["test"] } },
+      },
+      {
+        input: "/test/nested",
+        expected: {
+          path: "/test/nested",
+          params: { path: ["test", "nested"] },
+        },
+      },
+    ],
+  },
+  {
+    path: "/*path/",
+    options: { end: false },
+    tests: [
+      {
+        input: "/foo/bar/",
+        expected: { path: "/foo/bar/", params: { path: ["foo", "bar"] } },
+      },
+      {
+        input: "/foo/bar/baz",
+        expected: false,
+      },
+      {
+        input: "/foo/bar/baz/",
+        expected: {
+          path: "/foo/bar/baz/",
+          params: { path: ["foo", "bar", "baz"] },
+        },
+      },
+    ],
+  },
+  {
+    path: "/*path.:ext",
+    options: { end: false },
+    tests: [
+      {
+        input: "/foo/bar.html",
+        expected: {
+          path: "/foo/bar.html",
+          params: { path: ["foo", "bar"], ext: "html" },
+        },
+      },
+      {
+        input: "/foo/bar.html/baz",
+        expected: {
+          path: "/foo/bar.html",
+          params: { path: ["foo", "bar"], ext: "html" },
+        },
+      },
+      {
+        input: "/foo/bar.html/baz.html",
+        expected: {
+          path: "/foo/bar.html/baz.html",
+          params: { path: ["foo", "bar.html", "baz"], ext: "html" },
+        },
+      },
+    ],
+  },
 
   /**
    * Longer prefix.
@@ -2239,7 +2368,7 @@ export const MATCH_TESTS: MatchTestSet[] = [
         input: "%25555....222%25",
         expected: {
           path: "%25555....222%25",
-          params: { foo: "555.", bar: ".222" },
+          params: { foo: "555..", bar: "222" },
         },
       },
     ],

src/index.ts

@@ -463,7 +463,8 @@ export function pathToRegexp(
     sensitive = false,
     trailing = true,
   } = options;
-  const root = new SourceNode("^");
+  const keys: Keys = [];
+  const sources: string[] = [];
   const paths: Array<Path | Path[]> = [path];
   let combinations = 0;
 
@@ -481,49 +482,17 @@ export function pathToRegexp(
         throw new PathError("Too many path combinations", data.originalPath);
       }
 
-      let node = root;
-
-      for (const part of toRegExpSource(tokens, delimiter, data.originalPath)) {
-        node = node.add(part.source, part.key);
-      }
-
-      node.add(""); // Mark the end of the source.
+      sources.push(toRegExpSource(tokens, delimiter, keys, data.originalPath));
     });
   }
 
-  const keys: Keys = [];
-  let pattern = toRegExp(root, keys);
+  let pattern = `^(?:${sources.join("|")})`;
   if (trailing) pattern += "(?:" + escape(delimiter) + "$)?";
   pattern += end ? "$" : "(?=" + escape(delimiter) + "|$)";
 
   return { regexp: new RegExp(pattern, sensitive ? "" : "i"), keys };
 }
 
-function toRegExp(node: SourceNode, keys: Keys): string {
-  if (node.key) keys.push(node.key);
-
-  const children = Object.keys(node.children);
-  const text = children
-    .map((id) => toRegExp(node.children[id], keys))
-    .join("|");
-
-  return node.source + (children.length < 2 ? text : `(?:${text})`);
-}
-
-class SourceNode {
-  children: Record<string, SourceNode> = Object.create(null);
-
-  constructor(
-    public source: string,
-    public key?: Key,
-  ) {}
-
-  add(source: string, key?: Key) {
-    const id = source + ":" + (key ? key.name : "");
-    return (this.children[id] ||= new SourceNode(source, key));
-  }
-}
-
 /**
  * Generate a flat list of sequence tokens from the given tokens.
  */
@@ -549,23 +518,16 @@ function flatten(
   callback(result);
 }
 
-/**
- * Simplest token for the trie deduplication.
- */
-interface RegExpPart {
-  source: string;
-  key?: Key;
-}
-
 /**
  * Transform a flat sequence of tokens into a regular expression.
  */
 function toRegExpSource(
   tokens: Exclude<Token, Group>[],
   delimiter: string,
+  keys: Keys,
   originalPath: string | undefined,
-): RegExpPart[] {
-  let result: RegExpPart[] = [];
+): string {
+  let result = "";
   let backtrack = "";
   let wildcardBacktrack = "";
   let prevCaptureType: 0 | 1 | 2 = 0;
@@ -597,7 +559,7 @@ function toRegExpSource(
     const token = tokens[index++];
 
     if (token.type === "text") {
-      result.push({ source: escape(token.value) });
+      result += escape(token.value);
       backtrack += token.value;
       if (prevCaptureType === 2) wildcardBacktrack += token.value;
       if (token.value.includes(delimiter)) hasSegmentCapture = 0;
@@ -613,34 +575,29 @@ function toRegExpSource(
       }
 
       if (token.type === "param") {
-        result.push({
-          source:
-            hasSegmentCapture & 2 // Seen wildcard in segment.
-              ? `(${negate(delimiter, backtrack)}+)`
-              : hasInSegment(index, "wildcard") // See wildcard later in segment.
-                ? `(${negate(delimiter, peekText(index))}+)`
-                : hasSegmentCapture & 1 // Seen parameter in segment.
-                  ? `(${negate(delimiter, backtrack)}+|${escape(backtrack)})`
-                  : `(${negate(delimiter, "")}+?)`,
-          key: token,
-        });
+        result +=
+          hasSegmentCapture & 2 // Seen wildcard in segment.
+            ? `(${negate(delimiter, backtrack)}+)`
+            : hasInSegment(index, "wildcard") // See wildcard later in segment.
+              ? `(${negate(delimiter, peekText(index))}+)`
+              : hasSegmentCapture & 1 // Seen parameter in segment.
+                ? `(${negate(delimiter, backtrack)}+|${escape(backtrack)})`
+                : `(${negate(delimiter, "")}+)`;
 
         hasSegmentCapture |= prevCaptureType = 1;
       } else {
-        result.push({
-          source:
-            hasSegmentCapture & 2 // Seen wildcard in segment.
-              ? `(${negate(backtrack, "")}+)`
-              : wildcardBacktrack // No capture in segment, seen wildcard in path.
-                ? `(${negate(wildcardBacktrack, "")}+|${negate(delimiter, "")}+)`
-                : `([^]+?)`,
-          key: token,
-        });
+        result +=
+          hasSegmentCapture & 2 // Seen wildcard in segment.
+            ? `(${negate(backtrack, "")}+)`
+            : wildcardBacktrack // No capture in segment, seen wildcard in path.
+              ? `(${negate(wildcardBacktrack, "")}+|${negate(delimiter, "")}+)`
+              : `([^]+)`;
 
         wildcardBacktrack = "";
         hasSegmentCapture |= prevCaptureType = 2;
       }
 
+      keys.push(token);
       backtrack = "";
       continue;
     }

src/redos.spec.ts

@@ -4,12 +4,14 @@ import { MATCH_TESTS } from "./cases.spec.js";
 import { describe, expect, it } from "vitest";
 
 describe.concurrent("redos", () => {
-  it.each(MATCH_TESTS.map((x) => [x.path, pathToRegexp(x.path).regexp]))(
-    "%s - %s",
-    { timeout: 10_000 },
-    async (_, regexp) => {
-      const result = await check(regexp.source, regexp.flags);
-      expect(result.status).toBe("safe");
-    },
-  );
+  it.each(
+    // Array regex currently has false positives.
+    MATCH_TESTS.filter((x) => typeof x.path === "string").map((x) => [
+      x.path,
+      pathToRegexp(x.path).regexp,
+    ]),
+  )("%s - %s", { timeout: 10_000 }, async (_, regexp) => {
+    const result = await check(regexp.source, regexp.flags);
+    expect(result.status).toBe("safe");
+  });
 });