Implement KDE ReversedException setting in Chrome

R=wtc,eroman
BUG=45199
TEST=Added unit tests, can also verify with steps in bug report

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

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

8 files changed, 143 insertions, 27 deletions

diff --git a/net/proxy/proxy_config.cc b/net/proxy/proxy_config.cc
index 6196330..6e73101 100644
--- a/net/proxy/proxy_config.cc
+++ b/net/proxy/proxy_config.cc
@@ -17,11 +17,20 @@ bool ProxyConfig::ProxyRules::Equals(const ProxyRules& other) const {
          proxy_for_https == other.proxy_for_https &&
          proxy_for_ftp == other.proxy_for_ftp &&
          socks_proxy == other.socks_proxy &&
-         bypass_rules.Equals(other.bypass_rules);
+         bypass_rules.Equals(other.bypass_rules) &&
+         reverse_bypass == other.reverse_bypass;
 }
 
 void ProxyConfig::ProxyRules::Apply(const GURL& url, ProxyInfo* result) {
-  if (empty() || bypass_rules.Matches(url)) {
+  if (empty()) {
+    result->UseDirect();
+    return;
+  }
+
+  bool bypass_proxy = bypass_rules.Matches(url);
+  if (reverse_bypass)
+    bypass_proxy = !bypass_proxy;
+  if (bypass_proxy) {
     result->UseDirect();
     return;
   }
@@ -216,7 +225,10 @@ std::ostream& operator<<(std::ostream& out, const net::ProxyConfig& config) {
       break;
   }
 
-  out << "  Bypass list: ";
+  if (config.proxy_rules().reverse_bypass)
+    out << "  Only use proxy for: ";
+  else
+    out << "  Bypass list: ";
   if (config.proxy_rules().bypass_rules.rules().empty()) {
     out << "[None]";
   } else {
diff --git a/net/proxy/proxy_config.h b/net/proxy/proxy_config.h
index 58e70e3..a247e38 100644
--- a/net/proxy/proxy_config.h
+++ b/net/proxy/proxy_config.h
@@ -41,7 +41,7 @@ class ProxyConfig {
 
     // Note that the default of TYPE_NO_RULES results in direct connections
     // being made when using this ProxyConfig.
-    ProxyRules() : type(TYPE_NO_RULES) {}
+    ProxyRules() : reverse_bypass(false), type(TYPE_NO_RULES) {}
 
     bool empty() const {
       return type == TYPE_NO_RULES;
@@ -81,6 +81,9 @@ class ProxyConfig {
     // Exceptions for when not to use a proxy.
     ProxyBypassRules bypass_rules;
 
+    // Reverse the meaning of |bypass_rules|.
+    bool reverse_bypass;
+
     Type type;
 
     // Set if |type| is TYPE_SINGLE_PROXY.
diff --git a/net/proxy/proxy_config_service_common_unittest.cc b/net/proxy/proxy_config_service_common_unittest.cc
index 7757cca..9e5fd76 100644
--- a/net/proxy/proxy_config_service_common_unittest.cc
+++ b/net/proxy/proxy_config_service_common_unittest.cc
@@ -77,20 +77,27 @@ std::string FlattenProxyBypass(const ProxyBypassRules& bypass_rules) {
     failed = true;
   }
 
+  if (rules.reverse_bypass != reverse_bypass) {
+    failure_details << "Bad reverse_bypass. Expected: " << reverse_bypass
+                    << " but got: " << rules.reverse_bypass;
+    failed = true;
+  }
+
   return failed ? failure_details : ::testing::AssertionSuccess();
 }
 
 // static
 ProxyRulesExpectation ProxyRulesExpectation::Empty() {
   return ProxyRulesExpectation(ProxyConfig::ProxyRules::TYPE_NO_RULES,
-                               "", "", "", "", "", "");
+                               "", "", "", "", "", "", false);
 }
 
 // static
 ProxyRulesExpectation ProxyRulesExpectation::EmptyWithBypass(
     const char* flattened_bypass_rules) {
   return ProxyRulesExpectation(ProxyConfig::ProxyRules::TYPE_NO_RULES,
-                               "", "", "", "", "", flattened_bypass_rules);
+                               "", "", "", "", "", flattened_bypass_rules,
+                               false);
 }
 
 // static
@@ -99,7 +106,7 @@ ProxyRulesExpectation ProxyRulesExpectation::Single(
     const char* flattened_bypass_rules) {
   return ProxyRulesExpectation(ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY,
                                single_proxy, "", "", "", "",
-                               flattened_bypass_rules);
+                               flattened_bypass_rules, false);
 }
 
 // static
@@ -110,7 +117,7 @@ ProxyRulesExpectation ProxyRulesExpectation::PerScheme(
     const char* flattened_bypass_rules) {
   return ProxyRulesExpectation(ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
                                "", proxy_http, proxy_https, proxy_ftp, "",
-                               flattened_bypass_rules);
+                               flattened_bypass_rules, false);
 }
 
 // static
@@ -122,7 +129,18 @@ ProxyRulesExpectation ProxyRulesExpectation::PerSchemeWithSocks(
     const char* flattened_bypass_rules) {
   return ProxyRulesExpectation(ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
                                "", proxy_http, proxy_https, proxy_ftp,
-                               socks_proxy, flattened_bypass_rules);
+                               socks_proxy, flattened_bypass_rules, false);
+}
+
+// static
+ProxyRulesExpectation ProxyRulesExpectation::PerSchemeWithBypassReversed(
+    const char* proxy_http,
+    const char* proxy_https,
+    const char* proxy_ftp,
+    const char* flattened_bypass_rules) {
+  return ProxyRulesExpectation(ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
+                               "", proxy_http, proxy_https, proxy_ftp, "",
+                               flattened_bypass_rules, true);
 }
 
 }  // namespace net
