fix(ios,tvos): Remove IPv4-only paths to prevent App Store warnings

[greenantdotcom]

Overview

For #300…

Refactored ios/RNCNetInfo.m from inet_ntoa (which causes warnings around Apple AppStore requirements for dual-stack networking code) to inet_ntop which itself supports both IPv4 and IPv6.

Options Considered

Approach 1: Don't call inet_ntoa which is IPv4-only (bare minimum)

Complexity: Low
Status: This PR

This invocation causes warnings related to Apple's AppStore requirements…

Starting June 1, 2016, all apps submitted to the App Store must support IPv6-only networking.

Ref: https://developer.apple.com/support/ipv6/_

Refactoring from inet_ntoa (IPv4-only) with inet_ntop (which supports both) in RNCNetInfo's ipAddress and subnet methods gets rid of a symbol match on inet_ntoa, and is a simple refactor to get the same value via the new function.

Ex:

char str[INET_ADDRSTRLEN];
inet_ntop(AF_INET, &((struct sockaddr_in *)temp_addr->ifa_addr)->sin_addr, str, INET_ADDRSTRLEN);
address = [NSString stringWithUTF8String:str];

What risks could come of adding? None foreseen - net net, this is replacing the same "what" with a different "how" but should return the same values.
What risks could come of not adding? Continued findings for adopting teams that use scanning tools to find issues such as AppStore guidline gaps, and teams will continue to see this warning.

Approach 2: inet_ntop is good, but not sufficient per Apple

Complexity: Low
Status: Can be included, but thinking best in a separate PR/issue

Apple isn't really after "better IPv4" code, and recommends being truly dual-stack-ready:

IMPORTANT: We encourage you to adopt address-family agnostic APIs rather than to maintain separate code paths for IPv4 and IPv6.

Ref: https://developer.apple.com/support/ipv6/

RNCNetInfo's ipAddress and subnet methods loop through all interfaces, but ignore any that aren't AF_INET (IPv4) skipping all other values. By not adding AF_INET6 support, the current implementation doesn't support IPv6-only.

The fix for this isn't that much more difficult and can be easily slotted in to the two methods…

if(interface_sa_family == AF_INET) {
    char str[INET_ADDRSTRLEN];
    inet_ntop(AF_INET, &((struct sockaddr_in *)temp_addr->ifa_netmask)->sin_addr, str, INET_ADDRSTRLEN);
    subnet = [NSString stringWithUTF8String:str];
}
else
if(interface_sa_family == AF_INET6) {
    char str[INET6_ADDRSTRLEN];
    inet_ntop(AF_INET6, &((struct sockaddr_in6 *)temp_addr->ifa_netmask)->sin6_addr, str, INET6_ADDRSTRLEN);
    subnet = [NSString stringWithUTF8String:str];
}

What risks could come of not adding?

Devices running on an IPv6-only networks would only ever return 0.0.0.0 as there would be no AF_INET record to extact another IP address or subnet from. So in some cases, the value being returned is not accurate for the user's situation, which in turn could complicate tasks such as debugging network issues related to connectivity.

What risks could come of adding?

I admittedly don't know enough to accurately predict global ramifications to consuming code if these methods return non-IPv4 records, but this was my starter list of possible issues…

1. INET6_ADDRSTRLEN is 46 bytes, which is larger than INET_ADDRSTRLEN (16 bytes) - anyone with a strict storage schema for, say, persisting the data where code is expecting only 16 bytes would either get errors or unexpected data truncation. (feels unlikely)
2. It is possible that downstream client-code relies on the value directly for some kind of logic that would break if they aren't expecting IPv6 addresses (which are larger and with different/more complex formatting). (feels unlikely)
3. Downstream data issues related to passing client IP as an arg to a downstream call - if those systems don't expect IPv6 addresses, then they will break which may cause consuming app breakage. (unsure)

Ref:

• https://github.com/react-native-netinfo/react-native-netinfo/blob/master/android/src/main/java/com/reactnativecommunity/netinfo/ConnectivityReceiver.java#L211 - calls

Approach 3: Add more clarity to the selection of the proper IP addresses/subnet

Complexity: Low-Medium
Status: Should be separate PR, waiting on input

Building on Approach 2's proposal, let's consider what would happen if we do add IPv6 support.

Further note that in the current algorithm…

