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>
This commit is contained in:
parent
3308e5ea2a
commit
437930c0cc
@ -22,6 +22,9 @@ function ClientRequest(options, cb) {
|
|||||||
|
|
||||||
if (typeof options === 'string') {
|
if (typeof options === 'string') {
|
||||||
options = url.parse(options);
|
options = url.parse(options);
|
||||||
|
if (!options.hostname) {
|
||||||
|
throw new Error('Unable to determine the domain name');
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
options = util._extend({}, options);
|
options = util._extend({}, options);
|
||||||
}
|
}
|
||||||
|
@ -163,6 +163,9 @@ exports.Agent = Agent;
|
|||||||
exports.request = function(options, cb) {
|
exports.request = function(options, cb) {
|
||||||
if (typeof options === 'string') {
|
if (typeof options === 'string') {
|
||||||
options = url.parse(options);
|
options = url.parse(options);
|
||||||
|
if (!options.hostname) {
|
||||||
|
throw new Error('Unable to determine the domain name');
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
options = util._extend({}, options);
|
options = util._extend({}, options);
|
||||||
}
|
}
|
||||||
|
19
test/parallel/test-http-invalid-urls.js
Normal file
19
test/parallel/test-http-invalid-urls.js
Normal file
@ -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);
|
Loading…
x
Reference in New Issue
Block a user