Fix IP address bug where 4 byte version wasn't converted to dotted decimal form

Introduces a tested `GNCIPAddress` class to make it more explicit of which IP address format is being used at call sites to prevent similar issues in the future.

PiperOrigin-RevId: 528572993

internal/platform/implementation/apple/BUILD

@@ -65,8 +65,10 @@ objc_library(
         "//third_party/apple_frameworks:Foundation",
         "//third_party/apple_frameworks:Network",
         "//third_party/objective_c/google_toolbox_for_mac:GTM_Logger",
+        "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/strings",
+        "@com_google_absl//absl/synchronization",
         "@com_google_absl//absl/time",
         "@com_google_absl//absl/types:optional",
         "@nlohmann_json//:json",

internal/platform/implementation/apple/Mediums/BUILD

@@ -25,6 +25,7 @@ objc_library(
         "GNCLeaks.h",
         "GNCLeaks.m",
         "GNCMConnection.m",
+        "WiFiLAN/GNCIPv4Address.m",
         "WiFiLAN/GNCWiFiLANError.m",
         "WiFiLAN/GNCWiFiLANMedium.m",
         "WiFiLAN/GNCWiFiLANServerSocket.m",
@@ -37,6 +38,7 @@ objc_library(
         "Ble/GNCMBlePeripheral.h",
         "Ble/GNCMBleUtils.h",
         "GNCMConnection.h",
+        "WiFiLAN/GNCIPv4Address.h",
         "WiFiLAN/GNCWiFiLANError.h",
         "WiFiLAN/GNCWiFiLANMedium.h",
         "WiFiLAN/GNCWiFiLANServerSocket.h",

internal/platform/implementation/apple/Mediums/WiFiLAN/GNCIPv4Address.h

@@ -0,0 +1,60 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#import <Foundation/Foundation.h>
+
+/** A container for IPv4 address information. */
+@interface GNCIPv4Address : NSObject
+
+/**
+ * @remark init is not an available initializer.
+ */
+- (nonnull instancetype)init NS_UNAVAILABLE;
+
+/**
+ * Creates a container for IPv4 address information.
+ *
+ * @param byte1 The first byte of the IP address.
+ * @param byte2 The second byte of the IP address.
+ * @param byte3 The third byte of the IP address.
+ * @param byte4 The forth byte of the IP address.
+ */
+- (nonnull instancetype)initWithByte1:(uint8_t)byte1
+                                byte2:(uint8_t)byte2
+                                byte3:(uint8_t)byte3
+                                byte4:(uint8_t)byte4 NS_DESIGNATED_INITIALIZER;
+
++ (nonnull instancetype)addressFromFourByteInt:(uint32_t)address;
+
++ (nonnull instancetype)addressFromData:(nonnull NSData *)address;
+
+/** The first byte of the IP address. */
+@property(nonatomic, readonly) uint8_t byte1;
+
+/** The second byte of the IP address. */
+@property(nonatomic, readonly) uint8_t byte2;
+
+/** The third byte of the IP address. */
+@property(nonatomic, readonly) uint8_t byte3;
+
+/** The forth byte of the IP address. */
+@property(nonatomic, readonly) uint8_t byte4;
+
+/** The 4 byte binary representation for the IPv4 address. */
+@property(nonatomic, nonnull, readonly) NSData *binaryRepresentation;
+
+/** The human readable dotted representation for the IPv4 address. */
+@property(nonatomic, nonnull, readonly) NSString *dottedRepresentation;
+
+@end

internal/platform/implementation/apple/Mediums/WiFiLAN/GNCIPv4Address.m

@@ -0,0 +1,59 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCIPv4Address.h"
+
+@implementation GNCIPv4Address
+
+- (instancetype)initWithByte1:(uint8_t)byte1
+                        byte2:(uint8_t)byte2
+                        byte3:(uint8_t)byte3
+                        byte4:(uint8_t)byte4 {
+  self = [super init];
+  if (self) {
+    _byte1 = byte1;
+    _byte2 = byte2;
+    _byte3 = byte3;
+    _byte4 = byte4;
+  }
+  return self;
+}
+
++ (instancetype)addressFromFourByteInt:(uint32_t)address {
+  uint8_t byte1 = (address >> (8 * 0)) & 0xff;
+  uint8_t byte2 = (address >> (8 * 1)) & 0xff;
+  uint8_t byte3 = (address >> (8 * 2)) & 0xff;
+  uint8_t byte4 = (address >> (8 * 3)) & 0xff;
+  return [[GNCIPv4Address alloc] initWithByte1:byte1 byte2:byte2 byte3:byte3 byte4:byte4];
+}
+
++ (instancetype)addressFromData:(NSData *)address {
+  NSAssert(address.length == 4, @"Address must be 4 bytes");
+  const uint8_t *bytes = address.bytes;
+  return [[GNCIPv4Address alloc] initWithByte1:bytes[0]
+                                         byte2:bytes[1]
+                                         byte3:bytes[2]
+                                         byte4:bytes[3]];
+}
+
+- (NSData *)binaryRepresentation {
+  const uint8_t bytes[] = {_byte1, _byte2, _byte3, _byte4};
+  return [NSData dataWithBytes:bytes length:4];
+}
+
+- (NSString *)dottedRepresentation {
+  return [NSString stringWithFormat:@"%d.%d.%d.%d", _byte1, _byte2, _byte3, _byte4];
+}
+
+@end

internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANMedium.h

@@ -14,6 +14,7 @@
 
 #import <Foundation/Foundation.h>
 
+@class GNCIPv4Address;
 @class GNCWiFiLANServerSocket;
 @class GNCWiFiLANSocket;
 
@@ -102,12 +103,12 @@ typedef void (^ServiceUpdateHandler)(NSString *_Nonnull serviceName,
 /**
  * Connects to an IP address and port.
  *
- * @param host The 4 byte binary representation IPv4 address to connect to.
+ * @param host The IPv4 address to connect to.
  * @param port The port to connect to.
  * @param[out] error Error that will be populated on failure.
  * @return Returns a connected socket or nil if an error has occured.
  */
-- (nullable GNCWiFiLANSocket *)connectToHost:(nonnull NSString *)host
+- (nullable GNCWiFiLANSocket *)connectToHost:(nonnull GNCIPv4Address *)host
                                         port:(NSInteger)port
                                        error:(NSError **_Nullable)error;
 

internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANMedium.m

@@ -17,6 +17,7 @@
 #import <Foundation/Foundation.h>
 #import <Network/Network.h>
 
+#import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCIPv4Address.h"
 #import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANError.h"
 #import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANServerSocket+Internal.h"
 #import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANServerSocket.h"
@@ -258,9 +259,11 @@ NSDictionary<NSString *, NSString *> *GNCTXTRecordForBrowseResult(nw_browse_resu
   return [self connectToEndpoint:endpoint error:error];
 }
 
-- (GNCWiFiLANSocket *)connectToHost:(NSString *)host port:(NSInteger)port error:(NSError **)error {
+- (GNCWiFiLANSocket *)connectToHost:(GNCIPv4Address *)host
+                               port:(NSInteger)port
+                              error:(NSError **)error {
   nw_endpoint_t endpoint =
-      nw_endpoint_create_host([host UTF8String], [[@(port) stringValue] UTF8String]);
+      nw_endpoint_create_host(host.dottedRepresentation.UTF8String, @(port).stringValue.UTF8String);
   return [self connectToEndpoint:endpoint error:error];
 }
 

internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANServerSocket.h

@@ -14,6 +14,7 @@
 
 #import <Foundation/Foundation.h>
 
+@class GNCIPv4Address;
 @class GNCWiFiLANSocket;
 
 @interface GNCWiFiLANServerSocket : NSObject
@@ -31,9 +32,9 @@
 - (nonnull instancetype)initWithPort:(NSInteger)port NS_DESIGNATED_INITIALIZER;
 
 /**
- * The 4 byte binary representation for the IPv4 address of the physical network interface.
+ * The IPv4 address of the physical network interface.
  */
-@property(nonatomic, readonly, copy) NSString *ipAddress;
+@property(nonatomic, readonly, copy) GNCIPv4Address *ipAddress;
 
 /**
  * The port of the server socket.

internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANServerSocket.m

@@ -22,6 +22,7 @@
 #include <netdb.h>
 #include <sys/socket.h>
 
+#import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCIPv4Address.h"
 #import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANError.h"
 #import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANServerSocket+Internal.h"
 #import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANSocket.h"
@@ -71,7 +72,7 @@
 
 @synthesize ipAddress = _ipAddress;
 
-- (NSString *)ipAddress {
+- (GNCIPv4Address *)ipAddress {
   if (!_ipAddress) {
     _ipAddress = [GNCWiFiLANServerSocket lookupIpAddress];
   }
@@ -235,7 +236,7 @@
  * Returns the IP address as a 4 byte string. If not available, this returns an empty string to
  * align with the Windows implementation.
  */
-+ (NSString *)lookupIpAddress {
++ (GNCIPv4Address *)lookupIpAddress {
   struct ifaddrs *ifaddr;
 
   // Note: The data returned by `getifaddrs()` is dynamically allocated and should be freed using
@@ -243,7 +244,7 @@
   //
   // See: https://linux.die.net/man/3/getifaddrs
   if (getifaddrs(&ifaddr) == -1) {
-    return @"";
+    return [GNCIPv4Address addressFromFourByteInt:0];
   }
 
   // Walk through linked list, maintaining head pointer so we can free list later.
@@ -264,22 +265,13 @@
       continue;
     }
 
-    // Break the 4 byte binary representation of the hostname into 4 separate bytes.
     uint32_t host = ((struct sockaddr_in *)address)->sin_addr.s_addr;
-    uint8_t byte1 = (host >> (8 * 0)) & 0xff;
-    uint8_t byte2 = (host >> (8 * 1)) & 0xff;
-    uint8_t byte3 = (host >> (8 * 2)) & 0xff;
-    uint8_t byte4 = (host >> (8 * 3)) & 0xff;
-
-    // Join the bytes into a 4 character string.
-    NSString *hostString = [NSString stringWithFormat:@"%c%c%c%c", byte1, byte2, byte3, byte4];
-
     freeifaddrs(ifaddr);
-    return hostString;
+    return [GNCIPv4Address addressFromFourByteInt:host];
   }
 
   freeifaddrs(ifaddr);
-  return @"";
+  return [GNCIPv4Address addressFromFourByteInt:0];
 }
 
 @end

internal/platform/implementation/apple/Tests/BUILD

@@ -26,6 +26,7 @@ objc_library(
         "GNCBleTest.mm",
         "GNCBluetoothAdapterTest.mm",
         "GNCCryptoTest.mm",
+        "GNCIPAddressTest.mm",
         "GNCMultiThreadExecutorTest.mm",
         "GNCScheduledExecutorTest.mm",
         "GNCSingleThreadExecutorTest.mm",
@@ -33,9 +34,15 @@ objc_library(
     ],
     features = ["-layering_check"],
     deps = [
+        "//internal/platform:base",
+        "//internal/platform/implementation:comm",
+        "//internal/platform/implementation:platform",
+        "//internal/platform/implementation:types",
         "//internal/platform/implementation/apple",
-        "//internal/platform/implementation/apple:Shared",
+        "//internal/platform/implementation/apple/Mediums",
+        "//third_party/apple_frameworks:Foundation",
         "//third_party/apple_frameworks:XCTest",
+        "@com_google_absl//absl/time",
     ],
 )
 

internal/platform/implementation/apple/Tests/GNCIPAddressTest.mm

@@ -0,0 +1,114 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#import <Foundation/Foundation.h>
+#import <XCTest/XCTest.h>
+
+#include <string>
+
+#import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCIPv4Address.h"
+
+@interface GNCIPv4AddressTest : XCTestCase
+@end
+
+@implementation GNCIPv4AddressTest
+
+- (void)testInitFromIntWithZero {
+  GNCIPv4Address *address = [GNCIPv4Address addressFromFourByteInt:0];
+  XCTAssertEqual(address.byte1, 0);
+  XCTAssertEqual(address.byte2, 0);
+  XCTAssertEqual(address.byte3, 0);
+  XCTAssertEqual(address.byte4, 0);
+}
+
+- (void)testInitFromIntWithRealIP {
+  GNCIPv4Address *address = [GNCIPv4Address addressFromFourByteInt:2869012672];
+  XCTAssertEqual(address.byte1, 192);
+  XCTAssertEqual(address.byte2, 168);
+  XCTAssertEqual(address.byte3, 1);
+  XCTAssertEqual(address.byte4, 171);
+}
+
+- (void)testInitFromDataWithZero {
+  std::string addressString = "\0\0\0\0";
+  NSData *addressData = [NSData dataWithBytes:addressString.data() length:4];
+  GNCIPv4Address *address = [GNCIPv4Address addressFromData:addressData];
+  XCTAssertEqual(address.byte1, 0);
+  XCTAssertEqual(address.byte2, 0);
+  XCTAssertEqual(address.byte3, 0);
+  XCTAssertEqual(address.byte4, 0);
+}
+
+- (void)testInitFromDataWithLeadingEmptyBytes {
+  std::string addressString = "\0\0\0\0";
+  addressString[3] = static_cast<char>(6);
+
+  NSData *addressData = [NSData dataWithBytes:addressString.data() length:4];
+  GNCIPv4Address *address = [GNCIPv4Address addressFromData:addressData];
+  XCTAssertEqual(address.byte1, 0);
+  XCTAssertEqual(address.byte2, 0);
+  XCTAssertEqual(address.byte3, 0);
+  XCTAssertEqual(address.byte4, 6);
+}
+
+- (void)testInitFromDataWithRealIP {
+  std::string addressString = "\0\0\0\0";
+  addressString[0] = static_cast<char>(192);
+  addressString[1] = static_cast<char>(168);
+  addressString[2] = static_cast<char>(1);
+  addressString[3] = static_cast<char>(171);
+
+  NSData *addressData = [NSData dataWithBytes:addressString.data() length:4];
+  GNCIPv4Address *address = [GNCIPv4Address addressFromData:addressData];
+  XCTAssertEqual(address.byte1, 192);
+  XCTAssertEqual(address.byte2, 168);
+  XCTAssertEqual(address.byte3, 1);
+  XCTAssertEqual(address.byte4, 171);
+}
+
+- (void)testInitFromDataThrowsForWrongByteSize {
+  std::string addressString = "\0";
+  NSData *addressData = [NSData dataWithBytes:addressString.data() length:addressString.size()];
+  XCTAssertThrows([GNCIPv4Address addressFromData:addressData]);
+}
+
+- (void)testDottedRepressentationWithZero {
+  GNCIPv4Address *address = [[GNCIPv4Address alloc] initWithByte1:0 byte2:0 byte3:0 byte4:0];
+  XCTAssertEqualObjects(address.dottedRepresentation, @"0.0.0.0");
+}
+
+- (void)testDottedRepressentationWithRealIP {
+  GNCIPv4Address *address = [[GNCIPv4Address alloc] initWithByte1:192 byte2:168 byte3:1 byte4:171];
+  XCTAssertEqualObjects(address.dottedRepresentation, @"192.168.1.171");
+}
+
+- (void)testBinaryRepressentationWithZero {
+  GNCIPv4Address *address = [[GNCIPv4Address alloc] initWithByte1:0 byte2:0 byte3:0 byte4:0];
+  const uint8_t *bytes = (uint8_t *)address.binaryRepresentation.bytes;
+  XCTAssertEqual(bytes[0], 0);
+  XCTAssertEqual(bytes[1], 0);
+  XCTAssertEqual(bytes[2], 0);
+  XCTAssertEqual(bytes[3], 0);
+}
+
+- (void)testBinaryRepressentationWithRealIP {
+  GNCIPv4Address *address = [[GNCIPv4Address alloc] initWithByte1:192 byte2:168 byte3:1 byte4:171];
+  const uint8_t *bytes = (uint8_t *)address.binaryRepresentation.bytes;
+  XCTAssertEqual(bytes[0], 192);
+  XCTAssertEqual(bytes[1], 168);
+  XCTAssertEqual(bytes[2], 1);
+  XCTAssertEqual(bytes[3], 171);
+}
+
+@end

internal/platform/implementation/apple/wifi_lan.mm

@@ -13,11 +13,13 @@
 // limitations under the License.
 
 #import "internal/platform/implementation/apple/wifi_lan.h"
+#import <Foundation/Foundation.h>
 
 #include <memory>
 #include <string>
 #include <utility>
 
+#import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCIPv4Address.h"
 #import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANMedium.h"
 #import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANServerSocket.h"
 #import "internal/platform/implementation/apple/Mediums/WiFiLAN/GNCWiFiLANSocket.h"
@@ -94,7 +96,8 @@ WifiLanServerSocket::WifiLanServerSocket(GNCWiFiLANServerSocket* server_socket)
     : server_socket_(server_socket) {}
 
 std::string WifiLanServerSocket::GetIPAddress() const {
-  return [server_socket_.ipAddress UTF8String];
+  NSData* addressData = server_socket_.ipAddress.binaryRepresentation;
+  return std::string((char*)addressData.bytes, addressData.length);
 }
 
 int WifiLanServerSocket::GetPort() const { return server_socket_.port; }
@@ -204,7 +207,12 @@ std::unique_ptr<api::WifiLanSocket> WifiLanMedium::ConnectToService(
 std::unique_ptr<api::WifiLanSocket> WifiLanMedium::ConnectToService(
     const std::string& ip_address, int port, CancellationFlag* cancellation_flag) {
   NSError* error = nil;
-  NSString* host = @(ip_address.c_str());
+  if (ip_address.size() != 4) {
+    GTMLoggerError(@"Error IP address must be 4 bytes, but is %lu bytes", ip_address.size());
+    return nil;
+  }
+  NSData* hostData = [NSData dataWithBytes:ip_address.data() length:ip_address.size()];
+  GNCIPv4Address* host = [GNCIPv4Address addressFromData:hostData];
   GNCWiFiLANSocket* socket = [medium_ connectToHost:host port:port error:&error];
   if (socket != nil) {
     return std::make_unique<WifiLanSocket>(socket);