From f1622f89897f244e7f30a5956551e09fb91d4822 Mon Sep 17 00:00:00 2001 From: Nathan Friedly Date: Wed, 29 Jul 2015 13:14:47 -0400 Subject: [PATCH] Make http.request correctly parse {host: "hostname:port"} 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. --- lib/_http_client.js | 15 ++++++-- .../test-http-request-host-port-ipv6.js | 34 +++++++++++++++++++ test/parallel/test-http-request-host-port.js | 29 ++++++++++++++++ 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-http-request-host-port-ipv6.js create mode 100644 test/parallel/test-http-request-host-port.js diff --git a/lib/_http_client.js b/lib/_http_client.js index a7d714f7e0b0b2..58195c320b6f13 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -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; diff --git a/test/parallel/test-http-request-host-port-ipv6.js b/test/parallel/test-http-request-host-port-ipv6.js new file mode 100644 index 00000000000000..96e40da60a151d --- /dev/null +++ b/test/parallel/test-http-request-host-port-ipv6.js @@ -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'); +}); diff --git a/test/parallel/test-http-request-host-port.js b/test/parallel/test-http-request-host-port.js new file mode 100644 index 00000000000000..f44d9ef0c8c833 --- /dev/null +++ b/test/parallel/test-http-request-host-port.js @@ -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'); +});