diff --git a/net/proxy/proxy_config_service_common_unittest.h b/net/proxy/proxy_config_service_common_unittest.h
index c6f14b7..3b70ef0 100644
--- a/net/proxy/proxy_config_service_common_unittest.h
+++ b/net/proxy/proxy_config_service_common_unittest.h
@@ -27,14 +27,16 @@ struct ProxyRulesExpectation {
                         const char* proxy_for_https,
                         const char* proxy_for_ftp,
                         const char* socks_proxy,
-                        const char* flattened_bypass_rules)
+                        const char* flattened_bypass_rules,
+                        bool reverse_bypass)
     : type(type),
       single_proxy(single_proxy),
       proxy_for_http(proxy_for_http),
       proxy_for_https(proxy_for_https),
       proxy_for_ftp(proxy_for_ftp),
       socks_proxy(socks_proxy),
-      flattened_bypass_rules(flattened_bypass_rules) {
+      flattened_bypass_rules(flattened_bypass_rules),
+      reverse_bypass(reverse_bypass) {
   }
 
 
@@ -71,6 +73,13 @@ struct ProxyRulesExpectation {
       const char* socks_proxy,
       const char* flattened_bypass_rules);
 
+  // Same as PerScheme, but with the bypass rules reversed
+  static ProxyRulesExpectation PerSchemeWithBypassReversed(
+      const char* proxy_http,
+      const char* proxy_https,
+      const char* proxy_ftp,
+      const char* flattened_bypass_rules);
+
   ProxyConfig::ProxyRules::Type type;
   const char* single_proxy;
   const char* proxy_for_http;
@@ -78,6 +87,7 @@ struct ProxyRulesExpectation {
   const char* proxy_for_ftp;
   const char* socks_proxy;
   const char* flattened_bypass_rules;
+  bool reverse_bypass;
 };
 
 }  // namespace net
diff --git a/net/proxy/proxy_config_service_linux.cc b/net/proxy/proxy_config_service_linux.cc
index bf25ebb..072a5ae 100644
--- a/net/proxy/proxy_config_service_linux.cc
+++ b/net/proxy/proxy_config_service_linux.cc
@@ -350,6 +350,11 @@ class GConfSettingGetterImplGConf
     return true;
   }
 
+  virtual bool BypassListIsReversed() {
+    // This is a KDE-specific setting.
+    return false;
+  }
+
  private:
   // Logs and frees a glib error. Returns false if there was no error
   // (error is NULL).
@@ -413,7 +418,7 @@ class GConfSettingGetterImplKDE
  public:
   explicit GConfSettingGetterImplKDE(base::EnvVarGetter* env_var_getter)
       : inotify_fd_(-1), notify_delegate_(NULL), indirect_manual_(false),
