Repository: sindresorhus/got (ts)

Issue:
Got throws on unsupported protocol
Hey, we've recently started using the latest version of [probe-image-size](https://github.com/nodeca/probe-image-size), which under the hood uses `got`.

Since then, we've seen a bunch of those:

```js
_http_client.js:55
    throw new Error('Protocol "' + protocol + '" not supported. ' +
    ^
2017-05-03T12:32:25.058531056Z 
Error: Protocol "c:" not supported. Expected "http:"
    at new ClientRequest (_http_client.js:55:11)
    at Object.exports.request (http.js:31:10)
    at opts (/usr/src/app/node_modules/got/index.js:33:18)
    at ClientRequest.fn.request.res (/usr/src/app/node_modules/got/index.js:51:5)
    at ClientRequest.g (events.js:286:16)
    at emitOne (events.js:96:13)
    at ClientRequest.emit (events.js:188:7)
    at HTTPParser.parserOnIncomingClient [as onIncoming] (_http_client.js:472:21)
    at HTTPParser.parserOnHeadersComplete (_http_common.js:105:23)
    at TLSSocket.socketOnData (_http_client.js:361:20)
    at emitOne (events.js:96:13)
    at TLSSocket.emit (events.js:188:7)
    at readableAddChunk (_stream_readable.js:172:18)
    at TLSSocket.Readable.push (_stream_readable.js:130:10)
    at TLSWrap.onread (net.js:542:20)
```

We're running an application that requires scraping a wide variety of web pages, which in turn contain an even wider variety of images, which we probe using `got`.

While image urls starting with `c:` are not common at all and will likely not resolve to anything helpful, we'd like to not return 5xx errors when such weird images are found on a page, and fail more helpfully instead.

Adding a simple url validity check before requesting them would solve this.

I already implemented such a check in our application code, which now fails early instead of trying to probe an url that uses a scheme different from `http://` or `https://`.

This isn't enough, as such errors are still coming in, which I've identified as coming from redirects, e.g. <https://tinyurl.com/lbf3aq9> is valid itself, but will 301 redirect to the fictitious and invalid `c://users/pioul/Downloads/icon.jpg`, which will throw.

This fix would probably be as simple as adding a check [around here](https://github.com/sindresorhus/got/blob/7b12b7582d71abc652013d3b967473a776abf481/index.js#L32) – what do you think?

Pull request:
Check protocol before sending request
Fixes #294 by making sure an url has a valid protocol (http/https) before sending a request.

Let me know if any other tweak would make sense before merging this PR!

Patch:
diff --git a/index.js b/index.js
index b7cab05..1f553b9 100644
--- a/index.js
+++ b/index.js
@@ -28,6 +28,11 @@ function requestAsEventEmitter(opts) {
 	let redirectUrl;
 
 	const get = opts => {
+		if (opts.protocol !== 'http:' && opts.protocol !== 'https:') {
+			ee.emit('error', new got.UnsupportedProtocolError(opts));
+			return;
+		}
+
 		const fn = opts.protocol === 'https:' ? https : http;
 
 		const req = fn.request(opts, res => {
@@ -208,11 +213,13 @@ function normalizeArguments(url, opts) {
 
 	opts = Object.assign(
 		{
-			protocol: 'http:',
 			path: '',
 			retries: 5
 		},
 		url,
+		{
+			protocol: url.protocol || 'http:' // Override both null/undefined with default protocol
+		},
 		opts
 	);
 
@@ -364,4 +371,9 @@ got.MaxRedirectsError = createErrorClass('MaxRedirectsError', function (statusCo
 	this.message = 'Redirected 10 times. Aborting.';
 });
 
+got.UnsupportedProtocolError = createErrorClass('UnsupportedProtocolError', function (opts) {
+	stdError.call(this, {}, opts);
+	this.message = `Unsupported protocol "${opts.protocol}"`;
+});
+
 module.exports = got;


Test patch:
diff --git a/test/arguments.js b/test/arguments.js
index a772a57..ac40c96 100644
--- a/test/arguments.js
+++ b/test/arguments.js
@@ -24,12 +24,8 @@ test.before('setup', async () => {
 });
 
 test('url is required', async t => {
-	try {
-		await got();
-		t.fail('Exception was not thrown');
-	} catch (err) {
-		t.regex(err.message, /Parameter `url` must be a string or object, not undefined/);
-	}
+	const err = await t.throws(got());
+	t.regex(err.message, /Parameter `url` must be a string or object, not undefined/);
 });
 
 test('options are optional', async t => {
diff --git a/test/http.js b/test/http.js
index 6b0c997..ad9985c 100644
--- a/test/http.js
+++ b/test/http.js
@@ -67,6 +67,11 @@ test('status code 304 doesn\'t throw', async t => {
 	t.is(response.body, '');
 });
 
+test('invalid protocol throws', async t => {
+	const err = await t.throws(got('c:/nope.com', {json: true}));
+	t.is(err.constructor, got.UnsupportedProtocolError);
+});
+
 test('buffer on encoding === null', async t => {
 	const data = (await got(s.url, {encoding: null})).body;
 	t.truthy(Buffer.isBuffer(data));