Skip to content

Commit

Permalink
Make http.request correctly parse {host: "hostname:port"}
Browse files Browse the repository at this point in the history
The issue:
http.request() treats host as an alternate form of hostname, which
it sort of is, except that host is (normally) allowed to contain a
port number.

Before this change, if host contains a port number and there isn't
a hostname field to override it, http.request() attempts a DNS
lookup on the entire host field, including the port number. This
always fails.

The fix:
If a host field is present, use the url lib to parse it and use
the parsed hostname nad port values as fallbacks if
options.hostname and options.port are unset.

Includes tests for localhost:12346 and [::1]:12346.
  • Loading branch information
nfriedly committed Jul 30, 2015
1 parent bc733f7 commit f1622f8
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 2 deletions.
15 changes: 13 additions & 2 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,19 @@ function ClientRequest(options, cb) {
const defaultPort = options.defaultPort ||
self.agent && self.agent.defaultPort;

var port = options.port = options.port || defaultPort || 80;
var host = options.host = options.hostname || options.host || 'localhost';
const parsedHost = {host: options.host};
if (options.host) {
url.Url.prototype.parseHost.call(parsedHost);

// unwrap brackets from ipv6 ip addresses
const hostname = parsedHost.hostname;
if (hostname[0] === '[' && hostname[hostname.length - 1] === ']') {
parsedHost.hostname = hostname.substr(1, hostname.length - 2);
}
}

const port = options.port = options.port || parsedHost.port || defaultPort || 80;
const host = options.host = options.hostname || parsedHost.hostname || 'localhost';

if (options.setHost === undefined) {
var setHost = true;
Expand Down
34 changes: 34 additions & 0 deletions test/parallel/test-http-request-host-port-ipv6.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');

if (!common.hasIPv6) {
console.log('1..0 # Skipped: no IPv6 support');
return;
}

var connected = false;

const server = http.createServer(function(req, res) {
connected = true;
res.writeHead(204);
res.end();
});

server.listen(common.PORT, '::1', function() {
http.get({
host: '[::1]:' + common.PORT
}, function(res) {
res.resume();
server.close();
}).on('error', function(e) {
throw e;
});
});

process.on('exit', function() {
assert(connected, 'http.request should correctly parse ' +
'{host: "hostname:port"} and connect to the specified host');
});
29 changes: 29 additions & 0 deletions test/parallel/test-http-request-host-port.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');

var connected = false;

const server = http.createServer(function(req, res) {
connected = true;
res.writeHead(204);
res.end();
});

server.listen(common.PORT, function() {
http.get({
host: 'localhost:' + common.PORT
}, function(res) {
res.resume();
server.close();
}).on('error', function(e) {
throw e;
});
});

process.on('exit', function() {
assert(connected, 'http.request should correctly parse ' +
'{host: "hostname:port"} and connect to the specified host');
});

0 comments on commit f1622f8

Please sign in to comment.