Skip to content

Commit

Permalink
https: do not automatically use invalid servername
Browse files Browse the repository at this point in the history
Stop automatically setting servername in https.request() if the target
host is specified with an IP address. Doing so is invalid, and triggers
a deprecation warning. It is still possible to send an IP address as a
servername if its required, but it needs to be explicity configured, it
won't happen automatically.

PR-URL: #28209
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
sam-github authored and targos committed Jun 18, 2019
1 parent 9ea74b7 commit faeed80
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 2 deletions.
12 changes: 10 additions & 2 deletions doc/api/https.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,23 @@ An [`Agent`][] object for HTTPS similar to [`http.Agent`][]. See
[`https.request()`][] for more information.

### new Agent([options])

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/28209
description: do not automatically set servername if the target host was
specified using an IP address.
-->
* `options` {Object} Set of configurable options to set on the agent.
Can have the same fields as for [`http.Agent(options)`][], and
* `maxCachedSessions` {number} maximum number of TLS cached sessions.
Use `0` to disable TLS session caching. **Default:** `100`.
* `servername` {string} the value of
[Server Name Indication extension][sni wiki] to be sent to the server. Use
empty string `''` to disable sending the extension.
**Default:** hostname or IP address of the target server.
**Default:** hostname of the target server, unless the target server
is specified using an IP address, in which case the default is `''` (no
extension).

See [`Session Resumption`][] for infomation about TLS session reuse.

Expand Down
3 changes: 3 additions & 0 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ function calculateServerName(options, req) {
servername = hostHeader.split(':', 1)[0];
}
}
// Don't implicitly set invalid (IP) servernames.
if (net.isIP(servername))
servername = '';
return servername;
}

Expand Down
3 changes: 3 additions & 0 deletions test/parallel/test-https-simple.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ if (!common.hasCrypto)
const assert = require('assert');
const https = require('https');

// Assert that the IP-as-servername deprecation warning does not occur.
process.on('warning', common.mustNotCall());

const options = {
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem')
Expand Down

0 comments on commit faeed80

Please sign in to comment.