Fix searchParams setter dropping the value when a URL is set (#2454)

Author: chatman-media

source/core/options.ts

@@ -2067,7 +2067,8 @@ export default class Options {
 		if (is.string(value)) {
 			updated = new URLSearchParams(value);
 		} else if (value instanceof URLSearchParams) {
-			updated = value;
+			// Clone so the caller-owned object is not stored by reference.
+			updated = new URLSearchParams(value);
 		} else {
 			validateSearchParameters(value);
 
@@ -2100,9 +2101,10 @@ export default class Options {
 				searchParameters.append(key, value);
 			}
 		} else if (url) {
-			url.search = searchParameters.toString();
+			// Overrides the query string in the URL.
+			url.search = updated.toString();
 		} else {
-			this.#internals.searchParams = searchParameters;
+			this.#internals.searchParams = updated;
 		}
 	}
 

test/hooks.ts

@@ -415,6 +415,44 @@ test('beforeRequest allows modifications', withServer, async (t, server, got) =>
 	t.is(body.foo, 'bar');
 });
 
+test('beforeRequest allows overriding searchParams with an object', withServer, async (t, server, got) => {
+	server.get('/', (request, response) => {
+		response.end(request.url);
+	});
+
+	const {body} = await got('', {
+		searchParams: {old: 'value'},
+		hooks: {
+			beforeRequest: [
+				options => {
+					options.searchParams = {foo: 'bar'};
+				},
+			],
+		},
+	});
+
+	t.is(body, '/?foo=bar');
+});
+
+test('beforeRequest allows overriding searchParams with a string', withServer, async (t, server, got) => {
+	server.get('/', (request, response) => {
+		response.end(request.url);
+	});
+
+	const {body} = await got('', {
+		searchParams: {old: 'value'},
+		hooks: {
+			beforeRequest: [
+				options => {
+					options.searchParams = 'foo=bar';
+				},
+			],
+		},
+	});
+
+	t.is(body, '/?foo=bar');
+});
+
 test('beforeRequest is called with context', withServer, async (t, server, got) => {
 	server.get('/', echoHeaders);
 

test/normalize-arguments.ts

@@ -167,6 +167,18 @@ test('searchParams - multiple values for one key', t => {
 	);
 });
 
+test('searchParams - assigning a URLSearchParams clones it', t => {
+	const searchParameters = new URLSearchParams('foo=bar');
+
+	const options = new Options();
+	options.searchParams = searchParameters;
+
+	// Mutating the caller-owned object must not leak into the stored options.
+	searchParameters.set('foo', 'changed');
+
+	t.is(options.searchParams.toString(), 'foo=bar');
+});
+
 test('__proto__ in options does not cause prototype pollution', t => {
 	const malicious = JSON.parse('{"method": "POST", "__proto__": {"injected": true}}');
 	const options = new Options('https://example.com', malicious);