Making command-line specified proxy settings more flexible - allowing for setting of auto-detect, pac url, per-schema proxy settings, proxy bypass urls.

BUG=http://crbug.com/266

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

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

9 files changed, 162 insertions, 104 deletions

diff --git a/net/proxy/proxy_config.cc b/net/proxy/proxy_config.cc
index a860c99..751049b 100644
--- a/net/proxy/proxy_config.cc
+++ b/net/proxy/proxy_config.cc
@@ -25,6 +25,10 @@ bool ProxyConfig::Equals(const ProxyConfig& other) const {
          proxy_bypass_local_names == other.proxy_bypass_local_names;
 }
 
+bool ProxyConfig::MayRequirePACResolver() const {
+  return auto_detect || !pac_url.is_empty();
+}
+
 void ProxyConfig::ProxyRules::ParseFromString(const std::string& proxy_rules) {
   // Reset.
   type = TYPE_NO_RULES;
@@ -76,6 +80,71 @@ const ProxyServer* ProxyConfig::ProxyRules::MapSchemeToProxy(
   return NULL;  // No mapping for this scheme.
 }
 
+namespace {
+
+// Returns true if the given string represents an IP address.
+bool IsIPAddress(const std::string& domain) {
+  // From GURL::HostIsIPAddress()
+  url_canon::RawCanonOutputT<char, 128> ignored_output;
+  url_parse::Component ignored_component;
+  url_parse::Component domain_comp(0, domain.size());
+  return url_canon::CanonicalizeIPAddress(domain.c_str(), domain_comp,
+                                          &ignored_output,
+                                          &ignored_component);
+}
+
+}  // namespace
+
+void ProxyConfig::ParseNoProxyList(const std::string& no_proxy) {
+  proxy_bypass.clear();
+  if (no_proxy.empty())
+    return;
+  // Traditional semantics:
+  // A single "*" is specifically allowed and unproxies anything.
+  // "*" wildcards other than a single "*" entry are not universally
+  // supported. We will support them, as we get * wildcards for free
+  // (see MatchPattern() called from ProxyService::ShouldBypassProxyForURL()).
+  // no_proxy is a comma-separated list of <trailing_domain>[:<port>].
+  // If no port is specified then any port matches.
+  // The historical definition has trailing_domain match using a simple
+  // string "endswith" test, so that the match need not correspond to a
+  // "." boundary. For example: "google.com" matches "igoogle.com" too.
+  // Seems like that could be confusing, but we'll obey tradition.
+  // IP CIDR patterns are supposed to be supported too. We intend
+  // to do this in proxy_service.cc, but it's currently a TODO.
+  // See: http://crbug.com/9835.
+  StringTokenizer no_proxy_list(no_proxy, ",");
+  while (no_proxy_list.GetNext()) {
+    std::string bypass_entry = no_proxy_list.token();
+    TrimWhitespaceASCII(bypass_entry, TRIM_ALL, &bypass_entry);
+    if (bypass_entry.empty())
+      continue;
+    if (bypass_entry.at(0) != '*') {
+      // Insert a wildcard * to obtain an endsWith match, unless the
+      // entry looks like it might be an IP or CIDR.
+      // First look for either a :<port> or CIDR mask length suffix.
+      std::string::const_iterator begin = bypass_entry.begin();
+      std::string::const_iterator scan = bypass_entry.end() - 1;
+      while (scan > begin && IsAsciiDigit(*scan))
+        --scan;
+      std::string potential_ip;
+      if (*scan == '/' || *scan == ':')
+        potential_ip = std::string(begin, scan - 1);
+      else
+        potential_ip = bypass_entry;
+      if (!IsIPAddress(potential_ip)) {
+        // Do insert a wildcard.
+        bypass_entry.insert(0, "*");
+      }
+      // TODO(sdoyon): When CIDR matching is implemented in
+      // proxy_service.cc, consider making proxy_bypass more
+      // sophisticated to avoid parsing out the string on every
+      // request.
+    }
+    proxy_bypass.push_back(bypass_entry);
+  }
+}
+
 }  // namespace net
 
 namespace {
diff --git a/net/proxy/proxy_config.h b/net/proxy/proxy_config.h
index 868fba9..b90e9b7 100644
--- a/net/proxy/proxy_config.h
+++ b/net/proxy/proxy_config.h
@@ -42,6 +42,8 @@ class ProxyConfig {
       TYPE_PROXY_PER_SCHEME,
     };
 
+    // Note that the default of TYPE_NO_RULES results in direct connections
+    // being made when using this ProxyConfig.
     ProxyRules() : type(TYPE_NO_RULES) {}
 
     bool empty() const {
@@ -91,6 +93,11 @@ class ProxyConfig {
 
   ProxyRules proxy_rules;
 
+  // Parses entries from a comma-separated list of hosts for which proxy
+  // configurations should be bypassed. Clears proxy_bypass and sets it to the
+  // resulting list.
+  void ParseNoProxyList(const std::string& no_proxy);
+
   // Indicates a list of hosts that should bypass any proxy configuration.  For
   // these hosts, a direct connection should always be used.
   // The form <host>:<port> is also supported, meaning that only
@@ -103,6 +110,10 @@ class ProxyConfig {
   // Returns true if the given config is equivalent to this config.
   bool Equals(const ProxyConfig& other) const;
 
+  // Returns true if this config could possibly require the proxy service to
+  // use a PAC resolver.
+  bool MayRequirePACResolver() const;
+
  private:
   int id_;
 };
diff --git a/net/proxy/proxy_config_service_fixed.h b/net/proxy/proxy_config_service_fixed.h
index dfa9351..361c2b4 100644
--- a/net/proxy/proxy_config_service_fixed.h
+++ b/net/proxy/proxy_config_service_fixed.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2009 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.
 
@@ -12,17 +12,16 @@ namespace net {
 // Implementation of ProxyConfigService that returns a fixed result.
 class ProxyConfigServiceFixed : public ProxyConfigService {
  public:
-  explicit ProxyConfigServiceFixed(const ProxyInfo& pi) { pi_.Use(pi); }
+  explicit ProxyConfigServiceFixed(const ProxyConfig& pc) : pc_(pc) {}
 
   // ProxyConfigService methods:
   virtual int GetProxyConfig(ProxyConfig* config) {
-    config->proxy_rules.type = ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY;
-    config->proxy_rules.single_proxy = pi_.proxy_server();
+    *config = pc_;
     return OK;
   }
 
  private:
-  ProxyInfo pi_;
+  ProxyConfig pc_;
 };
 
 }  // namespace net
diff --git a/net/proxy/proxy_config_service_linux.cc b/net/proxy/proxy_config_service_linux.cc
index 3c94cdf..cce5835 100644
--- a/net/proxy/proxy_config_service_linux.cc
+++ b/net/proxy/proxy_config_service_linux.cc
@@ -114,71 +114,6 @@ bool ProxyConfigServiceLinux::GetProxyFromEnvVar(
                                      result_server);
 }
 
-namespace {
-
-// Returns true if the given string represents an IP address.
-bool IsIPAddress(const std::string& domain) {
-  // From GURL::HostIsIPAddress()
-  url_canon::RawCanonOutputT<char, 128> ignored_output;
-  url_parse::Component ignored_component;
-  url_parse::Component domain_comp(0, domain.size());
-  return url_canon::CanonicalizeIPAddress(domain.c_str(), domain_comp,
-                                          &ignored_output,
-                                          &ignored_component);
-}
-
-}  // namespace
-
-void ProxyConfigServiceLinux::ParseNoProxyList(const std::string& no_proxy,
-                                               ProxyConfig* config) {
-  if (no_proxy.empty())
-    return;
-  // Traditional semantics:
-  // A single "*" is specifically allowed and unproxies anything.
-  // "*" wildcards other than a single "*" entry are not universally
-  // supported. We will support them, as we get * wildcards for free
-  // (see MatchPattern() called from ProxyService::ShouldBypassProxyForURL()).
-  // no_proxy is a comma-separated list of <trailing_domain>[:<port>].
-  // If no port is specified then any port matches.
-  // The historical definition has trailing_domain match using a simple
-  // string "endswith" test, so that the match need not correspond to a
-  // "." boundary. For example: "google.com" matches "igoogle.com" too.
-  // Seems like that could be confusing, but we'll obey tradition.
-  // IP CIDR patterns are supposed to be supported too. We intend
-  // to do this in proxy_service.cc, but it's currently a TODO.
-  // See: http://crbug.com/9835.
-  StringTokenizer no_proxy_list(no_proxy, ",");
-  while (no_proxy_list.GetNext()) {
-    std::string bypass_entry = no_proxy_list.token();
-    TrimWhitespaceASCII(bypass_entry, TRIM_ALL, &bypass_entry);
-    if (bypass_entry.empty())
-      continue;
-    if (bypass_entry.at(0) != '*') {
-      // Insert a wildcard * to obtain an endsWith match, unless the
-      // entry looks like it might be an IP or CIDR.
-      // First look for either a :<port> or CIDR mask length suffix.
-      std::string::const_iterator begin = bypass_entry.begin();
-      std::string::const_iterator scan = bypass_entry.end() - 1;
-      while (scan > begin && IsAsciiDigit(*scan))
-        --scan;
-      std::string potential_ip;
-      if (*scan == '/' || *scan == ':')
-        potential_ip = std::string(begin, scan - 1);
-      else
-        potential_ip = bypass_entry;
-      if (!IsIPAddress(potential_ip)) {
-        // Do insert a wildcard.
-        bypass_entry.insert(0, "*");
-      }
-      // TODO(sdoyon): When CIDR matching is implemented in
-      // proxy_service.cc, consider making config->proxy_bypass more
-      // sophisticated to avoid parsing out the string on every
-      // request.
-    }
-    config->proxy_bypass.push_back(bypass_entry);
-  }
-}
-
 bool ProxyConfigServiceLinux::GetConfigFromEnv(ProxyConfig* config) {
   // Check for automatic configuration first, in
   // "auto_proxy". Possibly only the "environment_proxy" firefox
@@ -242,7 +177,7 @@ bool ProxyConfigServiceLinux::GetConfigFromEnv(ProxyConfig* config) {
     // connections.
     return !no_proxy.empty();
   }
-  ParseNoProxyList(no_proxy, config);
+  config->ParseNoProxyList(no_proxy);
   return true;
 }
 
diff --git a/net/proxy/proxy_config_service_linux.h b/net/proxy/proxy_config_service_linux.h
index 51436af..354bd29 100644
--- a/net/proxy/proxy_config_service_linux.h
+++ b/net/proxy/proxy_config_service_linux.h
@@ -81,9 +81,7 @@ class ProxyConfigServiceLinux : public ProxyConfigService {
                                    ProxyServer* result_server);
   // As above but with scheme set to HTTP, for convenience.
   bool GetProxyFromEnvVar(const char* variable, ProxyServer* result_server);
-  // Parses entries from the value of the no_proxy env var, and stuffs
-  // them into config->proxy_bypass.
-  void ParseNoProxyList(const std::string& no_proxy, ProxyConfig* config);
+
   // Fills proxy config from environment variables. Returns true if
   // variables were found and the configuration is valid.
   bool GetConfigFromEnv(ProxyConfig* config);
diff --git a/net/proxy/proxy_config_unittest.cc b/net/proxy/proxy_config_unittest.cc
index 9e50e3b..a4192df 100644
--- a/net/proxy/proxy_config_unittest.cc
+++ b/net/proxy/proxy_config_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "net/proxy/proxy_config.h"
+#include "net/proxy/proxy_config_service_common_unittest.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -197,3 +198,36 @@ TEST(ProxyConfigTest, ParseProxyRules) {
                             config.proxy_rules.proxy_for_ftp);
   }
 }
