summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSakthipriyan Vairamani <thechargingvolcano@gmail.com>2015-09-20 13:07:03 +0530
committerSakthipriyan Vairamani <thechargingvolcano@gmail.com>2015-10-27 08:47:23 +0530
commit437930c0cca8b53f5ba6f6d070e2006177585bb7 (patch)
treebaa594df59b57dea3c2234cee82df232ea4e8932
parent3308e5ea2a729b15a8154c4c30b9ebb15ed002db (diff)
downloadandroid-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.js3
-rw-r--r--lib/https.js3
-rw-r--r--test/parallel/test-http-invalid-urls.js19
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);