From 76a8bb49965cd1c56ad9569994492aceb5bce560 Mon Sep 17 00:00:00 2001
From: James <james@eli.cx>
Date: Wed, 12 Jul 2023 23:27:57 +0100
Subject: [PATCH] fix: query params not appending with the same key (#380)

---
 .changeset/neat-terms-boil.md                 |  5 ++
 .../templates/_worker.js/utils/http.ts        | 16 +++---
 .../tests/templates/utils/http.test.ts        | 55 +++++++++++++++++++
 3 files changed, 69 insertions(+), 7 deletions(-)
 create mode 100644 .changeset/neat-terms-boil.md

diff --git a/.changeset/neat-terms-boil.md b/.changeset/neat-terms-boil.md
new file mode 100644
index 000000000..4c6e66ea1
--- /dev/null
+++ b/.changeset/neat-terms-boil.md
@@ -0,0 +1,5 @@
+---
+'@cloudflare/next-on-pages': patch
+---
+
+Fix multiple search params with the same key not being preserved
diff --git a/packages/next-on-pages/templates/_worker.js/utils/http.ts b/packages/next-on-pages/templates/_worker.js/utils/http.ts
index 03085261e..656350c4f 100644
--- a/packages/next-on-pages/templates/_worker.js/utils/http.ts
+++ b/packages/next-on-pages/templates/_worker.js/utils/http.ts
@@ -45,7 +45,7 @@ export function isUrl(url: string): boolean {
 /**
  * Merges search params from one URLSearchParams object to another.
  *
- * Only updates the a parameter if the target does not contain it, or the source value is not empty.
+ * Only appends the parameter if the target does not contain it, or if the value is different and not undefined.
  *
  * For params prefixed with `nxtP`, it also sets the param without the prefix if it does not exist.
  * The `nxtP` prefix indicates that it is for Next.js dynamic route parameters. In some cases,
@@ -60,13 +60,15 @@ export function applySearchParams(
 	source: URLSearchParams,
 ) {
 	for (const [key, value] of source.entries()) {
-		if (!target.has(key) || !!value) {
+		const paramMatch = /^nxtP(.+)$/.exec(key);
+		if (paramMatch?.[1]) {
 			target.set(key, value);
-
-			const paramMatch = /^nxtP(.+)$/.exec(key);
-			if (paramMatch?.[1] && !target.has(paramMatch[1])) {
-				target.set(paramMatch[1], value);
-			}
+			target.set(paramMatch[1], value);
+		} else if (
+			!target.has(key) ||
+			(!!value && !target.getAll(key).includes(value))
+		) {
+			target.append(key, value);
 		}
 	}
 }
diff --git a/packages/next-on-pages/tests/templates/utils/http.test.ts b/packages/next-on-pages/tests/templates/utils/http.test.ts
index dc8460ccd..0ce790c26 100644
--- a/packages/next-on-pages/tests/templates/utils/http.test.ts
+++ b/packages/next-on-pages/tests/templates/utils/http.test.ts
@@ -81,6 +81,61 @@ describe('applySearchParams', () => {
 			'http://localhost/page?other=value&foo=bar',
 		);
 	});
+
+	test('allows multiple query params with the same key', () => {
+		const source = new URL('http://localhost/page?foo=bar');
+		const target = new URL(
+			'http://localhost/page?other=value&foo=baz&foo=test',
+		);
+
+		expect([...source.searchParams.entries()].length).toEqual(1);
+		expect([...target.searchParams.entries()].length).toEqual(3);
+
+		applySearchParams(target.searchParams, source.searchParams);
+
+		expect([...source.searchParams.entries()].length).toEqual(1);
+		expect([...target.searchParams.entries()].length).toEqual(4);
+
+		expect(target.toString()).toEqual(
+			'http://localhost/page?other=value&foo=baz&foo=test&foo=bar',
+		);
+	});
+
+	test('multiple query params with the same key must be unique values', () => {
+		const source = new URL('http://localhost/page?foo=bar&foo=baz&foo=baz');
+		const target = new URL('http://localhost/page?other=value&foo=baz');
+
+		expect([...source.searchParams.entries()].length).toEqual(3);
+		expect([...target.searchParams.entries()].length).toEqual(2);
+
+		applySearchParams(target.searchParams, source.searchParams);
+
+		expect([...source.searchParams.entries()].length).toEqual(3);
+		expect([...target.searchParams.entries()].length).toEqual(3);
+
+		expect(target.toString()).toEqual(
+			'http://localhost/page?other=value&foo=baz&foo=bar',
+		);
+	});
+
+	test('Next.js page params (nxtP) always override', () => {
+		const source = new URL('http://localhost/page?nxtPfoo=bar');
+		const target = new URL(
+			'http://localhost/page?other=value&foo=baz&foo=test',
+		);
+
+		expect([...source.searchParams.entries()].length).toEqual(1);
+		expect([...target.searchParams.entries()].length).toEqual(3);
+
+		applySearchParams(target.searchParams, source.searchParams);
+
+		expect([...source.searchParams.entries()].length).toEqual(1);
+		expect([...target.searchParams.entries()].length).toEqual(3);
+
+		expect(target.toString()).toEqual(
+			'http://localhost/page?other=value&foo=bar&nxtPfoo=bar',
+		);
+	});
 });
 
 describe('createRouteRequest', () => {