From f069954e351fdbcf6645de83d4ec5287f17b33d6 Mon Sep 17 00:00:00 2001 From: Thijs Wijnmaalen Date: Mon, 11 Mar 2024 22:36:23 +0100 Subject: [PATCH 1/2] fix: consider scenario where / is part of query string in withoutTrailingSlash() --- src/utils.ts | 9 ++++----- test/trailing-slash.test.ts | 6 ++++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index e5d3708b..052d4efe 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -84,7 +84,7 @@ export function hasTrailingSlash( respectQueryAndFragment?: boolean ): boolean { if (!respectQueryAndFragment) { - return input.endsWith("/"); + return !input.includes("?") && input.endsWith("/"); } return TRAILING_SLASH_RE.test(input); } @@ -92,7 +92,7 @@ export function hasTrailingSlash( /** * Removes trailing slash from the URL or pathname. * - * If second argument is is true, it will only remove the trailing slash if it's not part of the query or fragment with cost of more expensive operations. + * If second argument is true, it will only remove the trailing slash if it's not part of the query or fragment with cost of more expensive operations. * * @example * @@ -122,10 +122,9 @@ export function withoutTrailingSlash( fragment = input.slice(fragmentIndex); } const [s0, ...s] = path.split("?"); + const cleanPath = s0.endsWith("/") ? s0.slice(0, -1) : s0; return ( - (s0.slice(0, -1) || "/") + - (s.length > 0 ? `?${s.join("?")}` : "") + - fragment + (cleanPath || "/") + (s.length > 0 ? `?${s.join("?")}` : "") + fragment ); } diff --git a/test/trailing-slash.test.ts b/test/trailing-slash.test.ts index 21a1e37b..317aab2d 100644 --- a/test/trailing-slash.test.ts +++ b/test/trailing-slash.test.ts @@ -57,6 +57,8 @@ describe("withoutTrailingSlash, queryParams: false", () => { "foo?123": "foo?123", "foo/?123": "foo/?123", "foo/?123#abc": "foo/?123#abc", + "foo/?k=v": "foo/?k=v", + "foo/?k=/": "foo/?k=/", }; for (const input in tests) { @@ -81,6 +83,10 @@ describe("withoutTrailingSlash, queryParams: true", () => { "foo?123": "foo?123", "foo/?123": "foo?123", "foo/?123#abc": "foo?123#abc", + "foo/?k=123": "foo?k=123", + "foo?k=/": "foo?k=/", + "foo/?k=/": "foo?k=/", + "foo/?k=/&x=y#abc": "foo?k=/&x=y#abc", "/a/#abc": "/a#abc", "/#abc": "/#abc", }; From 2c37ec304815a7f760879081366444382ce65bb3 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Fri, 15 Mar 2024 17:18:49 +0100 Subject: [PATCH 2/2] revert change in `hasTrailingSlash` --- src/utils.ts | 2 +- test/trailing-slash.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 052d4efe..f5b2b4ee 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -84,7 +84,7 @@ export function hasTrailingSlash( respectQueryAndFragment?: boolean ): boolean { if (!respectQueryAndFragment) { - return !input.includes("?") && input.endsWith("/"); + return input.endsWith("/"); } return TRAILING_SLASH_RE.test(input); } diff --git a/test/trailing-slash.test.ts b/test/trailing-slash.test.ts index 317aab2d..af141d0a 100644 --- a/test/trailing-slash.test.ts +++ b/test/trailing-slash.test.ts @@ -58,7 +58,7 @@ describe("withoutTrailingSlash, queryParams: false", () => { "foo/?123": "foo/?123", "foo/?123#abc": "foo/?123#abc", "foo/?k=v": "foo/?k=v", - "foo/?k=/": "foo/?k=/", + "foo/?k=/": "foo/?k=", }; for (const input in tests) {