• Only en0 and en1 are considered for iOS/tvOS/(watchOS?)
• Upon finding a result, the loop continues through all interfaces after an initial value is found (i.e. there is no break after this https://github.com/react-native-netinfo/react-native-netinfo/blob/master/ios/RNCNetInfo.m#L167)

Example 1

Let's take first a mixed config approach:

Interface	IPv4 Address	IPv6 Address
en0	-none-	en0.ipv6
en1	en1.ipv4	-none-

• Current algorithm:: en1.ipv4
• W/ Changes in chore: Remove deprecated code #2 above: en1.ipv4

Example 2

Now, let's flip that table:

Interface	IPv4 Address	IPv6 Address
en0	en0.ipv4	-none-
en1	-none-	en1.ipv6

• Current algorithm:: en0.ipv4
• W/ Changes in chore: Remove deprecated code #2 above: en1.ipv6

Because there is no break, the last value wins, and in this case, the IPv6 record would win

Example 3

Now, let's say we have a full config like so:

Interface	IPv4 Address	IPv6 Address
en0	en0.ipv4	en0.ipv6
en1	en1.ipv4	en1.ipv6

• Current algorithm:: en1.ipv4
• W/ Changes in chore: Remove deprecated code #2 above: en1.ipv4

Example 4

Now, let's say it's IPv6 only

Interface	IPv4 Address	IPv6 Address
en0	-none-	en0.ipv6
en1	-none-	en1.ipv6

• Current algorithm:: 0.0.0.0 (default, no IPv4 found)
• W/ Changes in chore: Remove deprecated code #2 above: en1.ipv6

–––

Unpacking the above, I think this produces an ambiguous result and it's not clear what the user actually is looking for.

• In example 1, you get the first available IPv4 address, but you don't get the first seen IP Address (which is en0.ipv6)
• In example 2, you get en1.ipv6 even though an address (en0.ipv4) was seen previously.
• In example 3, you get en1.ipv4 even though en0 has both IPv4 and IPv6 records and there's also the matter of en1.ipv6
• In example 4, you get en1.ipv6 even though en0 had a value as well

In the first three of these cases, a value that is one of the addresses is returned, but it's not clear to me if it eliminates the principle of least surprise for the caller.

Proposals

To remove the ambiguity, I think there could be one of a few algorithms, with two provided here:

1. Prefer first interface with an IPv4/6 address, and prefer IPv4 over IPv6 (this is what chore: Remove deprecated code #2 effectively is with a break added in)

   Example	New Answer	Rationale
   Ex 1	en0.ipv6	First interface w/ addr, IPv6 because there is no IPv4 for that interface
   Ex 2	en0.ipv4	First interface w/ addr
   Ex 3	en0.ipv4	First interface w/ addr, IPv4 as we prefer it over IPv6
   Ex 4	en0.ipv6	First interface w/ addr

2. Prefer first IPv4 address regardless of interface, then IPv6 if no IPv4

   Example	New Answer	Rationale
   Ex 1	en1.ipv4	Second interface w/ addr, but first IPv4 record seen
   Ex 2	en0.ipv4	First interface w/ addr
   Ex 3	en0.ipv4	First interface w/ addr, IPv4 as we prefer it over IPv6
   Ex 4	en0.ipv6	No IPv4 available, first interface w/ addr

Add'l note

Trying to assess things holistically, the ssid and bssid methods take a wholly different codepaths to grab their values, and I would suggest that they should also be aligned with the above algo and aligned to the same interface chosen for the IP/subnet values.

That is, if the chosen IP/subnet interface is an Ethernet interface, logically, there is no (B)SSID.

As there have been notes about more full macOS support, which has so many other potential interfaces (i.e. USB Ethernet adapters, FireWire/Thunderbolt hubs w/ Ethernet, etc.) and let's you order the interfaces yourself in System Preferences, I thought it was at least worth the note that disambiguation may be preferable.

Summary

I'm honestly not sure what the right algo is. But if Option 2 were implemented, I think it's worth the discussion.

Test Plan

Attn: @matt-oakes

• @tom-un - visibility as contributor of pr/312 for GitHub actions support

I have not yet been able to validate what I think is a fairly small change as I cannot reproduce the testing path described in #312 and the repo instructions as the repo doesn't build cleanly for me, and I would ask for some guidance.

Where did I test?

Locally

• macOS 10.15.7
• Xcode 12.2

GitHub Actions

In my fork, I enabled Actions, and it failed the same way, and seems to be…

• macOS 10.15.7
• Xcode 12.0.1

Per https://github.com/actions/virtual-environments/blob/main/images/macos/macos-10.15-Readme.md#xcode)

Note: As of 30NOV2020, the default XCode version will jump to XCode 12.2 (actions/runner-images#2056)

–––

There are some related issues that had to do with Xcode 12 that I see in the react-native-macos project

• Xcode12 beta4 compiler fixes microsoft/react-native-macos#534 - Xcode12 beta4 compiler fixes
• Unblock Xcode 12 beta 4 microsoft/react-native-macos#535 - Unblock Xcode 12 beta 4

In experimenting, I tried updating the react-native-macos version to a version that fixes #534 above…

-    "react-native-macos": "0.60.0-microsoft.50",
+    "react-native-macos": "^0.61.65",

…which seems the earliest version that has fixes for both of the above PRs. While that fixed the issue related to missing symbols, errors related to building detox and some libraries not found persisted.

I am uncertain how to proceed as the patch here may fix the purported issue (it does in a separate command line Xcode project where I copied over the data), I can't give y'all affirmative signal - and I suspect that there is some deeper work needed to fully support Xcode 12.

Any advice would be appreciated.

[matt-oakes]

Thanks for sending this in and sorry it's taken so long to look at.

I think this approach is good for a first pass to get the warnings dealt with. Anyone who's looking to help should feel free to open a follow up PR with the 2nd approach outlined which adds proper support for ipv6.

Thanks again!

[react-native-community-bot]

🎉 This PR is included in version 5.9.10 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