Repository: sindresorhus/got
Pull request #515 (2018-07-11 13:47:13)

Fix null search being appended to path when legacy parsing URL
This still needs to be rebased after #513 is merged. 

alextes: Ah and same problem with `hash` of course.

Patch:
diff --git a/source/as-promise.js b/source/as-promise.js
index 00808a6..4edef58 100644
--- a/source/as-promise.js
+++ b/source/as-promise.js
@@ -77,9 +77,11 @@ module.exports = options => {
 		});
 
 		emitter.once('error', reject);
-		emitter.on('redirect', proxy.emit.bind(proxy, 'redirect'));
-		emitter.on('uploadProgress', proxy.emit.bind(proxy, 'uploadProgress'));
-		emitter.on('downloadProgress', proxy.emit.bind(proxy, 'downloadProgress'));
+		[
+			'redirect',
+			'uploadProgress',
+			'downloadProgress'
+		].forEach(event => emitter.on(event, (...args) => proxy.emit(event, ...args)));
 	});
 
 	const promise = cancelable;
diff --git a/source/as-stream.js b/source/as-stream.js
index 0118272..c5e359b 100644
--- a/source/as-stream.js
+++ b/source/as-stream.js
@@ -60,10 +60,12 @@ module.exports = options => {
 		proxy.emit('response', response);
 	});
 
-	emitter.on('error', proxy.emit.bind(proxy, 'error'));
-	emitter.on('redirect', proxy.emit.bind(proxy, 'redirect'));
-	emitter.on('uploadProgress', proxy.emit.bind(proxy, 'uploadProgress'));
-	emitter.on('downloadProgress', proxy.emit.bind(proxy, 'downloadProgress'));
+	[
+		'error',
+		'redirect',
+		'uploadProgress',
+		'downloadProgress'
+	].forEach(event => emitter.on(event, (...args) => proxy.emit(event, ...args)));
 
 	return proxy;
 };
diff --git a/source/assign-options.js b/source/assign-options.js
index efe18d8..dc3a2b2 100644
--- a/source/assign-options.js
+++ b/source/assign-options.js
@@ -8,7 +8,6 @@ module.exports = (defaults, options = {}) => {
 		for (const [key, value] of Object.entries(options.headers)) {
 			if (is.nullOrUndefined(value)) {
 				delete opts.headers[key];
-				continue;
 			}
 		}
 	}
diff --git a/source/url-to-options.js b/source/url-to-options.js
index df94c97..848ef30 100644
--- a/source/url-to-options.js
+++ b/source/url-to-options.js
@@ -1,6 +1,6 @@
 'use strict';
+const is = require('@sindresorhus/is');
 
-// From: https://github.com/nodejs/node/blob/8476053c132fd9613aab547aba165190f8064254/lib/internal/url.js#L1318-L1340
 module.exports = url => {
 	const options = {
 		protocol: url.protocol,
@@ -8,11 +8,10 @@ module.exports = url => {
 		hash: url.hash,
 		search: url.search,
 		pathname: url.pathname,
-		path: `${url.pathname}${url.search}`,
 		href: url.href
 	};
 
-	if (url.port !== '') {
+	if (is.string(url.port) && url.port.length > 0) {
 		options.port = Number(url.port);
 	}
 
@@ -20,5 +19,7 @@ module.exports = url => {
 		options.auth = `${url.username}:${url.password}`;
 	}
 
+	options.path = is.null(url.search) ? url.pathname : `${url.pathname}${url.search}`;
+
 	return options;
 };


Test patch:
diff --git a/test/promise.js b/test/promise.js
index 471e3c7..dbb7e59 100644
--- a/test/promise.js
+++ b/test/promise.js
@@ -1,3 +1,5 @@
+import {ClientRequest} from 'http';
+import {Transform} from 'stream';
 import test from 'ava';
 import {createServer} from './helpers/server';
 import got from '..';
@@ -14,14 +16,16 @@ test.before('setup', async () => {
 });
 
 test('should emit request event as promise', async t => {
-	await got(s.url, {json: true}).on('request', () => {
-		t.pass();
+	await got(s.url, {json: true}).on('request', req => {
+		t.true(req instanceof ClientRequest);
 	});
 });
 
 test('should emit response event as promise', async t => {
-	await got(s.url, {json: true}).on('response', () => {
-		t.pass();
+	await got(s.url, {json: true}).on('response', res => {
+		t.true(res instanceof Transform);
+		t.true(res.readable);
+		t.is(res.statusCode, 200);
 	});
 });
 
diff --git a/test/timeout.js b/test/timeout.js
index 9abda99..ba922cf 100644
--- a/test/timeout.js
+++ b/test/timeout.js
@@ -5,7 +5,7 @@ import got from '../source';
 import {createServer} from './helpers/server';
 
 let s;
-const reqDelay = 80;
+const reqDelay = 160;
 
 test.before('setup', async () => {
 	s = await createServer();
diff --git a/test/url-to-options.js b/test/url-to-options.js
index 13890ad..e301826 100644
--- a/test/url-to-options.js
+++ b/test/url-to-options.js
@@ -2,9 +2,8 @@ import url from 'url';
 import test from 'ava';
 import urlToOptions from '../source/url-to-options';
 
-const exampleURL = 'https://user:password@github.com:443/say?hello=world#bang';
-
 test('converts node legacy URL to options', t => {
+	const exampleURL = 'https://user:password@github.com:443/say?hello=world#bang';
 	const parsedURL = url.parse(exampleURL);
 	const options = urlToOptions(parsedURL);
 	const expected = {
@@ -22,6 +21,7 @@ test('converts node legacy URL to options', t => {
 });
 
 test('converts URL to options', t => {
+	const exampleURL = 'https://user:password@github.com:443/say?hello=world#bang';
 	// TODO: Use the `URL` global when targeting Node.js 10
 	const parsedURL = new url.URL(exampleURL);
 	const options = urlToOptions(parsedURL);
@@ -74,3 +74,43 @@ test('only adds port to options for URLs with ports', t => {
 	t.deepEqual(options, expected);
 	t.false(Reflect.has(options, 'port'));
 });
+
+test('does not concat null search to path', t => {
+	const exampleURL = 'https://github.com/';
+	const parsedURL = url.parse(exampleURL);
+
+	t.is(parsedURL.search, null);
+
+	const options = urlToOptions(parsedURL);
+	const expected = {
+		hash: null,
+		hostname: 'github.com',
+		href: 'https://github.com/',
+		path: '/',
+		pathname: '/',
+		protocol: 'https:',
+		search: null
+	};
+
+	t.deepEqual(options, expected);
+});
+
+test('does not add null port to options', t => {
+	const exampleURL = 'https://github.com/';
+	const parsedURL = url.parse(exampleURL);
+
+	t.is(parsedURL.port, null);
+
+	const options = urlToOptions(parsedURL);
+	const expected = {
+		hash: null,
+		hostname: 'github.com',
+		href: 'https://github.com/',
+		path: '/',
+		pathname: '/',
+		protocol: 'https:',
+		search: null
+	};
+
+	t.deepEqual(options, expected);
+});