From 53cc16c7ddd6a7487e17d94a28d313369faafd5f Mon Sep 17 00:00:00 2001
From: Sam Roberts <vieuxtech@gmail.com>
Date: Sun, 2 Jun 2019 18:11:48 +0200
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/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>
---
 doc/api/https.md                   | 12 ++++++++++--
 lib/_http_agent.js                 |  3 +++
 test/parallel/test-https-simple.js |  3 +++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/doc/api/https.md b/doc/api/https.md
index 9b6ec83a6e7796..b544b420a1fef8 100644
--- a/doc/api/https.md
+++ b/doc/api/https.md
@@ -24,7 +24,13 @@ 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.
@@ -32,7 +38,9 @@ An [`Agent`][] object for HTTPS similar to [`http.Agent`][]. See
   * `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.
 
diff --git a/lib/_http_agent.js b/lib/_http_agent.js
index 12f8529c38097c..e1cfa6d7fc9d17 100644
--- a/lib/_http_agent.js
+++ b/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 = '';
   return servername;
 }
 
diff --git a/test/parallel/test-https-simple.js b/test/parallel/test-https-simple.js
index b6a7c692ebb5ae..269db1655efcf8 100644
--- a/test/parallel/test-https-simple.js
+++ b/test/parallel/test-https-simple.js
@@ -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')