From e2ba2e82357633ed89018baa03080beac9ae7106 Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Mon, 28 Oct 2024 22:09:53 +1100 Subject: [PATCH 01/11] encode "+" character in query string --- packages/open-next/src/core/routing/matcher.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/open-next/src/core/routing/matcher.ts b/packages/open-next/src/core/routing/matcher.ts index 81537b403..5878cfa29 100644 --- a/packages/open-next/src/core/routing/matcher.ts +++ b/packages/open-next/src/core/routing/matcher.ts @@ -200,10 +200,16 @@ export function handleRewrites( ); // We need to use a localized path if the rewrite is not locale specific const pathToUse = rewrite.locale === false ? rawPath : localizedRawPath; + // We need to encode the "+" character with its UTF-8 equivalent "%20" to avoid 2 issues: + // 1. The compile function from path-to-regexp will throw an error if it finds a "+" character. + // https://github.com/pillarjs/path-to-regexp?tab=readme-ov-file#unexpected--or- + // 2. The convertToQueryString function will replace the "+" character with %2B instead of %20. + // %2B does not get interpreted as a space in the URL thus breaking the query string. + const encodePlusQueryString = queryString.replaceAll("+", "%20") debug("urlParts", { pathname, protocol, hostname, queryString }); const toDestinationPath = compile(escapeRegex(pathname ?? "") ?? ""); const toDestinationHost = compile(escapeRegex(hostname ?? "") ?? ""); - const toDestinationQuery = compile(escapeRegex(queryString ?? "") ?? ""); + const toDestinationQuery = compile(escapeRegex(encodePlusQueryString ?? "") ?? ""); let params = { // params for the source ...getParamsFromSource(match(escapeRegex(rewrite?.source) ?? ""))( @@ -219,7 +225,7 @@ export function handleRewrites( }, {}), }; const isUsingParams = Object.keys(params).length > 0; - let rewrittenQuery = queryString; + let rewrittenQuery = encodePlusQueryString; let rewrittenHost = hostname; let rewrittenPath = pathname; if (isUsingParams) { From 0d9927d26d670f950be57f49b9090868e77e211b Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Mon, 28 Oct 2024 22:10:15 +1100 Subject: [PATCH 02/11] Decode query string for URLSearchParams --- packages/open-next/src/core/routing/util.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/open-next/src/core/routing/util.ts b/packages/open-next/src/core/routing/util.ts index a29b96efd..37b305039 100644 --- a/packages/open-next/src/core/routing/util.ts +++ b/packages/open-next/src/core/routing/util.ts @@ -98,12 +98,15 @@ export function convertRes(res: OpenNextNodeResponse): InternalResult { * @__PURE__ */ export function convertToQueryString(query: Record) { + // URLSearchParams is a a representation of the PARSED query. + // So we must decode the value before appending it to the URLSearchParams. + // https://stackoverflow.com/a/45516812 const urlQuery = new URLSearchParams(); Object.entries(query).forEach(([key, value]) => { if (Array.isArray(value)) { - value.forEach((entry) => urlQuery.append(key, entry)); + value.forEach((entry) => urlQuery.append(key, decodeURIComponent(entry))); } else { - urlQuery.append(key, value); + urlQuery.append(key, decodeURIComponent(value)); } }); const queryString = urlQuery.toString(); From d5f294d753d6d4a32065b5b224d100573fcc1854 Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Mon, 28 Oct 2024 22:18:48 +1100 Subject: [PATCH 03/11] add test case for + character and query string --- .../tests/core/routing/matcher.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/tests-unit/tests/core/routing/matcher.test.ts b/packages/tests-unit/tests/core/routing/matcher.test.ts index 360a6947f..042a05c2d 100644 --- a/packages/tests-unit/tests/core/routing/matcher.test.ts +++ b/packages/tests-unit/tests/core/routing/matcher.test.ts @@ -266,6 +266,24 @@ describe("handleRedirects", () => { expect(result).toBeUndefined(); }); + + it("should redirect with + character and query string", () => { + const event = createEvent({ + url: "/foo", + }); + + const result = handleRedirects(event, [ + { + source: "/foo", + destination: "/search?bar=hello+world&baz=new%2C+earth", + internal: true, + statusCode: 308 + }, + ]); + + expect(result.statusCode).toEqual(308); + expect(result.headers.Location).toEqual("/search?bar=hello+world&baz=new%2C+earth"); + }); }); describe("handleRewrites", () => { From dbb57ba71a35a5594952ef36c3c42a2b77f6be7d Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Wed, 30 Oct 2024 10:19:53 +1100 Subject: [PATCH 04/11] fix format with prettier --- packages/open-next/src/core/routing/matcher.ts | 6 ++++-- packages/tests-unit/tests/core/routing/matcher.test.ts | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/open-next/src/core/routing/matcher.ts b/packages/open-next/src/core/routing/matcher.ts index 5878cfa29..0558be086 100644 --- a/packages/open-next/src/core/routing/matcher.ts +++ b/packages/open-next/src/core/routing/matcher.ts @@ -205,11 +205,13 @@ export function handleRewrites( // https://github.com/pillarjs/path-to-regexp?tab=readme-ov-file#unexpected--or- // 2. The convertToQueryString function will replace the "+" character with %2B instead of %20. // %2B does not get interpreted as a space in the URL thus breaking the query string. - const encodePlusQueryString = queryString.replaceAll("+", "%20") + const encodePlusQueryString = queryString.replaceAll("+", "%20"); debug("urlParts", { pathname, protocol, hostname, queryString }); const toDestinationPath = compile(escapeRegex(pathname ?? "") ?? ""); const toDestinationHost = compile(escapeRegex(hostname ?? "") ?? ""); - const toDestinationQuery = compile(escapeRegex(encodePlusQueryString ?? "") ?? ""); + const toDestinationQuery = compile( + escapeRegex(encodePlusQueryString ?? "") ?? "", + ); let params = { // params for the source ...getParamsFromSource(match(escapeRegex(rewrite?.source) ?? ""))( diff --git a/packages/tests-unit/tests/core/routing/matcher.test.ts b/packages/tests-unit/tests/core/routing/matcher.test.ts index 042a05c2d..9ccf33dd9 100644 --- a/packages/tests-unit/tests/core/routing/matcher.test.ts +++ b/packages/tests-unit/tests/core/routing/matcher.test.ts @@ -277,12 +277,14 @@ describe("handleRedirects", () => { source: "/foo", destination: "/search?bar=hello+world&baz=new%2C+earth", internal: true, - statusCode: 308 + statusCode: 308, }, ]); expect(result.statusCode).toEqual(308); - expect(result.headers.Location).toEqual("/search?bar=hello+world&baz=new%2C+earth"); + expect(result.headers.Location).toEqual( + "/search?bar=hello+world&baz=new%2C+earth", + ); }); }); From fc64a2c3cd3d3e615d437de5b7d3ed7068c1ce18 Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Wed, 30 Oct 2024 12:20:30 +1100 Subject: [PATCH 05/11] fix up comment --- packages/open-next/src/core/routing/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/open-next/src/core/routing/util.ts b/packages/open-next/src/core/routing/util.ts index 37b305039..90d16b181 100644 --- a/packages/open-next/src/core/routing/util.ts +++ b/packages/open-next/src/core/routing/util.ts @@ -98,7 +98,7 @@ export function convertRes(res: OpenNextNodeResponse): InternalResult { * @__PURE__ */ export function convertToQueryString(query: Record) { - // URLSearchParams is a a representation of the PARSED query. + // URLSearchParams is a representation of the PARSED query. // So we must decode the value before appending it to the URLSearchParams. // https://stackoverflow.com/a/45516812 const urlQuery = new URLSearchParams(); From ea90c3c40e1a7a8cc8fdfa1a83cb553f369406ae Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Thu, 31 Oct 2024 10:21:21 +1100 Subject: [PATCH 06/11] internal redirect expects result to be undefined --- packages/tests-unit/tests/core/routing/matcher.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/tests-unit/tests/core/routing/matcher.test.ts b/packages/tests-unit/tests/core/routing/matcher.test.ts index 9ccf33dd9..74a35568c 100644 --- a/packages/tests-unit/tests/core/routing/matcher.test.ts +++ b/packages/tests-unit/tests/core/routing/matcher.test.ts @@ -281,10 +281,7 @@ describe("handleRedirects", () => { }, ]); - expect(result.statusCode).toEqual(308); - expect(result.headers.Location).toEqual( - "/search?bar=hello+world&baz=new%2C+earth", - ); + expect(result).toBeUndefined(); }); }); From dab4f8abc3f9c64e310fbe747d711ed6abdefb83 Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Thu, 31 Oct 2024 10:31:01 +1100 Subject: [PATCH 07/11] Revert "internal redirect expects result to be undefined" This reverts commit ea90c3c40e1a7a8cc8fdfa1a83cb553f369406ae. --- packages/tests-unit/tests/core/routing/matcher.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/tests-unit/tests/core/routing/matcher.test.ts b/packages/tests-unit/tests/core/routing/matcher.test.ts index 74a35568c..9ccf33dd9 100644 --- a/packages/tests-unit/tests/core/routing/matcher.test.ts +++ b/packages/tests-unit/tests/core/routing/matcher.test.ts @@ -281,7 +281,10 @@ describe("handleRedirects", () => { }, ]); - expect(result).toBeUndefined(); + expect(result.statusCode).toEqual(308); + expect(result.headers.Location).toEqual( + "/search?bar=hello+world&baz=new%2C+earth", + ); }); }); From a6f5ccb5239ee2d849e3819b701ffcf32bd48383 Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Thu, 31 Oct 2024 14:58:29 +1100 Subject: [PATCH 08/11] internal redirect expects result to be undefined --- packages/tests-unit/tests/core/routing/matcher.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/tests-unit/tests/core/routing/matcher.test.ts b/packages/tests-unit/tests/core/routing/matcher.test.ts index 9ccf33dd9..74a35568c 100644 --- a/packages/tests-unit/tests/core/routing/matcher.test.ts +++ b/packages/tests-unit/tests/core/routing/matcher.test.ts @@ -281,10 +281,7 @@ describe("handleRedirects", () => { }, ]); - expect(result.statusCode).toEqual(308); - expect(result.headers.Location).toEqual( - "/search?bar=hello+world&baz=new%2C+earth", - ); + expect(result).toBeUndefined(); }); }); From 4e1d63a1bab7f518702a3a44543e0619ae18daf9 Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Thu, 31 Oct 2024 19:38:48 +1100 Subject: [PATCH 09/11] Revert "internal redirect expects result to be undefined" This reverts commit a6f5ccb5239ee2d849e3819b701ffcf32bd48383. --- packages/tests-unit/tests/core/routing/matcher.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/tests-unit/tests/core/routing/matcher.test.ts b/packages/tests-unit/tests/core/routing/matcher.test.ts index 74a35568c..9ccf33dd9 100644 --- a/packages/tests-unit/tests/core/routing/matcher.test.ts +++ b/packages/tests-unit/tests/core/routing/matcher.test.ts @@ -281,7 +281,10 @@ describe("handleRedirects", () => { }, ]); - expect(result).toBeUndefined(); + expect(result.statusCode).toEqual(308); + expect(result.headers.Location).toEqual( + "/search?bar=hello+world&baz=new%2C+earth", + ); }); }); From 139912829e72a790a744d4450f42f3eaf1897e53 Mon Sep 17 00:00:00 2001 From: John Nguyen Date: Thu, 31 Oct 2024 19:42:43 +1100 Subject: [PATCH 10/11] remove internal and set locale to false --- packages/tests-unit/tests/core/routing/matcher.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/tests-unit/tests/core/routing/matcher.test.ts b/packages/tests-unit/tests/core/routing/matcher.test.ts index 9ccf33dd9..9dba40561 100644 --- a/packages/tests-unit/tests/core/routing/matcher.test.ts +++ b/packages/tests-unit/tests/core/routing/matcher.test.ts @@ -276,8 +276,9 @@ describe("handleRedirects", () => { { source: "/foo", destination: "/search?bar=hello+world&baz=new%2C+earth", - internal: true, + locale: false, statusCode: 308, + regex: "^(?!/_next)/foo(?:/)?$", }, ]); From 8628f5bd717382a3e9634f31cbeca35f2fe4d86b Mon Sep 17 00:00:00 2001 From: conico974 Date: Thu, 31 Oct 2024 10:17:07 +0100 Subject: [PATCH 11/11] Create big-grapes-hammer.md --- .changeset/big-grapes-hammer.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/big-grapes-hammer.md diff --git a/.changeset/big-grapes-hammer.md b/.changeset/big-grapes-hammer.md new file mode 100644 index 000000000..864c99504 --- /dev/null +++ b/.changeset/big-grapes-hammer.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/aws": patch +--- + +Fix redirect when containing "+" and decode values for URLSearchParams