HTTP2 client incorrectly sets TLS ServerName to an IP address

[the-ress]

Version

v23.3.0

Platform

Linux d5c04eefb9b4 5.15.167.4-microsoft-standard-WSL2 #1 SMP Tue Nov 5 00:21:55 UTC 2024 x86_64 GNU/Linux

Subsystem

http2

What steps will reproduce the bug?

const http2 = require('http2');
const session = http2.connect('https://1.1.1.1');
session.once('remoteSettings', () => {
    session.close();
});

How often does it reproduce? Is there a required condition?

always

What is the expected behavior? Why is that the expected behavior?

• no deprecation warning about TLS server name should appear because I'm not passing any and it's added in the http2 module
• TLS server name should not be set because it's not permitted by RFC 6066

What do you see instead?

• a deprecation warning:

# node --trace-deprecation repro.js
(node:275) [DEP0123] DeprecationWarning: Setting the TLS ServerName to an IP address is not permitted by RFC 6066. This will be ignored in a future version.
    at Object.connect (node:_tls_wrap:1812:15)
    at Object.connect (node:internal/http2/core:3332:22)
    at Object.<anonymous> (/repro.js:2:23)
    at Module._compile (node:internal/modules/cjs/loader:1546:14)
    at Object..js (node:internal/modules/cjs/loader:1698:10)
    at Module.load (node:internal/modules/cjs/loader:1303:32)
    at Function._load (node:internal/modules/cjs/loader:1117:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:218:24)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:170:5)

• the outgoing request contains 1.1.1.1 as the server_name TLS extension

Additional information

The server name is set in http2/core.js. That function is missing a check for net.isIP that's used in _http_agent.js for example.

However the fix isn't that simple because the servername is then used to construct originSet and just adding the check would probably regress #39919.