KDE treats all host patterns as wildcard patterns

Also correct a test description that became inaccurate when we supported a
reversed bypass list, and remove spaces from host rules earlier. (KDE actually
treats both comma and space as valid delimiters anyway.)

R=wtc,eroman
BUG=48486
TEST=ProxyConfigServiceLinuxTest.KDEConfigParser

Review URL: http://codereview.chromium.org/2848041

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51971 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 65 insertions, 9 deletions

diff --git a/net/proxy/proxy_bypass_rules.cc b/net/proxy/proxy_bypass_rules.cc
index 50481c4..e273d67 100644
--- a/net/proxy/proxy_bypass_rules.cc
+++ b/net/proxy/proxy_bypass_rules.cc
@@ -174,6 +174,11 @@ bool ProxyBypassRules::AddRuleFromString(const std::string& raw) {
   return AddRuleFromStringInternalWithLogging(raw, false);
 }
 
+bool ProxyBypassRules::AddRuleFromStringUsingSuffixMatching(
+    const std::string& raw) {
+  return AddRuleFromStringInternalWithLogging(raw, true);
+}
+
 void ProxyBypassRules::Clear() {
   rules_.clear();
 }
diff --git a/net/proxy/proxy_bypass_rules.h b/net/proxy/proxy_bypass_rules.h
index 76d1dcc..267fdc9 100644
--- a/net/proxy/proxy_bypass_rules.h
+++ b/net/proxy/proxy_bypass_rules.h
@@ -137,6 +137,17 @@ class ProxyBypassRules {
   //
   bool AddRuleFromString(const std::string& raw);
 
+  // This is a variant of AddFromString, which interprets hostname patterns as
+  // suffix tests rather than hostname tests (so "google.com" would actually
+  // match "*google.com"). This is used for KDE which interprets every rule as
+  // a suffix test. It is less flexible, since with the suffix matching format
+  // you can't match an individual host.
+  //
+  // Returns true if the rule was successfully added.
+  //
+  // NOTE: Use AddRuleFromString() unless you truly need this behavior.
+  bool AddRuleFromStringUsingSuffixMatching(const std::string& raw);
+
   // Removes all the rules.
   void Clear();
 
diff --git a/net/proxy/proxy_config_service_linux.cc b/net/proxy/proxy_config_service_linux.cc
index 072a5ae..eadedbf6 100644
--- a/net/proxy/proxy_config_service_linux.cc
+++ b/net/proxy/proxy_config_service_linux.cc
@@ -355,6 +355,10 @@ class GConfSettingGetterImplGConf
     return false;
   }
 
+  virtual bool MatchHostsUsingSuffixMatching() {
+    return false;
+  }
+
  private:
   // Logs and frees a glib error. Returns false if there was no error
   // (error is NULL).
@@ -578,6 +582,10 @@ class GConfSettingGetterImplKDE
     return reversed_bypass_list_;
   }
 
