From b891ab9ef6c80a5f7a8730aa05f65485753c8425 Mon Sep 17 00:00:00 2001 From: Blake Embrey Date: Wed, 11 Sep 2024 17:04:23 -0700 Subject: [PATCH 1/3] Add backtrack protection to 6.x --- package-lock.json | 102 ++++++++++++++++++++++++++++++++++++++++++++++ package.json | 1 + redos.ts | 21 ++++++++++ src/index.spec.ts | 61 +++++++-------------------- src/index.ts | 34 ++++++++++++---- 5 files changed, 165 insertions(+), 54 deletions(-) create mode 100644 redos.ts diff --git a/package-lock.json b/package-lock.json index eeeb609..9eb45e4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,6 +14,7 @@ "@types/node": "^20.4.9", "@types/semver": "^7.3.1", "@vitest/coverage-v8": "^1.4.0", + "recheck": "^4.4.5", "semver": "^7.3.5", "size-limit": "^11.1.2", "typescript": "^5.1.6" @@ -2682,6 +2683,67 @@ "node": ">=8.10.0" } }, + "node_modules/recheck": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck/-/recheck-4.4.5.tgz", + "integrity": "sha512-J80Ykhr+xxWtvWrfZfPpOR/iw2ijvb4WY8d9AVoN8oHsPP07JT1rCAalUSACMGxM1cvSocb6jppWFjVS6eTTrA==", + "dev": true, + "engines": { + "node": ">=14" + }, + "optionalDependencies": { + "recheck-jar": "4.4.5", + "recheck-linux-x64": "4.4.5", + "recheck-macos-x64": "4.4.5", + "recheck-windows-x64": "4.4.5" + } + }, + "node_modules/recheck-jar": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck-jar/-/recheck-jar-4.4.5.tgz", + "integrity": "sha512-a2kMzcfr+ntT0bObNLY22EUNV6Z6WeZ+DybRmPOUXVWzGcqhRcrK74tpgrYt3FdzTlSh85pqoryAPmrNkwLc0g==", + "dev": true, + "optional": true + }, + "node_modules/recheck-linux-x64": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck-linux-x64/-/recheck-linux-x64-4.4.5.tgz", + "integrity": "sha512-s8OVPCpiSGw+tLCxH3eei7Zp2AoL22kXqLmEtWXi0AnYNwfuTjZmeLn2aQjW8qhs8ZPSkxS7uRIRTeZqR5Fv/Q==", + "cpu": [ + "x64" + ], + "dev": true, + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/recheck-macos-x64": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck-macos-x64/-/recheck-macos-x64-4.4.5.tgz", + "integrity": "sha512-Ouup9JwwoKCDclt3Na8+/W2pVbt8FRpzjkDuyM32qTR2TOid1NI+P1GA6/VQAKEOjvaxgGjxhcP/WqAjN+EULA==", + "cpu": [ + "x64" + ], + "dev": true, + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/recheck-windows-x64": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck-windows-x64/-/recheck-windows-x64-4.4.5.tgz", + "integrity": "sha512-mkpzLHu9G9Ztjx8HssJh9k/Xm1d1d/4OoT7etHqFk+k1NGzISCRXBD22DqYF9w8+J4QEzTAoDf8icFt0IGhOEQ==", + "cpu": [ + "x64" + ], + "dev": true, + "optional": true, + "os": [ + "win32" + ] + }, "node_modules/restore-cursor": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/restore-cursor/-/restore-cursor-4.0.0.tgz", @@ -5422,6 +5484,46 @@ "picomatch": "^2.2.1" } }, + "recheck": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck/-/recheck-4.4.5.tgz", + "integrity": "sha512-J80Ykhr+xxWtvWrfZfPpOR/iw2ijvb4WY8d9AVoN8oHsPP07JT1rCAalUSACMGxM1cvSocb6jppWFjVS6eTTrA==", + "dev": true, + "requires": { + "recheck-jar": "4.4.5", + "recheck-linux-x64": "4.4.5", + "recheck-macos-x64": "4.4.5", + "recheck-windows-x64": "4.4.5" + } + }, + "recheck-jar": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck-jar/-/recheck-jar-4.4.5.tgz", + "integrity": "sha512-a2kMzcfr+ntT0bObNLY22EUNV6Z6WeZ+DybRmPOUXVWzGcqhRcrK74tpgrYt3FdzTlSh85pqoryAPmrNkwLc0g==", + "dev": true, + "optional": true + }, + "recheck-linux-x64": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck-linux-x64/-/recheck-linux-x64-4.4.5.tgz", + "integrity": "sha512-s8OVPCpiSGw+tLCxH3eei7Zp2AoL22kXqLmEtWXi0AnYNwfuTjZmeLn2aQjW8qhs8ZPSkxS7uRIRTeZqR5Fv/Q==", + "dev": true, + "optional": true + }, + "recheck-macos-x64": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck-macos-x64/-/recheck-macos-x64-4.4.5.tgz", + "integrity": "sha512-Ouup9JwwoKCDclt3Na8+/W2pVbt8FRpzjkDuyM32qTR2TOid1NI+P1GA6/VQAKEOjvaxgGjxhcP/WqAjN+EULA==", + "dev": true, + "optional": true + }, + "recheck-windows-x64": { + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/recheck-windows-x64/-/recheck-windows-x64-4.4.5.tgz", + "integrity": "sha512-mkpzLHu9G9Ztjx8HssJh9k/Xm1d1d/4OoT7etHqFk+k1NGzISCRXBD22DqYF9w8+J4QEzTAoDf8icFt0IGhOEQ==", + "dev": true, + "optional": true + }, "restore-cursor": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/restore-cursor/-/restore-cursor-4.0.0.tgz", diff --git a/package.json b/package.json index df50fe3..e2e097e 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "@types/node": "^20.4.9", "@types/semver": "^7.3.1", "@vitest/coverage-v8": "^1.4.0", + "recheck": "^4.4.5", "semver": "^7.3.5", "size-limit": "^11.1.2", "typescript": "^5.1.6" diff --git a/redos.ts b/redos.ts new file mode 100644 index 0000000..ccf8f07 --- /dev/null +++ b/redos.ts @@ -0,0 +1,21 @@ +import { checkSync } from "recheck"; +import { pathToRegexp } from "./src/index.js"; + +let safe = 0; +let fail = 0; + +const tests = ["/:x{/foobar/:y}?-:z"]; + +for (const path of tests) { + const regexp = pathToRegexp(path); + const result = checkSync(regexp.source, regexp.flags); + if (result.status === "safe") { + safe++; + console.log("Safe:", path, String(regexp)); + } else { + fail++; + console.log("Fail:", path, String(regexp)); + } +} + +console.log("Safe:", safe, "Fail:", fail); diff --git a/src/index.spec.ts b/src/index.spec.ts index f681e87..e50b3fe 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -1353,7 +1353,7 @@ const TESTS: Test[] = [ prefix: ".", suffix: "", modifier: "+", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!\\.)[^\\/#\\?])+?", }, ], [ @@ -1397,7 +1397,7 @@ const TESTS: Test[] = [ prefix: ".", suffix: "", modifier: "", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!\\.)[^\\/#\\?])+?", }, ".", ], @@ -1430,13 +1430,13 @@ const TESTS: Test[] = [ prefix: ".", suffix: "", modifier: "", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!\\.)[^\\/#\\?])+?", }, ], [ ["/route.html", ["/route.html", "route", "html"]], ["/route", null], - ["/route.html.json", ["/route.html.json", "route", "html.json"]], + ["/route.html.json", ["/route.html.json", "route.html", "json"]], ], [ [{}, null], @@ -1459,13 +1459,13 @@ const TESTS: Test[] = [ prefix: ".", suffix: "", modifier: "?", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!\\.)[^\\/#\\?])+?", }, ], [ ["/route", ["/route", "route", undefined]], ["/route.json", ["/route.json", "route", "json"]], - ["/route.json.html", ["/route.json.html", "route", "json.html"]], + ["/route.json.html", ["/route.json.html", "route.json", "html"]], ], [ [{ test: "route" }, "/route"], @@ -1491,13 +1491,13 @@ const TESTS: Test[] = [ prefix: ".", suffix: "", modifier: "?", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!\\.)[^\\/#\\?])+?", }, ], [ ["/route", ["/route", "route", undefined]], ["/route.json", ["/route.json", "route", "json"]], - ["/route.json.html", ["/route.json.html", "route", "json.html"]], + ["/route.json.html", ["/route.json.html", "route.json", "html"]], ], [ [{ test: "route" }, "/route"], @@ -2084,7 +2084,7 @@ const TESTS: Test[] = [ prefix: "", suffix: "", modifier: "?", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!\\()[^\\/#\\?])+?", }, ")", ], @@ -2290,7 +2290,7 @@ const TESTS: Test[] = [ prefix: ".", suffix: "", modifier: "", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!\\.)[^\\/#\\?])+?", }, ], [ @@ -2356,14 +2356,14 @@ const TESTS: Test[] = [ [ { name: "foo", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!\\$)[^\\/#\\?])+?", prefix: "$", suffix: "", modifier: "", }, { name: "bar", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!\\$)[^\\/#\\?])+?", prefix: "$", suffix: "", modifier: "?", @@ -2392,14 +2392,14 @@ const TESTS: Test[] = [ }, { name: "attr2", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!-)[^\\/#\\?])+?", prefix: "-", suffix: "", modifier: "?", }, { name: "attr3", - pattern: "[^\\/#\\?]+?", + pattern: "(?:(?!-)[^\\/#\\?])+?", prefix: "-", suffix: "", modifier: "?", @@ -2597,39 +2597,6 @@ const TESTS: Test[] = [ [{ foo: "#" }, null], ], ], - /** - * https://github.com/pillarjs/path-to-regexp/issues/260 - */ - [ - ":name*", - undefined, - [ - { - name: "name", - prefix: "", - suffix: "", - modifier: "*", - pattern: "[^\\/#\\?]+?", - }, - ], - [["foobar", ["foobar", "foobar"]]], - [[{ name: "foobar" }, "foobar"]], - ], - [ - ":name+", - undefined, - [ - { - name: "name", - prefix: "", - suffix: "", - modifier: "+", - pattern: "[^\\/#\\?]+?", - }, - ], - [["foobar", ["foobar", "foobar"]]], - [[{ name: "foobar" }, "foobar"]], - ], ]; /** diff --git a/src/index.ts b/src/index.ts index 4454098..2b62547 100644 --- a/src/index.ts +++ b/src/index.ts @@ -139,8 +139,7 @@ export interface ParseOptions { */ export function parse(str: string, options: ParseOptions = {}): Token[] { const tokens = lexer(str); - const { prefixes = "./" } = options; - const defaultPattern = `[^${escapeString(options.delimiter || "/#?")}]+?`; + const { prefixes = "./", delimiter = "/#?" } = options; const result: Token[] = []; let key = 0; let i = 0; @@ -166,6 +165,25 @@ export function parse(str: string, options: ParseOptions = {}): Token[] { return result; }; + const isSafe = (value: string): boolean => { + for (const char of delimiter) if (value.indexOf(char) > -1) return true; + return false; + }; + + const safePattern = (prefix: string) => { + const prev = result[result.length - 1]; + const prevText = prefix || (prev && typeof prev === "string" ? prev : ""); + + if (prev && !prevText) { + throw new TypeError( + `No support for parameters without text between them after "${(prev as Key).name}"`, + ); + } + + if (!prevText || isSafe(prevText)) return `[^${escapeString(delimiter)}]+?`; + return `(?:(?!${escapeString(prevText)})[^${escapeString(delimiter)}])+?`; + }; + while (i < tokens.length) { const char = tryConsume("CHAR"); const name = tryConsume("NAME"); @@ -188,7 +206,7 @@ export function parse(str: string, options: ParseOptions = {}): Token[] { name: name || key++, prefix, suffix: "", - pattern: pattern || defaultPattern, + pattern: pattern || safePattern(prefix), modifier: tryConsume("MODIFIER") || "", }); continue; @@ -216,7 +234,7 @@ export function parse(str: string, options: ParseOptions = {}): Token[] { result.push({ name: name || (pattern ? key++ : ""), - pattern: name && !pattern ? defaultPattern : pattern, + pattern: name && !pattern ? safePattern(prefix) : pattern, prefix, suffix, modifier: tryConsume("MODIFIER") || "", @@ -564,10 +582,12 @@ export function tokensToRegexp( } } else { if (token.modifier === "+" || token.modifier === "*") { - route += `((?:${token.pattern})${token.modifier})`; - } else { - route += `(${token.pattern})${token.modifier}`; + throw new TypeError( + `Can not repeat ${token.name} with no prefix or suffix, e.g. "/:param${token.modifier}"`, + ); } + + route += `(${token.pattern})${token.modifier}`; } } else { route += `(?:${prefix}${suffix})${token.modifier}`; From a1cfa60de43e3161a30cea95d01b318451b79d92 Mon Sep 17 00:00:00 2001 From: Blake Embrey Date: Wed, 11 Sep 2024 17:59:43 -0700 Subject: [PATCH 2/3] Increase package size --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e2e097e..2cda434 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "size-limit": [ { "path": "dist.es2015/index.js", - "limit": "2 kB" + "limit": "2.1 kB" } ], "ts-scripts": { From f0b30bbd4fecf8986425dcb9c5b1fcef01860c45 Mon Sep 17 00:00:00 2001 From: Blake Embrey Date: Wed, 11 Sep 2024 18:04:42 -0700 Subject: [PATCH 3/3] Add coverage for errors --- src/index.spec.ts | 18 ++++++++++++++++++ src/index.ts | 4 ++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/index.spec.ts b/src/index.spec.ts index e50b3fe..22b3e91 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -2766,6 +2766,24 @@ describe("path-to-regexp", () => { pathToRegexp.pathToRegexp("/foo?"); }).toThrow(new TypeError("Unexpected MODIFIER at 4, expected END")); }); + + it("should throw on parameters without text between them", () => { + expect(() => { + pathToRegexp.pathToRegexp("/:x:y"); + }).toThrow( + new TypeError( + `Must have text between two parameters, missing text after "x"`, + ), + ); + }); + + it("should throw on unrepeatable params", () => { + expect(() => { + pathToRegexp.pathToRegexp("/foo:x*"); + }).toThrow( + new TypeError(`Can not repeat "x" without a prefix and suffix`), + ); + }); }); describe("tokens", () => { diff --git a/src/index.ts b/src/index.ts index 2b62547..83d00ea 100644 --- a/src/index.ts +++ b/src/index.ts @@ -176,7 +176,7 @@ export function parse(str: string, options: ParseOptions = {}): Token[] { if (prev && !prevText) { throw new TypeError( - `No support for parameters without text between them after "${(prev as Key).name}"`, + `Must have text between two parameters, missing text after "${(prev as Key).name}"`, ); } @@ -583,7 +583,7 @@ export function tokensToRegexp( } else { if (token.modifier === "+" || token.modifier === "*") { throw new TypeError( - `Can not repeat ${token.name} with no prefix or suffix, e.g. "/:param${token.modifier}"`, + `Can not repeat "${token.name}" without a prefix and suffix`, ); }