arc/net: replace ARC proxy bypass list delimiter

Chrome forwards the proxy exception bypass list as a string with host names
delimited by semicolon [;] while ARC expects a colon [,] delimiter. This
causes loss of network conectivity in ARC.

This CL formats the proxy bypass list sent to ARC by replacing delimiters
and removing the rendundant delimiter at the end of the string bypass list.

  1. setup an Ethernet proxy connection with no bypass list
  2. verified ARC and ARC app have network connectivity
  3. added one host in the bypass list
  4. verified ARC connectivity is lost
  5. deployed Chrome with fix on the device
  6. verified ARC has network connectivity
  7. verified app on ARC respects proxy and proxy bypass item
  8. added three host names in the bypass list
  9. verified ARC app respects bypass list for all hostnames

Bug: b:133193788
Change-Id: I363879fe886e3c7abc6557ef163b0d930398b7d7
Tests: manually tested with following steps
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630481Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664960}

chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc

@@ -12,6 +12,9 @@
 #include "base/gtest_prod_util.h"
 #include "base/json/json_writer.h"
 #include "base/memory/singleton.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/arc/arc_util.h"
@@ -47,6 +50,7 @@
 #include "components/proxy_config/proxy_config_dictionary.h"
 #include "components/proxy_config/proxy_config_pref_names.h"
 #include "content/public/common/page_zoom.h"
+#include "net/proxy_resolution/proxy_bypass_rules.h"
 #include "net/proxy_resolution/proxy_config.h"
 
 using ::chromeos::system::TimezoneSettings;
@@ -58,6 +62,8 @@ constexpr char kSetFontScaleAction[] =
 constexpr char kSetPageZoomAction[] =
     "org.chromium.arc.intent_helper.SET_PAGE_ZOOM";
 
+constexpr char kArcProxyBypassListDelimeter[] = ",";
+
 bool GetHttpProxyServer(const ProxyConfigDictionary* proxy_config_dict,
                         std::string* host,
                         int* port) {
@@ -547,6 +553,14 @@ void ArcSettingsServiceImpl::SyncProxySettings() const {
       std::string bypass_list;
       if (proxy_config_dict->GetBypassList(&bypass_list) &&
           !bypass_list.empty()) {
+        // Chrome uses semicolon [;] as delimiter for the proxy bypass list
+        // while ARC expects comma [,] delimiter.  Using the wrong delimiter
+        // causes loss of network connectivity for many apps in ARC.
+        auto bypassed_hosts = base::SplitStringPiece(
+            bypass_list, net::ProxyBypassRules::kBypassListDelimeter,
+            base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+        bypass_list =
+            base::JoinString(bypassed_hosts, kArcProxyBypassListDelimeter);
         extras.SetString("bypassList", bypass_list);
       }
       break;

chrome/browser/chromeos/arc/intent_helper/arc_settings_service_browsertest.cc

@@ -592,6 +592,32 @@ IN_PROC_BROWSER_TEST_F(ArcSettingsServiceTest, DefaultNetworkProxyConfigTest) {
             1);
 }
 
+IN_PROC_BROWSER_TEST_F(ArcSettingsServiceTest, ProxyBypassListTest) {
+  fake_intent_helper_instance_->clear_broadcasts();
+
+  const char kChromeProxyBypassList[] = "test1.org;test2.org;";
+  const char kArcProxyBypassList[] = "test1.org,test2.org";
+
+  base::Value proxy_config(base::Value::Type::DICTIONARY);
+  proxy_config.SetKey("mode",
+                      base::Value(ProxyPrefs::kFixedServersProxyModeName));
+  proxy_config.SetKey("server", base::Value("proxy:8080"));
+  proxy_config.SetKey("bypass_list", base::Value(kChromeProxyBypassList));
+  SetProxyConfigForNetworkService(kDefaultServicePath, std::move(proxy_config));
+  RunUntilIdle();
+
+  base::Value expected_proxy_config(base::Value::Type::DICTIONARY);
+  expected_proxy_config.SetKey(
+      "mode", base::Value(ProxyPrefs::kFixedServersProxyModeName));
+  expected_proxy_config.SetKey("host", base::Value("proxy"));
+  expected_proxy_config.SetKey("port", base::Value(8080));
+  expected_proxy_config.SetKey("bypassList", base::Value(kArcProxyBypassList));
+
+  EXPECT_EQ(CountProxyBroadcasts(fake_intent_helper_instance_->broadcasts(),
+                                 &expected_proxy_config),
+            1);
+}
+
 IN_PROC_BROWSER_TEST_F(ArcSettingsServiceTest, DefaultNetworkDisconnectedTest) {
   ConnectWifiNetworkService(kWifi0ServicePath, kWifi0Guid, kWifi0Ssid);
   fake_intent_helper_instance_->clear_broadcasts();

net/proxy_resolution/proxy_bypass_rules.cc

@@ -270,6 +270,8 @@ std::unique_ptr<ProxyBypassRules::Rule> ParseRule(
 
 }  // namespace
 
+constexpr char net::ProxyBypassRules::kBypassListDelimeter[];
+
 ProxyBypassRules::Rule::Rule() = default;
 
 ProxyBypassRules::Rule::~Rule() = default;
@@ -403,7 +405,7 @@ std::string ProxyBypassRules::ToString() const {
   std::string result;
   for (auto rule(rules_.begin()); rule != rules_.end(); ++rule) {
     result += (*rule)->ToString();
-    result += ";";
+    result += kBypassListDelimeter;
   }
   return result;
 }

net/proxy_resolution/proxy_bypass_rules.h

@@ -164,6 +164,10 @@ class NET_EXPORT ProxyBypassRules {
   // (localhost or link local).
   static bool MatchesImplicitRules(const GURL& url);
 
+  // The delimiter used by |ToString()| for the string representation of the
+  // proxy bypass rules.
+  constexpr static char kBypassListDelimeter[] = ";";
+
  private:
   RuleList rules_;
 };