+
+TEST(ProxyConfigTest, ParseProxyBypassList) {
+  struct bypass_test {
+    const char* proxy_bypass_input;
+    const char* flattened_output;
+  };
+
+  const struct {
+    const char* proxy_bypass_input;
+    const char* flattened_output;
+  } tests[] = {
+    {
+      "*",
+      "*\n"
+    },
+    {
+      ".google.com, .foo.com:42",
+      "*.google.com\n*.foo.com:42\n"
+    },
+    {
+      ".google.com, foo.com:99, 1.2.3.4:22, 127.0.0.1/8",
+      "*.google.com\n*foo.com:99\n1.2.3.4:22\n127.0.0.1/8\n"
+    }
+  };
+
+  net::ProxyConfig config;
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
+    config.ParseNoProxyList(tests[i].proxy_bypass_input);
+    EXPECT_EQ(tests[i].flattened_output,
+              net::FlattenProxyBypass(config.proxy_bypass));
+  }
+}
diff --git a/net/proxy/proxy_script_fetcher_unittest.cc b/net/proxy/proxy_script_fetcher_unittest.cc
index fae8c2d..e1c4179 100644
--- a/net/proxy/proxy_script_fetcher_unittest.cc
+++ b/net/proxy/proxy_script_fetcher_unittest.cc
@@ -27,7 +27,7 @@ struct FetchResult {
 class RequestContext : public URLRequestContext {
  public:
   RequestContext() {
-    net::ProxyInfo no_proxy;
+    net::ProxyConfig no_proxy;
     proxy_service_ = net::ProxyService::Create(&no_proxy);
     http_transaction_factory_ = net::HttpNetworkLayer::CreateFactory(
         proxy_service_);
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index 0919f45..c2ae532 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -71,8 +71,8 @@ class NotifyFetchCompletionTask : public Task {
 // We rely on the fact that the origin thread (and its message loop) will not
 // be destroyed until after the PAC thread is destroyed.
 
-class ProxyService::PacRequest :
-    public base::RefCountedThreadSafe<ProxyService::PacRequest> {
+class ProxyService::PacRequest
+    : public base::RefCountedThreadSafe<ProxyService::PacRequest> {
  public:
   // |service| -- the ProxyService that owns this request.
   // |url|     -- the url of the query.
@@ -193,46 +193,58 @@ ProxyService::ProxyService(ProxyConfigService* config_service,
 }
 
 // static
-ProxyService* ProxyService::Create(const ProxyInfo* pi) {
-  if (pi) {
-    // The ProxyResolver is set to NULL, since it should never be called
-    // (because the configuration will never require PAC).
-    return new ProxyService(new ProxyConfigServiceFixed(*pi), NULL);
+ProxyService* ProxyService::Create(const ProxyConfig* pc) {
+  scoped_ptr<ProxyConfigService> proxy_config_service;
+  scoped_ptr<ProxyResolver> proxy_resolver;
+  if (pc) {
+    proxy_config_service.reset(new ProxyConfigServiceFixed(*pc));
   }
+
 #if defined(OS_WIN)
-  return new ProxyService(new ProxyConfigServiceWin(),
-                          new ProxyResolverWinHttp());
+  if (proxy_config_service == NULL)
+    proxy_config_service.reset(new ProxyConfigServiceWin());
+  proxy_resolver.reset(new ProxyResolverWinHttp());
 #elif defined(OS_MACOSX)
-  return new ProxyService(new ProxyConfigServiceMac(),
-                          new ProxyResolverMac());
+  if (proxy_config_service == NULL)
+    proxy_config_service.reset(new ProxyConfigServiceMac());
+  proxy_resolver.reset(new ProxyResolverMac());
 #elif defined(OS_LINUX)
   // On Linux we use the V8Resolver, no fallback implementation.
-  return CreateNull();
+  // This means that if we got called with a ProxyConfig that could require a
+  // resolver, or if the caller is trying to create a regular ProxyService via
+  // this method instead of CreateUsingV8Resolver() we have to return a
+  // nulled-out ProxyService.
+  if (!pc || pc->MayRequirePACResolver()) {
+    LOG(WARNING) << "Attempting to create a ProxyService with a non-v8 "
+                 << "resolver using an invalid ProxyConfig.";
+    return CreateNull();
+  }
 #else
   return CreateNull();
 #endif
+
+  return new ProxyService(proxy_config_service.release(),
+                          proxy_resolver.release());
 }
 
 // static
 ProxyService* ProxyService::CreateUsingV8Resolver(
-    const ProxyInfo* pi, URLRequestContext* url_request_context) {
-  if (pi) {
-    // The ProxyResolver is set to NULL, since it should never be called
-    // (because the configuration will never require PAC).
-    return new ProxyService(new ProxyConfigServiceFixed(*pi), NULL);
-  }
-
+    const ProxyConfig* pc, URLRequestContext* url_request_context) {
   // Choose the system configuration service appropriate for each platform.
-  ProxyConfigService* config_service;
+  ProxyConfigService* config_service = NULL;
+  if (pc) {
+    config_service = new ProxyConfigServiceFixed(*pc);
+  } else {
 #if defined(OS_WIN)
-  config_service = new ProxyConfigServiceWin();
+    config_service = new ProxyConfigServiceWin();
 #elif defined(OS_MACOSX)
-  config_service = new ProxyConfigServiceMac();
+    config_service = new ProxyConfigServiceMac();
 #elif defined(OS_LINUX)
-  config_service = new ProxyConfigServiceLinux();
+    config_service = new ProxyConfigServiceLinux();
 #else
-  return CreateNull();
+    return CreateNull();
 #endif
+  }
 
   // Create a ProxyService that uses V8 to evaluate PAC scripts.
   ProxyService* proxy_service = new ProxyService(
diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h
index 3ebee8d..f2fbbe7 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -86,14 +86,14 @@ class ProxyService {
   // |proxy_script_fetcher|.
   void SetProxyScriptFetcher(ProxyScriptFetcher* proxy_script_fetcher);
 
-  // Creates a proxy service using the specified settings. If |pi| is NULL then
+  // Creates a proxy service using the specified settings. If |pc| is NULL then
   // the system's default proxy settings will be used (on Windows this will
   // use IE's settings).
-  static ProxyService* Create(const ProxyInfo* pi);
+  static ProxyService* Create(const ProxyConfig* pc);
 
-  // Creates a proxy service using the specified settings. If |pi| is NULL then
+  // Creates a proxy service using the specified settings. If |pc| is NULL then
   // the system's default proxy settings will be used. This is basically the
-  // same as Create(const ProxyInfo*), however under the hood it uses a
+  // same as Create(const ProxyConfig*), however under the hood it uses a
   // different implementation (V8). |url_request_context| is the URL request
   // context that will be used if a PAC script needs to be fetched.
   // ##########################################################################
@@ -102,7 +102,7 @@ class ProxyService {
   // # other V8's running in the process must use v8::Locker.
   // ##########################################################################
   static ProxyService* CreateUsingV8Resolver(
-      const ProxyInfo* pi,
+      const ProxyConfig* pc,
       URLRequestContext* url_request_context);
 
   // Creates a proxy service that always fails to fetch the proxy configuration,