-        auto_no_pac_(false), reversed_exception_(false),
+        auto_no_pac_(false), reversed_bypass_list_(false),
         env_var_getter_(env_var_getter), file_loop_(NULL) {
     // Derive the location of the kde config dir from the environment.
     std::string home;
@@ -569,13 +574,17 @@ class GConfSettingGetterImplKDE
     return true;
   }
 
+  virtual bool BypassListIsReversed() {
+    return reversed_bypass_list_;
+  }
+
  private:
   void ResetCachedSettings() {
     string_table_.clear();
     strings_table_.clear();
     indirect_manual_ = false;
     auto_no_pac_ = false;
-    reversed_exception_ = false;
+    reversed_bypass_list_ = false;
   }
 
   FilePath KDEHomeToConfigPath(const FilePath& kde_home) {
@@ -645,7 +654,7 @@ class GConfSettingGetterImplKDE
       // We count "true" or any nonzero number as true, otherwise false.
       // Note that if the value is not actually numeric StringToInt()
       // will return 0, which we count as false.
-      reversed_exception_ = (value == "true" || StringToInt(value));
+      reversed_bypass_list_ = (value == "true" || StringToInt(value));
     } else if (key == "NoProxyFor") {
       AddHostList("/system/http_proxy/ignore_hosts", value);
     } else if (key == "AuthMode") {
@@ -698,14 +707,6 @@ class GConfSettingGetterImplKDE
       // Remove the PAC URL; we're not supposed to use it.
       string_table_.erase("/system/proxy/autoconfig_url");
     }
-    if (reversed_exception_) {
-      // We don't actually support this setting. (It means to use the proxy
-      // *only* for the exception list, rather than everything but them.)
-      // Nevertheless we can do better than *exactly the opposite* of the
-      // desired behavior by clearing the exception list and warning.
-      strings_table_.erase("/system/http_proxy/ignore_hosts");
-      LOG(WARNING) << "KDE reversed proxy exception list not supported";
-    }
   }
 
   // Reads kioslaverc one line at a time and calls AddKDESetting() to add
@@ -859,7 +860,7 @@ class GConfSettingGetterImplKDE
   FilePath kde_config_dir_;
   bool indirect_manual_;
   bool auto_no_pac_;
-  bool reversed_exception_;
+  bool reversed_bypass_list_;
   // We don't own |env_var_getter_|.  It's safe to hold a pointer to it, since
   // both it and us are owned by ProxyConfigServiceLinux::Delegate, and have the
   // same lifetime.
@@ -1022,6 +1023,9 @@ bool ProxyConfigServiceLinux::Delegate::GetConfigFromGConf(
   // Note that there are no settings with semantics corresponding to
   // bypass of local names.
 
+  // KDE allows one to reverse the bypass rules.
+  config->proxy_rules().reverse_bypass = gconf_getter_->BypassListIsReversed();
+
   return true;
 }
 
diff --git a/net/proxy/proxy_config_service_linux.h b/net/proxy/proxy_config_service_linux.h
index 904441d..a038e75 100644
--- a/net/proxy/proxy_config_service_linux.h
+++ b/net/proxy/proxy_config_service_linux.h
@@ -76,6 +76,10 @@ class ProxyConfigServiceLinux : public ProxyConfigService {
     virtual bool GetStringList(const char* key,
                                std::vector<std::string>* result) = 0;
 
+    // Returns true if the bypass list should be interpreted as a proxy
+    // whitelist rather than blacklist. (This is KDE-specific.)
+    virtual bool BypassListIsReversed() = 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 ba0bb09..6b0a882 100644
--- a/net/proxy/proxy_config_service_linux_unittest.cc
+++ b/net/proxy/proxy_config_service_linux_unittest.cc
@@ -213,6 +213,10 @@ class MockGConfSettingGetter
     return !result->empty();
   }
 
+  virtual bool BypassListIsReversed() {
+    return false;
+  }
+
   // Intentionally public, for convenience when setting up a test.
   GConfValues values;
 
@@ -1077,11 +1081,11 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
 
       false,                                   // auto_detect
       GURL(),                                  // pac_url
