fix(apple): don't send connlib DNS server IPs that aren't IPs - #9242
Merged
Conversation
jamilbk
requested review from
Copilot and
thomaseizinger
and removed request for
Copilot
May 27, 2025 00:12
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where connlib fails when given IPv6 addresses containing scopes by normalizing the resolver IP addresses using Swift-native types.
- Normalize IP addresses by converting them to IPv4Address or IPv6Address and using their string representations
- Log a warning if an address fails to parse as either IPv4 or IPv6
Comment on lines
+456
to
+463
| return parsedResolvers.append("\(ipv4Address)") | ||
| } | ||
|
|
||
| if let ipv6Address = IPv6Address(stringAddress) { | ||
| return parsedResolvers.append("\(ipv6Address)") | ||
| } | ||
|
|
||
| Log.warning("IP address \(stringAddress) did not parse as either IPv4 or IPv6") |
There was a problem hiding this comment.
Consider using an if/else-if chain instead of separate if statements to clarify that only one address type should be added and to avoid needless condition checks.
Suggested change
| return parsedResolvers.append("\(ipv4Address)") | |
| } | |
| if let ipv6Address = IPv6Address(stringAddress) { | |
| return parsedResolvers.append("\(ipv6Address)") | |
| } | |
| Log.warning("IP address \(stringAddress) did not parse as either IPv4 or IPv6") | |
| parsedResolvers.append("\(ipv4Address)") | |
| } else if let ipv6Address = IPv6Address(stringAddress) { | |
| parsedResolvers.append("\(ipv6Address)") | |
| } else { | |
| Log.warning("IP address \(stringAddress) did not parse as either IPv4 or IPv6") | |
| } |
Member
Author
|
Tested on iOS and macOS with IPv4 and IPv6 resolvers with manual scopes applied. |
| var parsedResolvers: [String] = [] | ||
|
|
||
| // Normalize addresses to remove any possible scope suffixes | ||
| resolvers.forEach { stringAddress in |
Member
There was a problem hiding this comment.
Is there no iterator-like way of doing this? Like filter_map and then collect? Would avoid the mutability.
thomaseizinger
approved these changes
May 27, 2025
thomaseizinger
changed the title
jamilbk
added a commit
that referenced
this pull request
Jun 1, 2025
A regression was introduced in #9242 where it appears that some Sentry events don't contain messages, so the filtering module is updated only to act on events with messages.
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.
When pulling IPs from system resolvers, it's possible the IPv6 addresses may contain scopes which will cause connlib to barf when parsing.
To fix these, we first convert to the Swift-native type
IPv4AddressorIPv6Addressand then use the string representation of those types, which normalizes them to plain addresses.Fixes #9055