Repository navigation
fix(dns): preserve servername on dual-stack fallback - #6016
Open
arshsmith1 wants to merge 1 commit into
Open
arshsmith1 wants to merge 1 commit into
arshsmith1 wants to merge 1 commit into
Conversation
The initial dispatch sets servername so TLS verifies the certificate against the request hostname, but the other-family fallback in onResponseError rebuilds the dispatch options from this.#opts, which never carried servername. The retried HTTPS request then has no servername and an IP-literal origin, so the connector verifies the certificate against the IP instead of the hostname. Set servername to the original hostname on the fallback dispatch, matching the initial one. Signed-off-by: arshiya tabasum <arshi@bugqore.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6016 +/- ##
=======================================
Coverage 91.57% 91.57%
=======================================
Files 113 113
Lines 43679 43680 +1
=======================================
+ Hits 39999 40000 +1
Misses 3680 3680 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
metcoder95
approved these changes
Oct 15, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This relates to...
The
dnsinterceptor's dual-stack fallback path.Rationale
When the
dnsinterceptor resolves a hostname it dispatches to the IP literal but setsservernameto the original hostname so TLS checks the certificate against the hostname, not the IP. The dual-stack fallback inDNSDispatchHandler.onResponseError(the ETIMEDOUT/ECONNREFUSED retry to the other address family) rebuilds the dispatch options fromthis.#opts, a copy of the original options that never carriedservername(it was only added to the separate first-try object). The retried HTTPS request then has noservernameand an IP-literal origin, sobuildConnectorfalls back togetServerName(<ip>), which returns'', and Node verifies the certificate against the IP literal instead of the hostname. The initial attempt and the fallback end up pinned to different TLS identities for the same logical origin.Changes
Set
servernameto the original hostname on the fallback dispatch, mirroring the initial dispatch, and add a regression test asserting both attempts carry the hostnameservername.Features
N/A
Bug Fixes
The
dnsinterceptor keeps the hostnameservernameon the dual-stack fallback, so certificate verification stays pinned to the hostname instead of the resolved IP literal.Breaking Changes and Deprecations
N/A
Status