Change ProxyRules to handle ProxyLists rather than just single ProxyServer instances.

net/proxy/proxy_config_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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 {
 namespace {
 
 void ExpectProxyServerEquals(const char* expectation,
-                             const ProxyServer& proxy_server) {
+                             const ProxyList& proxy_servers) {
   if (expectation == NULL) {
-    EXPECT_FALSE(proxy_server.is_valid());
+    EXPECT_TRUE(proxy_servers.IsEmpty());
   } else {
-    EXPECT_EQ(expectation, proxy_server.ToURI());
+    EXPECT_EQ(expectation, proxy_servers.ToPacString());
   }
 }
 
 TEST(ProxyConfigTest, Equals) {
   // Test |ProxyConfig::auto_detect|.
 
   ProxyConfig config1;
   config1.set_auto_detect(true);
 
   ProxyConfig config2;
@@ skipping 15 matching lines @@
   EXPECT_FALSE(config2.Equals(config1));
 
   config1.set_pac_url(GURL("http://wpad/wpad.dat"));
 
   EXPECT_TRUE(config1.Equals(config2));
   EXPECT_TRUE(config2.Equals(config1));
 
   // Test |ProxyConfig::proxy_rules|.
 
   config2.proxy_rules().type = ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY;
-  config2.proxy_rules().single_proxy =
-      ProxyServer::FromURI("myproxy:80", ProxyServer::SCHEME_HTTP);
+  config2.proxy_rules().single_proxies.SetSingleProxyServer(
+      ProxyServer::FromURI("myproxy:80", ProxyServer::SCHEME_HTTP));
 
   EXPECT_FALSE(config1.Equals(config2));
   EXPECT_FALSE(config2.Equals(config1));
 
   config1.proxy_rules().type = ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY;
-  config1.proxy_rules().single_proxy =
-      ProxyServer::FromURI("myproxy:100", ProxyServer::SCHEME_HTTP);
+  config1.proxy_rules().single_proxies.SetSingleProxyServer(
+      ProxyServer::FromURI("myproxy:100", ProxyServer::SCHEME_HTTP));
 
   EXPECT_FALSE(config1.Equals(config2));
   EXPECT_FALSE(config2.Equals(config1));
 
-  config1.proxy_rules().single_proxy =
-      ProxyServer::FromURI("myproxy", ProxyServer::SCHEME_HTTP);
+  config1.proxy_rules().single_proxies.SetSingleProxyServer(
+      ProxyServer::FromURI("myproxy", ProxyServer::SCHEME_HTTP));
 
   EXPECT_TRUE(config1.Equals(config2));
   EXPECT_TRUE(config2.Equals(config1));
 
   // Test |ProxyConfig::bypass_rules|.
 
   config2.proxy_rules().bypass_rules.AddRuleFromString("*.google.com");
 
   EXPECT_FALSE(config1.Equals(config2));
   EXPECT_FALSE(config2.Equals(config1));
@@ skipping 14 matching lines @@
 
   EXPECT_TRUE(config1.Equals(config2));
   EXPECT_TRUE(config2.Equals(config1));
 }
 
 TEST(ProxyConfigTest, ParseProxyRules) {
   const struct {
     const char* proxy_rules;
 
     ProxyConfig::ProxyRules::Type type;
+    // These will be PAC-stle strings, eg 'PROXY foo.com'
     const char* single_proxy;
     const char* proxy_for_http;
     const char* proxy_for_https;
     const char* proxy_for_ftp;
     const char* fallback_proxy;
   } tests[] = {
     // One HTTP proxy for all schemes.
     {
       "myproxy:80",
 
       ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY,
-      "myproxy:80",
+      "PROXY myproxy:80",
       NULL,
       NULL,
       NULL,
       NULL,
     },
 
     // Only specify a proxy server for "http://" urls.
     {
       "http=myproxy:80",
 
       ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
       NULL,
-      "myproxy:80",
+      "PROXY myproxy:80",
       NULL,
       NULL,
       NULL,
     },
 
     // Specify an HTTP proxy for "ftp://" and a SOCKS proxy for "https://" urls.
     {
       "ftp=ftp-proxy ; https=socks4://foopy",
 
       ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
       NULL,
       NULL,
-      "socks4://foopy:1080",
-      "ftp-proxy:80",
+      "SOCKS foopy:1080",
+      "PROXY ftp-proxy:80",
       NULL,
     },
 
     // Give a scheme-specific proxy as well as a non-scheme specific.
     // The first entry "foopy" takes precedance marking this list as
     // TYPE_SINGLE_PROXY.
     {
       "foopy ; ftp=ftp-proxy",
 
       ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY,
-      "foopy:80",
+      "PROXY foopy:80",
       NULL,
       NULL,
       NULL,
       NULL,
     },
 
     // Give a scheme-specific proxy as well as a non-scheme specific.
     // The first entry "ftp=ftp-proxy" takes precedance marking this list as
     // TYPE_PROXY_PER_SCHEME.
     {
       "ftp=ftp-proxy ; foopy",
 
       ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
       NULL,
       NULL,
       NULL,
-      "ftp-proxy:80",
+      "PROXY ftp-proxy:80",
       NULL,
     },
 
-    // Include duplicate entries -- last one wins.

[eroman, 2013/03/01 00:05:13]

Please keep this test as well (need a test to see what happens when there are
multiple scheme=<list> entries; i don't really care what the particular policy
is, as long as it is tested).

[marq_use_my_chromium_address, 2013/03/04 18:04:20]

Done.

+    // Include duplicate entries for a single scheme.
     {
-      "ftp=ftp1 ; ftp=ftp2 ; ftp=ftp3",
+      "ftp=ftp1,ftp2,ftp3",
 
       ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
       NULL,
       NULL,
       NULL,
-      "ftp3:80",
+      "PROXY ftp1:80;PROXY ftp2:80;PROXY ftp3:80",
       NULL,
     },
 
+    // Include duplicate entries for a multiple schemes.
+    {
+      "ftp=ftp1,ftp2,ftp3 ; http=http1,http2; ",
+
+      ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
+      NULL,
+      "PROXY http1:80;PROXY http2:80",
+      NULL,
+      "PROXY ftp1:80;PROXY ftp2:80;PROXY ftp3:80",
+      NULL,
+    },
+
+    // Include non-default proxy schemes.
+    {
+      "http=https://secure_proxy; ftp=socks4://socks_proxy; https=socks://foo",
+
+      ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
+      NULL,
+      "HTTPS secure_proxy:443",
+      "SOCKS5 foo:1080",
+      "SOCKS socks_proxy:1080",
+      NULL,
+    },
+
+
     // Only SOCKS proxy present, others being blank.
     {
       "socks=foopy",
 
       ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
       NULL,
       NULL,
       NULL,
       NULL,
-      "socks4://foopy:1080",
+      "SOCKS foopy:1080",
       },
 
     // SOCKS proxy present along with other proxies too
     {
       "http=httpproxy ; https=httpsproxy ; ftp=ftpproxy ; socks=foopy ",
 
       ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
       NULL,
-      "httpproxy:80",
-      "httpsproxy:80",
-      "ftpproxy:80",
-      "socks4://foopy:1080",
+      "PROXY httpproxy:80",
+      "PROXY httpsproxy:80",
+      "PROXY ftpproxy:80",
+      "SOCKS foopy:1080",
     },
 
     // SOCKS proxy (with modifier) present along with some proxies
     // (FTP being blank)
     {
       "http=httpproxy ; https=httpsproxy ; socks=socks5://foopy ",
 
       ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
       NULL,
-      "httpproxy:80",
-      "httpsproxy:80",
+      "PROXY httpproxy:80",
+      "PROXY httpsproxy:80",
       NULL,
-      "socks5://foopy:1080",
+      "SOCKS5 foopy:1080",
       },
 
     // Include unsupported schemes -- they are discarded.
     {
       "crazy=foopy ; foo=bar ; https=myhttpsproxy",
 
       ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
       NULL,
       NULL,
-      "myhttpsproxy:80",
+      "PROXY myhttpsproxy:80",
       NULL,
       NULL,
     },
+
+    // direct:// discarded as a first or only option for a scheme.
+    {
+      "http=direct://,myhttpproxy; https=direct://",
+
+      ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
+      NULL,
+      "DIRECT;PROXY myhttpproxy:80",
+      "DIRECT",
+      NULL,
+      NULL,
+    },
+
+    // direct:// kept as a second option for a scheme.
+    {
+      "http=myhttpproxy,direct://",
+
+      ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME,
+      NULL,
+      "PROXY myhttpproxy:80;DIRECT",
+      NULL,
+      NULL,
+      NULL,
+    },
+
   };
 
   ProxyConfig config;
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
     config.proxy_rules().ParseFromString(tests[i].proxy_rules);
 
     EXPECT_EQ(tests[i].type, config.proxy_rules().type);
     ExpectProxyServerEquals(tests[i].single_proxy,
-                            config.proxy_rules().single_proxy);
+                            config.proxy_rules().single_proxies);
     ExpectProxyServerEquals(tests[i].proxy_for_http,
-                            config.proxy_rules().proxy_for_http);
+                            config.proxy_rules().proxies_for_http);
     ExpectProxyServerEquals(tests[i].proxy_for_https,
-                            config.proxy_rules().proxy_for_https);
+                            config.proxy_rules().proxies_for_https);
     ExpectProxyServerEquals(tests[i].proxy_for_ftp,
-                            config.proxy_rules().proxy_for_ftp);
+                            config.proxy_rules().proxies_for_ftp);
     ExpectProxyServerEquals(tests[i].fallback_proxy,
-                            config.proxy_rules().fallback_proxy);
+                            config.proxy_rules().fallback_proxies);
   }
 }
 
 TEST(ProxyConfigTest, ProxyRulesSetBypassFlag) {
   // Test whether the did_bypass_proxy() flag is set in proxy info correctly.
   ProxyConfig::ProxyRules rules;
   ProxyInfo  result;
 
   rules.ParseFromString("http=httpproxy:80");
   rules.bypass_rules.AddRuleFromString(".com");
@@ skipping 13 matching lines @@
   EXPECT_TRUE(result.is_direct_only());
   EXPECT_TRUE(result.did_bypass_proxy());
 
   rules.Apply(GURL("http://example.com"), &result);
   EXPECT_FALSE(result.is_direct());
   EXPECT_FALSE(result.did_bypass_proxy());
 }
 
 }  // namespace
 }  // namespace net