-      ProxyRulesExpectation::PerScheme(
+      ProxyRulesExpectation::PerSchemeWithBypassReversed(
           "www.google.com:80",  // http
           "",                   // https
           "",                   // ftp
-          ""),                  // bypass rules
+          "*.google.com"),      // bypass rules
     },
 
     {
@@ -1094,11 +1098,11 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
 
       false,                                   // auto_detect
       GURL(),                                  // pac_url
-      ProxyRulesExpectation::PerScheme(
+      ProxyRulesExpectation::PerSchemeWithBypassReversed(
           "www.google.com:80",  // http
           "",                   // https
           "",                   // ftp
-          ""),                  // bypass rules
+          "*.google.com"),      // bypass rules
     },
 
     {
diff --git a/net/proxy/proxy_config_unittest.cc b/net/proxy/proxy_config_unittest.cc
index 2182897..5806f30 100644
--- a/net/proxy/proxy_config_unittest.cc
+++ b/net/proxy/proxy_config_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "net/proxy/proxy_config.h"
 #include "net/proxy/proxy_config_service_common_unittest.h"
+#include "net/proxy/proxy_info.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -82,6 +83,18 @@ TEST(ProxyConfigTest, Equals) {
 
   EXPECT_TRUE(config1.Equals(config2));
   EXPECT_TRUE(config2.Equals(config1));
+
+  // Test |ProxyConfig::proxy_rules.reverse_bypass|.
+
+  config2.proxy_rules().reverse_bypass = true;
+
+  EXPECT_FALSE(config1.Equals(config2));
+  EXPECT_FALSE(config2.Equals(config1));
+
+  config1.proxy_rules().reverse_bypass = true;
+
+  EXPECT_TRUE(config1.Equals(config2));
+  EXPECT_TRUE(config2.Equals(config1));
 }
 
 TEST(ProxyConfigTest, ParseProxyRules) {
@@ -348,6 +361,28 @@ TEST(ProxyConfigTest, ToString) {
               "  Bypass list: [None]",
               ProxyConfigToString(config));
   }
+
+  // Manual proxy with bypass list + bypass local, list reversed.
+  {
+    ProxyConfig config;
+    config.set_auto_detect(false);
+    config.proxy_rules().ParseFromString("http://single-proxy:81");
+    config.proxy_rules().bypass_rules.AddRuleFromString("google.com");
+    config.proxy_rules().bypass_rules.AddRuleFromString("bypass2.net:1730");
+    config.proxy_rules().bypass_rules.AddRuleToBypassLocal();
+    config.proxy_rules().reverse_bypass = true;
+
+    EXPECT_EQ("Automatic settings:\n"
+              "  Auto-detect: No\n"
+              "  Custom PAC script: [None]\n"
+              "Manual settings:\n"
+              "  Proxy server: single-proxy:81\n"
+              "  Only use proxy for: \n"
+              "    google.com\n"
+              "    bypass2.net:1730\n"
+              "    <local>",
+              ProxyConfigToString(config));
+  }
 }
 
 TEST(ProxyConfigTest, MayRequirePACResolver) {
@@ -372,6 +407,32 @@ TEST(ProxyConfigTest, MayRequirePACResolver) {
   }
 }
 
+TEST(ProxyConfigTest, ReversedBypassList) {
+  {
+    ProxyConfig config;
+    config.set_auto_detect(false);
+    config.proxy_rules().ParseFromString("http://single-proxy:81");
+    config.proxy_rules().bypass_rules.AddRuleFromString("google.com");
+    config.proxy_rules().bypass_rules.AddRuleFromString("bypass2.net:1730");
+    config.proxy_rules().bypass_rules.AddRuleToBypassLocal();
+    config.proxy_rules().reverse_bypass = true;
+
+    ProxyInfo info[3];
+    GURL url0("http://google.com");
+    GURL url1("http://www.webkit.com");
+    GURL url2("http://bypass2.net:1730");
+
+    config.proxy_rules().Apply(url0, &info[0]);
+    EXPECT_EQ("single-proxy:81", info[0].proxy_server().ToURI());
+
+    config.proxy_rules().Apply(url1, &info[1]);
+    EXPECT_TRUE(info[1].is_direct());
+
+    config.proxy_rules().Apply(url2, &info[2]);
+    EXPECT_EQ("single-proxy:81", info[2].proxy_server().ToURI());
+  }
+}
+
 }  // namespace
 }  // namespace net