Prevent URL pollution

source/core/index.ts

@@ -589,6 +589,10 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 			if (url) {
 				options.url = url;
 			}
+
+			if (is.urlInstance(options.url)) {
+				options.url = new URL(options.url.toString());
+			}
 		}
 
 		// TODO: Deprecate URL options in Got 12.
@@ -730,9 +734,19 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 				throw new UnsupportedProtocolError(options as NormalizedOptions);
 			}
 
-			// Update `username` & `password`
-			options.url.username = options.username;
-			options.url.password = options.password;
+			// Update `username`
+			if (options.username === '') {
+				options.username = options.url.username;
+			} else {
+				options.url.username = options.username;
+			}
+
+			// Update `password`
+			if (options.password === '') {
+				options.password = options.url.password;
+			} else {
+				options.url.password = options.password;
+			}
 		}
 
 		// `options.cookieJar`

test/arguments.ts

@@ -522,3 +522,22 @@ test('allowGetBody sends json payload', withBodyParsingServer, async (t, server,
 	});
 	t.is(statusCode, 200);
 });
+
+test('no URL pollution', withServer, async (t, server) => {
+	server.get('/ok', echoUrl);
+
+	const url = new URL(server.url);
+
+	const {body} = await got(url, {
+		hooks: {
+			beforeRequest: [
+				options => {
+					options.url.pathname = '/ok';
+				}
+			]
+		}
+	});
+
+	t.is(url.pathname, '/');
+	t.is(body, '/ok');
+});

test/merge-instances.ts

@@ -102,3 +102,17 @@ test('default handlers are not duplicated', t => {
 	const instance = got.extend(got);
 	t.is(instance.defaults.handlers.length, 1);
 });
+
+test('URL is not polluted', withServer, async (t, server, got) => {
+	server.get('/', (_request, response) => {
+		response.end('ok');
+	});
+
+	await got({
+		username: 'foo'
+	});
+
+	const {options: normalizedOptions} = (await got({})).request;
+
+	t.is(normalizedOptions.username, '');
+});

test/normalize-arguments.ts

@@ -44,3 +44,38 @@ test('should replace URLs', t => {
 	t.is(options.url.href, 'http://localhost:41285/?page=1');
 	t.is(otherOptions.url.href, 'http://localhost:41285/?page=1');
 });
+
+test('should get username and password from the URL', t => {
+	const options = got.mergeOptions({
+		url: 'http://user:pass@localhost:41285'
+	});
+
+	t.is(options.username, 'user');
+	t.is(options.password, 'pass');
+});
+
+test('should get username and password from the options', t => {
+	const options = got.mergeOptions({
+		url: 'http://user:pass@localhost:41285',
+		username: 'user_OPT',
+		password: 'pass_OPT'
+	});
+
+	t.is(options.username, 'user_OPT');
+	t.is(options.password, 'pass_OPT');
+});
+
+test('should get username and password from the merged options', t => {
+	const options = got.mergeOptions(
+		{
+			url: 'http://user:pass@localhost:41285'
+		},
+		{
+			username: 'user_OPT_MERGE',
+			password: 'pass_OPT_MERGE'
+		}
+	);
+
+	t.is(options.username, 'user_OPT_MERGE');
+	t.is(options.password, 'pass_OPT_MERGE');
+});