mobile: Fixes to the Apple Core Foundation to C++ conversion

And adds unit tests for the Apple utility functions.

Signed-off-by: Ali Beyad <abeyad@google.com>

mobile/library/common/apple/utility.cc

@@ -6,6 +6,10 @@ namespace Envoy {
 namespace Apple {
 
 std::string toString(CFStringRef cf_string) {
+  if (cf_string == nullptr) {
+    return std::string();
+  }
+
   // A pointer to a C string or NULL if the internal storage of string
   // does not allow this to be returned efficiently.
   // CFStringGetCStringPtr will return a pointer to the string with no memory allocation and in
@@ -16,15 +20,18 @@ std::string toString(CFStringRef cf_string) {
   }
 
   CFIndex length = CFStringGetLength(cf_string);
-  // Adding 1 to accomodate the `\0` null delimiter in a C string.
+  if (length == 0) {
+    return std::string();
+  }
+
+  // Adding 1 to accommodate the `\0` null delimiter in a C string.
   CFIndex size = CFStringGetMaximumSizeForEncoding(length, kCFStringEncodingUTF8) + 1;
   char* c_str = static_cast<char*>(malloc(size));
   // Use less efficient method of getting c string if CFStringGetCStringPtr failed.
   const bool ret = CFStringGetCString(cf_string, c_str, size, kCFStringEncodingUTF8);
   ENVOY_BUG(ret, "CFStringGetCString failed to convert CFStringRef to C string.");
 
-  std::string ret_str;
-  ret_str = std::string(c_str);
+  std::string ret_str(c_str);
 
   free(c_str);
   return ret_str;

mobile/library/common/network/apple_system_proxy_settings_monitor.cc

@@ -81,14 +81,20 @@ AppleSystemProxySettingsMonitor::readSystemProxySettings() const {
   if (is_http_proxy_enabled) {
     CFStringRef cf_hostname =
         static_cast<CFStringRef>(CFDictionaryGetValue(proxy_settings, kCFNetworkProxiesHTTPProxy));
+    std::string hostname = Apple::toString(cf_hostname);
     CFNumberRef cf_port =
         static_cast<CFNumberRef>(CFDictionaryGetValue(proxy_settings, kCFNetworkProxiesHTTPPort));
-    settings = absl::make_optional<SystemProxySettings>(Apple::toString(cf_hostname),
-                                                        Apple::toInt(cf_port));
+    int port = Apple::toInt(cf_port);
+    if (!hostname.empty() && port > 0) {
+      settings = absl::make_optional<SystemProxySettings>(std::move(hostname), port);
+    }
   } else if (is_auto_config_proxy_enabled) {
     CFStringRef cf_pac_file_url_string = static_cast<CFStringRef>(
         CFDictionaryGetValue(proxy_settings, kCFNetworkProxiesProxyAutoConfigURLString));
-    settings = absl::make_optional<SystemProxySettings>(Apple::toString(cf_pac_file_url_string));
+    std::string pac_file_url_str = Apple::toString(cf_pac_file_url_string);
+    if (!pac_file_url_str.empty()) {
+      settings = absl::make_optional<SystemProxySettings>(std::move(pac_file_url_str));
+    }
   }
 
   CFRelease(proxy_settings);

mobile/library/common/network/connectivity_manager.cc

@@ -114,8 +114,6 @@ void ConnectivityManagerImpl::setProxySettings(ProxySettingsConstSharedPtr new_p
     ENVOY_LOG_EVENT(info, "netconf_proxy_change", "{}", new_proxy_settings->asString());
     proxy_settings_ = new_proxy_settings;
   }
-
-  return;
 }
 
 ProxySettingsConstSharedPtr ConnectivityManagerImpl::getProxySettings() { return proxy_settings_; }

mobile/test/common/apple/BUILD

@@ -0,0 +1,22 @@
+load("@envoy//bazel:envoy_build_system.bzl", "envoy_cc_test", "envoy_mobile_package")
+
+licenses(["notice"])  # Apache 2
+
+envoy_mobile_package()
+
+envoy_cc_test(
+    name = "utility_test",
+    srcs = select({
+        "@envoy//bazel:apple": [
+            "utility_test.cc",
+        ],
+        "//conditions:default": [],
+    }),
+    repository = "@envoy",
+    deps = select({
+        "@envoy//bazel:apple": [
+            "//library/common/apple:utility_lib",
+        ],
+        "//conditions:default": [],
+    }),
+)

mobile/test/common/apple/utility_test.cc

@@ -0,0 +1,36 @@
+#include "gtest/gtest.h"
+#include "library/common/apple/utility.h"
+
+namespace Envoy {
+namespace Apple {
+
+TEST(AppleUtilityTest, ToString) {
+  CFStringRef cf_string = CFSTR("hello, world");
+  ASSERT_EQ("hello, world", Apple::toString(cf_string));
+  CFRelease(cf_string);
+}
+
+TEST(AppleUtilityTest, ToStringEmpty) {
+  CFStringRef cf_string = CFSTR("");
+  ASSERT_EQ("", Apple::toString(cf_string));
+  CFRelease(cf_string);
+}
+
+TEST(AppleUtilityTest, ToStringNull) {
+  CFStringRef cf_string = nullptr;
+  ASSERT_EQ("", Apple::toString(cf_string));
+}
+
+TEST(AppleUtilityTest, ToInt) {
+  int int_value = 42;
+  CFNumberRef cf_number = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &int_value);
+  ASSERT_EQ(int_value, Apple::toInt(cf_number));
+}
+
+TEST(AppleUtilityTest, ToIntNull) {
+  CFNumberRef cf_number = nullptr;
+  ASSERT_EQ(0, Apple::toInt(cf_number));
+}
+
+} // namespace Apple
+} // namespace Envoy