https: do not automatically use invalid servername

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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

Author: sam-github

doc/api/https.md

@@ -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.
 

lib/_http_agent.js

@@ -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='';
 returnservername;
 }
 

test/parallel/test-https-simple.js

@@ -29,6 +29,9 @@ if (!common.hasCrypto)
 constassert=require('assert');
 consthttps=require('https');
 
+// Assert that the IP-as-servername deprecation warning does not occur.
+process.on('warning',common.mustNotCall());
+
 constoptions={
 key: fixtures.readKey('agent1-key.pem'),
 cert: fixtures.readKey('agent1-cert.pem')