From 437930c0cca8b53f5ba6f6d070e2006177585bb7 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Sun, 20 Sep 2015 13:07:03 +0530 Subject: [PATCH] http{s}: don't connect to localhost on invalid URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the URL passed to `http{s}.request` or `http{s}.get` is not properly parsable by `url.parse`, we fall back to use `localhost` and port 80. This creates confusing error messages like in this question http://stackoverflow.com/q/32675907/1903116. This patch throws an error message, if `url.parse` fails to parse the URL properly. Previous Discussion: https://github.com/nodejs/node/pull/2966 PR-URL: https://github.com/nodejs/node/pull/2967 Reviewed-By: Сковорода Никита Андреевич Reviewed-By: Evan Lucas Reviewed-By: James M Snell --- lib/_http_client.js | 3 +++ lib/https.js | 3 +++ test/parallel/test-http-invalid-urls.js | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+) create mode 100644 test/parallel/test-http-invalid-urls.js diff --git a/lib/_http_client.js b/lib/_http_client.js index 201593e61da..02d6e7ed374 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -22,6 +22,9 @@ function ClientRequest(options, cb) { if (typeof options === 'string') { options = url.parse(options); + if (!options.hostname) { + throw new Error('Unable to determine the domain name'); + } } else { options = util._extend({}, options); } diff --git a/lib/https.js b/lib/https.js index edf0aa4432f..90b6346bd95 100644 --- a/lib/https.js +++ b/lib/https.js @@ -163,6 +163,9 @@ exports.Agent = Agent; exports.request = function(options, cb) { if (typeof options === 'string') { options = url.parse(options); + if (!options.hostname) { + throw new Error('Unable to determine the domain name'); + } } else { options = util._extend({}, options); } diff --git a/test/parallel/test-http-invalid-urls.js b/test/parallel/test-http-invalid-urls.js new file mode 100644 index 00000000000..678e8eceeb2 --- /dev/null +++ b/test/parallel/test-http-invalid-urls.js @@ -0,0 +1,19 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const http = require('http'); +const https = require('https'); +const error = 'Unable to determine the domain name'; + +function test(host) { + ['get', 'request'].forEach((method) => { + [http, https].forEach((module) => { + assert.throws(() => module[method](host, () => { + throw new Error(`${module}.${method} should not connect to ${host}`); + }), error); + }); + }); +} + +['www.nodejs.org', 'localhost', '127.0.0.1', 'http://:80/'].forEach(test);