diff options
author | Sakthipriyan Vairamani <thechargingvolcano@gmail.com> | 2015-09-20 13:07:03 +0530 |
---|---|---|
committer | Sakthipriyan Vairamani <thechargingvolcano@gmail.com> | 2015-10-27 08:47:23 +0530 |
commit | 437930c0cca8b53f5ba6f6d070e2006177585bb7 (patch) | |
tree | baa594df59b57dea3c2234cee82df232ea4e8932 | |
parent | 3308e5ea2a729b15a8154c4c30b9ebb15ed002db (diff) | |
download | android-node-v8-437930c0cca8b53f5ba6f6d070e2006177585bb7.tar.gz android-node-v8-437930c0cca8b53f5ba6f6d070e2006177585bb7.tar.bz2 android-node-v8-437930c0cca8b53f5ba6f6d070e2006177585bb7.zip |
http{s}: don't connect to localhost on invalid URL
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: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
-rw-r--r-- | lib/_http_client.js | 3 | ||||
-rw-r--r-- | lib/https.js | 3 | ||||
-rw-r--r-- | test/parallel/test-http-invalid-urls.js | 19 |
3 files changed, 25 insertions, 0 deletions
diff --git a/lib/_http_client.js b/lib/_http_client.js index 201593e61d..02d6e7ed37 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 edf0aa4432..90b6346bd9 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 0000000000..678e8eceeb --- /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); |