add client-side proxy detection#525
Open
nickpatrick wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds client-side proxy detection capability to the RadarVerificationManager. The implementation checks for both system-configured proxy settings and VPN/proxy-related network interfaces to detect if the device is routing traffic through a proxy.
Changes:
- Added
isUsingProxymethod that detects proxy usage through system proxy settings (HTTP/HTTPS/SOCKS) and network interface inspection (utun, ipsec, ppp)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| NSString *name = [NSString stringWithUTF8String:cur->ifa_name]; | ||
| if ([name hasPrefix:@"utun"] || | ||
| [name hasPrefix:@"ipsec"] || | ||
| [name hasPrefix:@"ppp"]) { |
There was a problem hiding this comment.
Memory leak: When a VPN/proxy interface is detected and the method returns YES here, the ifaddr structure allocated by getifaddrs on line 719 is not freed. This will leak memory each time a proxy is detected via network interfaces. The freeifaddrs call on line 729 will not be reached. Add a freeifaddrs(ifaddr) call before returning YES.
Suggested change
| [name hasPrefix:@"ppp"]) { | |
| [name hasPrefix:@"ppp"]) { | |
| freeifaddrs(ifaddr); |
| } | ||
|
|
||
| - (BOOL)isUsingProxy { | ||
| CFDictionaryRef proxyRef = CFNetworkCopySystemProxySettings(); |
There was a problem hiding this comment.
Missing import: The CFNetworkCopySystemProxySettings function is being used but there is no explicit import for CFNetwork framework. While this may compile due to implicit inclusion, it's best practice to explicitly import CFNetwork at the top of the file with other framework imports.
| @@ -701,4 +701,35 @@ - (NSString *)kDeviceId { | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Missing documentation: This new method lacks a comment or documentation explaining what it does, what scenarios it detects (system proxy settings, VPN interfaces like utun/ipsec/ppp), and what the return value represents. Consider adding a brief comment above the method explaining its purpose and behavior.
Suggested change
| /// Returns YES if the device appears to be using a network proxy or VPN. | |
| /// Detection is based on: | |
| /// - System proxy settings from CFNetwork (HTTP/HTTPS/SOCKS). | |
| /// - Presence of network interfaces commonly used by VPNs (e.g., utun, ipsec, ppp). | |
| /// Returns NO if no such proxy or VPN-related configuration is detected. |
stevepopovich
added a commit
that referenced
this pull request
Jun 1, 2026
These weren't part of the Android PR — Android's RadarApiHelper already logged both paths before #525. Pulling them out keeps this PR a tight 1:1 port; the iOS-specific logging gap can land in its own follow-up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
stevepopovich
added a commit
that referenced
this pull request
Jun 1, 2026
These weren't part of the Android PR — Android's RadarApiHelper already logged both paths before #525. Pulling them out keeps this PR a tight 1:1 port; the iOS-specific logging gap can land in its own follow-up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
stevepopovich
added a commit
that referenced
this pull request
Jun 1, 2026
These weren't part of the Android PR — Android's RadarApiHelper already logged both paths before #525. Pulling them out keeps this PR a tight 1:1 port; the iOS-specific logging gap can land in its own follow-up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
stevepopovich
added a commit
that referenced
this pull request
Jun 1, 2026
* Make RadarLoggerTests deterministic to fix CI flakiness RadarLoggerTests asserted that async, MainActor-delivered didLog callbacks arrived within a 5s waitUntil poll. RadarLogger.log() spawns a detached Task that hops through the RadarLogBuffer actor and the MainActor before calling the delegate, so the assertion was really a race against the cooperative thread pool. On the slower GitHub Actions macos-15 runner that pool is saturated under parallel test load, the callbacks landed after the 5s budget, and the tests failed with delegate.messages.count == 0. Replace the wall-clock race with deterministic awaiting: - RadarLogger now retains its in-flight log Tasks while in test mode (logLevelOverride != nil) and exposes awaitPendingLogs(), which awaits them so the delegate callbacks are guaranteed to have run. Production is unaffected: logLevelOverride is nil, so no tasks are retained and the logging path is unchanged. All entry points (debug/info/warning/error and the @objc log overloads) funnel through the core log(), so the hook covers them uniformly, including objectiveCInterfaceWorks. - RadarLoggerTests now await awaitPendingLogs() instead of polling + sleeping. The tests are now timing-independent: they pass in ~0.02s in isolation and ~0.19s alongside the 159-test ObjC suite, with no dependence on runner speed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * FENCE-2857: Add NSError parameter to ipGeocode callback Ports radarlabs/radar-sdk-android#525 to iOS. Exposes the underlying NSError on ipGeocode failures so integrators can forward network, parse, and exception errors to error collectors (Sentry, Crashlytics, Datadog). Adds new +ipGeocodeWithErrorCompletionHandler: with a 4-arg RadarIPGeocodeWithErrorCompletionHandler typedef. Deprecates the legacy 3-arg +ipGeocodeWithCompletionHandler:, which now routes through the new method so existing integrators keep compiling. Swift sees only the new 4-arg block under Radar.ipGeocode { ... } (via NS_SWIFT_NAME). Threads NSError through the internal stack: RadarAPIHelper's URLSession, NSJSONSerialization, and @catch NSException paths now pass the underlying error to the (now 3-arg) RadarAPICompletionHandler. RadarAPIClient's internal RadarIPGeocodeAPICompletionHandler gains the NSError too. Other internal callsites take the new parameter and pass it through unused, matching the Android PR's pilot scope. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Revert logging additions in JSON parse and NSException paths These weren't part of the Android PR — Android's RadarApiHelper already logged both paths before #525. Pulling them out keeps this PR a tight 1:1 port; the iOS-specific logging gap can land in its own follow-up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Use nil-coalescing instead of String(describing:) for optional error description Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.
No description provided.