+  virtual bool MatchHostsUsingSuffixMatching() {
+    return true;
+  }
+
  private:
   void ResetCachedSettings() {
     string_table_.clear();
@@ -603,7 +611,7 @@ class GConfSettingGetterImplKDE
 
   void AddHostList(const std::string& key, const std::string& value) {
     std::vector<std::string> tokens;
-    StringTokenizer tk(value, ",");
+    StringTokenizer tk(value, ", ");
     while (tk.GetNext()) {
       std::string token = tk.token();
       if (!token.empty())
@@ -1017,11 +1025,18 @@ bool ProxyConfigServiceLinux::Delegate::GetConfigFromGConf(
   if (gconf_getter_->GetStringList("/system/http_proxy/ignore_hosts",
                                    &ignore_hosts_list)) {
     std::vector<std::string>::const_iterator it(ignore_hosts_list.begin());
-    for (; it != ignore_hosts_list.end(); ++it)
-      config->proxy_rules().bypass_rules.AddRuleFromString(*it);
+    for (; it != ignore_hosts_list.end(); ++it) {
+      if (gconf_getter_->MatchHostsUsingSuffixMatching()) {
+        config->proxy_rules().bypass_rules.
+            AddRuleFromStringUsingSuffixMatching(*it);
+      } else {
+        config->proxy_rules().bypass_rules.AddRuleFromString(*it);
+      }
+    }
   }
   // Note that there are no settings with semantics corresponding to
-  // bypass of local names.
+  // bypass of local names in GNOME. In KDE, "<local>" is supported
+  // as a hostname rule.
 
   // KDE allows one to reverse the bypass rules.
   config->proxy_rules().reverse_bypass = gconf_getter_->BypassListIsReversed();
diff --git a/net/proxy/proxy_config_service_linux.h b/net/proxy/proxy_config_service_linux.h
index a038e75..c6e9c90 100644
--- a/net/proxy/proxy_config_service_linux.h
+++ b/net/proxy/proxy_config_service_linux.h
@@ -80,6 +80,10 @@ class ProxyConfigServiceLinux : public ProxyConfigService {
     // whitelist rather than blacklist. (This is KDE-specific.)
     virtual bool BypassListIsReversed() = 0;
 
+    // Returns true if the bypass rules should be interpreted as
+    // suffix-matching rules.
+    virtual bool MatchHostsUsingSuffixMatching() = 0;
+
    private:
     DISALLOW_COPY_AND_ASSIGN(GConfSettingGetter);
   };
diff --git a/net/proxy/proxy_config_service_linux_unittest.cc b/net/proxy/proxy_config_service_linux_unittest.cc
index 501a411..ca9835b 100644
--- a/net/proxy/proxy_config_service_linux_unittest.cc
+++ b/net/proxy/proxy_config_service_linux_unittest.cc
@@ -220,6 +220,10 @@ class MockGConfSettingGetter
     return false;
   }
 
+  virtual bool MatchHostsUsingSuffixMatching() {
+    return false;
+  }
+
   // Intentionally public, for convenience when setting up a test.
   GConfValues values;
 
@@ -1041,7 +1045,7 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
 
       // Input.
       "[Proxy Settings]\nProxyType=1\nhttpProxy=www.google.com\n"
-          "NoProxyFor=*.google.com\n",
+          "NoProxyFor=.google.com\n",
       {},                                      // env_values
 
       false,                                   // auto_detect
@@ -1058,7 +1062,7 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
 
       // Input.
       "[Proxy Settings]\nProxyType=1\nhttpProxy=www.google.com\n"
-          "NoProxyFor=*.google.com,*.kde.org\n",
+          "NoProxyFor=.google.com,.kde.org\n",
       {},                                      // env_values
 
       false,                                   // auto_detect
@@ -1071,11 +1075,11 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
     },
 
     {
-      TEST_DESC("Ignore bypass list with ReversedException"),
+      TEST_DESC("Correctly parse bypass list with ReversedException"),
 
       // Input.
       "[Proxy Settings]\nProxyType=1\nhttpProxy=www.google.com\n"
-          "NoProxyFor=*.google.com\nReversedException=true\n",
+          "NoProxyFor=.google.com\nReversedException=true\n",
       {},                                      // env_values
 
       false,                                   // auto_detect
@@ -1088,11 +1092,28 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
     },
 
     {
+      TEST_DESC("Treat all hostname patterns as wildcard patterns"),
+
+      // Input.
+      "[Proxy Settings]\nProxyType=1\nhttpProxy=www.google.com\n"
+          "NoProxyFor=google.com,kde.org,<local>\n",
+      {},                                      // env_values
+
+      false,                                   // auto_detect
+      GURL(),                                  // pac_url
+      ProxyRulesExpectation::PerScheme(
+          "www.google.com:80",              // http
+          "",                               // https
+          "",                               // ftp
+          "*google.com,*kde.org,<local>"),  // bypass rules
+    },
+
+    {
       TEST_DESC("Allow trailing whitespace after boolean value"),
 
       // Input.
       "[Proxy Settings]\nProxyType=1\nhttpProxy=www.google.com\n"
-          "NoProxyFor=*.google.com\nReversedException=true  \n",
+          "NoProxyFor=.google.com\nReversedException=true  \n",
       {},                                      // env_values
 
       false,                                   // auto_detect