-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix VPN IPv6 connectivity #598
Conversation
"\(host):\(port)" | ||
switch host { | ||
case .name(let hostname, _): | ||
return "\(hostname):\(port)" | ||
case .ipv4(let address): | ||
return "\(address):\(port)" | ||
case .ipv6(let address): | ||
return "[\(address)]:\(port)" | ||
@unknown default: | ||
fatalError() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resetting this back to the old value for now, given that IPv6 needs to use the correct formatting when the port is present.
extension NWEndpoint.Host: CustomStringConvertible { | ||
public var description: String { | ||
extension NWEndpoint.Host { | ||
public var hostWithoutPort: String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this just to prove to myself that no other part of the codebase was using this implicitly, and relying on the behaviour. Now it's very clear that you're getting the host without the port.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks again for spotting and fixing 👍
* main: (39 commits) Fix privacy config fetch in debug mode (#606) Expose Internal User managing from Config (#610) Add Sync feature flags (#607) Fix Networking import into TestUtils (#609) Add Sync Success Rate pixel (#605) Add new logger (#604) Prevent VPN server list persistence failures (#603) SwiftLint plugin (#393) Update autofill to 10.0.2 (#599) Remove the reconnect/disconnect logic from the connection tester Fix an IPv6 regression. (#598) Quality metrics for Sync (#597) Report NetP connection attempts, tunnel failures, and latency (#584) Implement deleteAccount Sync endpoint (#596) Ensure that LinkPresentation framework is called on main thread (#595) No longer excluding the 10.0.0.0/8 range (#594) Update autofill to 10.0.1 (#591) Implement deleteAccount Sync endpoint (#596) Ensure that LinkPresentation framework is called on main thread (#595) No longer excluding the 10.0.0.0/8 range (#594) ...
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/0/1206136376883995/f
iOS PR: duckduckgo/iOS#2258
macOS PR: duckduckgo/macos-browser#1954
What kind of version bump will this require?: Major
Description:
This PR fixes a regression related to IPv6.
Steps to test this PR:
OS Testing:
Internal references:
Software Engineering Expectations
Technical Design Template