http2: omit server name when HTTP2 host is IP address #56530
Conversation
|
Review requested:
|
nodejs-github-bot
added
http2
| let hostName = socket.servername; | ||
| if (hostName === null || hostName === false) { | ||
| if (socket.remoteFamily === 'IPv6') { | ||
| hostName = `[${socket.remoteAddress}]`; | ||
| } else { | ||
| hostName = socket.remoteAddress; | ||
| } |
There was a problem hiding this comment.
Host: the value sent in Server Name Indication (SNI) ([RFC6066],
Section 3) converted to lower case; if SNI is not present, the
remote address of the connection (i.e., the server's IP address)
According to RFC 8336, when the server name is not set, the server's address should be used instead.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #56530 +/- ##
==========================================
+ Coverage 88.54% 89.19% +0.65%
==========================================
Files 657 662 +5
Lines 190689 191808 +1119
Branches 36602 36926 +324
==========================================
+ Hits 168844 171089 +2245
+ Misses 15029 13564 -1465
- Partials 6816 7155 +339
|
| }) | ||
| ); | ||
| }, 2)); | ||
| server.on('close', common.mustCall()); |
There was a problem hiding this comment.
| server.on('close', common.mustCall()); |
| let data = ''; | ||
| req.setEncoding('utf8'); | ||
| req.on('data', (d) => data += d); | ||
| req.on('end', common.mustCall(() => { |
There was a problem hiding this comment.
Can't the requests be done in parallel? Then server can be closed with something in the listeners of 'end' events
if (++done === 2) server.close();There was a problem hiding this comment.
I've made some adjustments to process the requests in parallel.
github-actions
bot
removed
the
request-ci
| const ipv4Url = `https://127.0.0.1:${server.address().port}`; | ||
| const ipv6Url = `https://[::1]:${server.address().port}`; | ||
| handleRequest(ipv4Url); | ||
| if (common.hasIPv6) handleRequest(ipv6Url); |
There was a problem hiding this comment.
I added this branch because the EADDRNOTAVAIL error was occurring in the tests.
Co-authored-by: Luigi Pinca <[email protected]>
github-actions
bot
removed
the
request-ci
lpinca
added
the
commit-queue-squash
| if (++done === 2) server.close(); | ||
| if (++done === testCount) server.close(); |
There was a problem hiding this comment.
Sorry, I missed that the number of tests changes if you don't test for IPv6.
github-actions
bot
removed
the
request-ci
Fixes: #56189