Repository navigation
Fix DNS lookups for IPv6 addresses. - #8053
Merged
Merged
Conversation
jellefoks
requested review from
andrewsavage1,
johnxwork,
y4vor and
yell0wd0g
November 14, 2025 21:17
andrewsavage1
left a comment
andrewsavage1
left a comment
Contributor
There was a problem hiding this comment.
This LGTM, but I'll ensure others get a chance to take a look. I assume we'll need to bring this into main/26.eap as well?
Contributor
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request fixes an issue with DNS lookups for IPv6 addresses by correctly copying sockaddr_in6 structures in the getaddrinfo wrapper. The changes also include extensive new tests to verify DNS resolution against known IP addresses and make other tests more robust by skipping them on environments where they cannot run.
My review has identified a critical issue in the new test logic that could lead to crashes, as well as a high-severity issue where the core fix could be made more robust. I've also included a medium-severity suggestion to improve test code maintainability by reducing duplication. Please see the detailed comments below.
kaidokert
approved these changes
Nov 14, 2025
kaidokert
left a comment
kaidokert
left a comment
Contributor
There was a problem hiding this comment.
Great find, and looks like a good fix to me.
y4vor
approved these changes
Nov 14, 2025
This comment was marked as resolved.
This comment was marked as resolved.
jellefoks
enabled auto-merge (squash)
November 15, 2025 01:01
jellefoks
force-pushed
the
dns6_lookup_fix
branch
2 times, most recently
from
November 15, 2025 05:57
be39676 to
53039b5
Compare
yell0wd0g
approved these changes
Nov 15, 2025
yell0wd0g
left a comment
yell0wd0g
left a comment
Contributor
There was a problem hiding this comment.
This LGTM, although I'm not sure my +1 is enough for this branch.
Particularly love the added tests!
jellefoks
force-pushed
the
dns6_lookup_fix
branch
4 times, most recently
from
November 16, 2025 22:31
970514d to
f26a78b
Compare
Update the hashes in the test to match the new certificates.
jellefoks
force-pushed
the
dns6_lookup_fix
branch
from
November 17, 2025 00:23
f26a78b to
200c1c6
Compare
jellefoks
added a commit
that referenced
this pull request
Dec 11, 2025
The Starboard layer wrapper for getaddrinfo() made incorrect copies of the address returned for IPv6 addresses. This fixes that and adds extensive testing using a host with known IP addresses. The fix is adopted from #7588 with a simplification and additional tests. This PR also includes an update to test certs for net_unittests that expired Oct 29 2025. Bug: 460175900 Bug: 438858995 Bug: 409716671 Bug: 460594994 Bug: 439002238 Bug: 405988446 --------- Co-authored-by: jimmy.ge <[email protected]>
jellefoks
added a commit
to jellefoks/cobalt
that referenced
this pull request
Dec 11, 2025
The Starboard layer wrapper for getaddrinfo() made incorrect copies of the address returned for IPv6 addresses. This fixes that and adds extensive testing using a host with known IP addresses. The fix is adopted from youtube#7588 with a simplification and additional tests. This PR also includes an update to test certs for net_unittests that expired Oct 29 2025. Bug: 460175900 Bug: 438858995 Bug: 409716671 Bug: 460594994 Bug: 439002238 Bug: 405988446 --------- Co-authored-by: jimmy.ge <[email protected]>
jellefoks
added a commit
to jellefoks/cobalt
that referenced
this pull request
Dec 15, 2025
The Starboard layer wrapper for getaddrinfo() made incorrect copies of the address returned for IPv6 addresses. This fixes that and adds extensive testing using a host with known IP addresses. The fix is adopted from youtube#7588 with a simplification and additional tests. This PR also includes an update to test certs for net_unittests that expired Oct 29 2025. Bug: 460175900 Bug: 438858995 Bug: 409716671 Bug: 460594994 Bug: 439002238 Bug: 405988446 --------- Co-authored-by: jimmy.ge <[email protected]>
jellefoks
added a commit
that referenced
this pull request
Dec 16, 2025
…8367) Refer to the original PR: #8053 The Starboard layer wrapper for getaddrinfo() made incorrect copies of the sockaddr structure for IPv6 addresses. This issue prevented proper resolution and use of IPv6 addresses for DNS lookups. This change modifies the wrapper to ensure that the entire sockaddr structure, including the address data, is correctly copied, thus resolving the IPv6 address lookup failures. Additionally, extensive tests have been added and updated to thoroughly verify both IPv4 and IPv6 address resolution for known external hosts and localhost. Bug: 460175900 Bug: 438858995 Bug: 409716671 Bug: 460594994 Bug: 439002238 Bug: 405988446 Co-authored-by: jimmy.ge <[email protected]>
cobalt-github-releaser-bot
pushed a commit
that referenced
this pull request
Dec 16, 2025
…8367) Refer to the original PR: #8053 The Starboard layer wrapper for getaddrinfo() made incorrect copies of the sockaddr structure for IPv6 addresses. This issue prevented proper resolution and use of IPv6 addresses for DNS lookups. This change modifies the wrapper to ensure that the entire sockaddr structure, including the address data, is correctly copied, thus resolving the IPv6 address lookup failures. Additionally, extensive tests have been added and updated to thoroughly verify both IPv4 and IPv6 address resolution for known external hosts and localhost. Bug: 460175900 Bug: 438858995 Bug: 409716671 Bug: 460594994 Bug: 439002238 Bug: 405988446 Co-authored-by: jimmy.ge <[email protected]> (cherry picked from commit 2280057)
jellefoks
added a commit
that referenced
this pull request
Dec 16, 2025
…s for IPv6 addresses. (#8428) Refer to the original PR: #8367 Refer to the original PR: #8053 The Starboard layer wrapper for getaddrinfo() made incorrect copies of the sockaddr structure for IPv6 addresses. This issue prevented proper resolution and use of IPv6 addresses for DNS lookups. This change modifies the wrapper to ensure that the entire sockaddr structure, including the address data, is correctly copied, thus resolving the IPv6 address lookup failures. Additionally, extensive tests have been added and updated to thoroughly verify both IPv4 and IPv6 address resolution for known external hosts and localhost. Bug: 460175900 Bug: 438858995 Bug: 409716671 Bug: 460594994 Bug: 439002238 Bug: 405988446 Co-authored-by: Jelle Foks <[email protected]> Co-authored-by: jimmy.ge <[email protected]>
msolya
pushed a commit
to msolya/cobalt
that referenced
this pull request
Mar 6, 2026
The Starboard layer wrapper for getaddrinfo() made incorrect copies of the address returned for IPv6 addresses. This fixes that and adds extensive testing using a host with known IP addresses. The fix is adopted from youtube#7588 with a simplification and additional tests. This PR also includes an update to test certs for net_unittests that expired Oct 29 2025. Bug: 460175900 Bug: 438858995 Bug: 409716671 Bug: 460594994 Bug: 439002238 Bug: 405988446 --------- Co-authored-by: jimmy.ge <[email protected]>
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.
The Starboard layer wrapper for getaddrinfo() made incorrect copies of the address returned for IPv6 addresses.
This fixes that and adds extensive testing using a host with known IP addresses.
The fix is adopted from #7588 with a simplification and additional tests.
This PR also includes an update to test certs for net_unittests that expired Oct 29 2025.
Bug: 460175900
Bug: 438858995
Bug: 409716671
Bug: 460594994
Bug: 439002238
Bug: 405988446