Skip to content

Commit

Permalink
http: Corrects IPv6 address in Host header
Browse files Browse the repository at this point in the history
IPv6 addresses in Host header (URI), must be enclosed within
square brackets, in order to properly separate the host address
from any port reference.

PR-URL: #5314
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
  • Loading branch information
mpotra authored and jasnell committed Apr 1, 2016
1 parent 5fc6938 commit dabe1d5
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 0 deletions.
11 changes: 11 additions & 0 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ function ClientRequest(options, cb) {
}
if (host && !this.getHeader('host') && setHost) {
var hostHeader = host;
var posColon = -1;

// For the Host header, ensure that IPv6 addresses are enclosed
// in square brackets, as defined by URI formatting
// https://tools.ietf.org/html/rfc3986#section-3.2.2
if (-1 !== (posColon = hostHeader.indexOf(':')) &&
-1 !== (posColon = hostHeader.indexOf(':', posColon)) &&

This comment has been minimized.

Copy link
@ashsearle

ashsearle Apr 11, 2016

Bug: if this is supposed to check for a second ':' then it should be indexOf(':', posColon + 1)

'[' !== hostHeader[0]) {
hostHeader = `[${hostHeader}]`;
}

if (port && +port !== defaultPort) {
hostHeader += ':' + port;
}
Expand Down
40 changes: 40 additions & 0 deletions test/parallel/test-http-host-header-ipv6-fail.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict';
/*
* When using the object form of http.request and using an IPv6 address
* as a hostname, and using a non-standard port, the Host header
* is improperly formatted.
* Issue: https://github.com/nodejs/node/issues/5308
* As per https://tools.ietf.org/html/rfc7230#section-5.4 and
* https://tools.ietf.org/html/rfc3986#section-3.2.2
* the IPv6 address should be enclosed in square brackets
*/

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

const hostname = '::1';
const port = common.PORT;

function httpreq() {
var req = http.request({
host: hostname,
port: port,
path: '/',
method: 'GET'
});
req.end();
}

if (!common.hasIPv6) {
console.error('Skipping test, no IPv6 support');
return;
}

const server = http.createServer(common.mustCall(function(req, res) {
assert.ok(req.headers.host, `[${hostname}]`);

This comment has been minimized.

Copy link
@ashsearle

ashsearle Apr 11, 2016

Bug: This isn't checking the content of req.headers.host, it's simply checking it's truthy. This probably needs to change to something like:

assert.equal(req.headers.host.indexOf(`[${hostname}]`), 0);
res.end();
server.close(true);
}));

server.listen(port, hostname, () => httpreq());

0 comments on commit dabe1d5

Please sign in to comment.