Fix handling of setting a single proxy in Proxy Settings API

chrome/browser/extensions/extension_proxy_api.cc

 // Copyright (c) 2011 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 "chrome/browser/extensions/extension_proxy_api.h"
 
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/prefs/proxy_prefs.h"
@@ skipping 31 matching lines @@
                               "ftp",
                               "socks" };
 
 }  // namespace
 
 COMPILE_ASSERT(SCHEME_MAX == SCHEME_SOCKS, SCHEME_MAX_must_equal_SCHEME_SOCKS);
 COMPILE_ASSERT(arraysize(field_name) == SCHEME_MAX + 1,
                field_name_array_is_wrong_size);
 COMPILE_ASSERT(arraysize(scheme_name) == SCHEME_MAX + 1,
                scheme_name_array_is_wrong_size);
+COMPILE_ASSERT(SCHEME_ALL == 0, singleProxy_must_be_first_option);
 
 void ProxySettingsFunction::ApplyPreference(const char* pref_path,
                                             Value* pref_value,
                                             bool incognito) {
   profile()->GetExtensionService()->extension_prefs()->
       SetExtensionControlledPref(extension_id(), pref_path, incognito,
                                  pref_value);
 }
 
 void ProxySettingsFunction::RemovePreference(const char* pref_path,
@@ skipping 105 matching lines @@
   // singleProxy that will supersede per-URL proxies, but it's worth it to keep
   // the code simple and extensible.
   for (size_t i = 0; i <= SCHEME_MAX; ++i) {
     has_proxy[i] = proxy_rules->GetDictionary(field_name[i], &proxy_dict[i]);
     if (has_proxy[i]) {
       if (!GetProxyServer(proxy_dict[i], &proxy_server[i]))
         return false;
     }
   }
 
-  // A single proxy supersedes individual HTTP, HTTPS, and FTP proxies.
+  // Handle case that only singleProxy is specified.
   if (has_proxy[SCHEME_ALL]) {
-    proxy_server[SCHEME_HTTP] = proxy_server[SCHEME_ALL];
-    proxy_server[SCHEME_HTTPS] = proxy_server[SCHEME_ALL];
-    proxy_server[SCHEME_FTP] = proxy_server[SCHEME_ALL];
-    has_proxy[SCHEME_HTTP] = true;
-    has_proxy[SCHEME_HTTPS] = true;
-    has_proxy[SCHEME_FTP] = true;
-    has_proxy[SCHEME_ALL] = false;
+    for (size_t i = 1; i <= SCHEME_MAX; ++i) {
+      if (has_proxy[i]) {
+        LOG(ERROR) << "Proxy rule for " << field_name[SCHEME_ALL] << " and "
+                   << field_name[i] << " cannot be set at the same time.";
+        return false;
+      }
+    }
+    if (!proxy_server[SCHEME_ALL].scheme.empty())
+      LOG(WARNING) << "Ignoring scheme attribute from proxy server.";
+    // Build the proxy preference string.
+    std::string proxy_pref;
+    proxy_pref.append(proxy_server[SCHEME_ALL].host);
+    if (proxy_server[SCHEME_ALL].port != ProxyServer::INVALID_PORT) {
+      proxy_pref.append(":");

[eroman, 2011/02/08 01:11:07]

It is not generally safe to form host:port pairs simply by concatenating
hostname and ports.

Can you use net::ProxyServer here instead?

A specific case which won't work is IPv6 literals.
Unfortunatly in IPv6, colon is a possible character in the IP address.

So if the host was say ::1 and the port was "80", it would be wrong to form the
concatenated destination as ::1:80. Instead the IPv6 literal needs to be
enclosed in square brackets:

[::1]:80

To avoid these problems, we use HostAndPortPair::ToString() to form the correct
string in cases when the hostname is an IP literal.

I realize that problem is not new with this change, but the new code seems to be
continuing the same problem.

I am ok with handling this issue separately (don't need to do it as part of this
CL).

+      proxy_pref.append(base::StringPrintf("%d",
+                                           proxy_server[SCHEME_ALL].port));
+    }
+    *out = proxy_pref;
+    return true;
   }
 
-  // TODO(pamg): Ensure that if a value is empty, that means "don't use a proxy
-  // for this scheme".
+  // Handle case the anything but singleProxy is specified.
 
   // Build the proxy preference string.
   std::string proxy_pref;
-  for (size_t i = 0; i <= SCHEME_MAX; ++i) {
+  for (size_t i = 1; i <= SCHEME_MAX; ++i) {
     if (has_proxy[i]) {
       // http=foopy:4010;ftp=socks://foopy2:80
       if (!proxy_pref.empty())
         proxy_pref.append(";");
       proxy_pref.append(scheme_name[i]);
       proxy_pref.append("=");
       proxy_pref.append(proxy_server[i].scheme);
       proxy_pref.append("://");
       proxy_pref.append(proxy_server[i].host);
       if (proxy_server[i].port != ProxyServer::INVALID_PORT) {
         proxy_pref.append(":");
         proxy_pref.append(base::StringPrintf("%d", proxy_server[i].port));
       }
     }
   }
 
   *out = proxy_pref;
   return true;
 }
 
 bool RemoveCustomProxySettingsFunction::RunImpl() {
   bool incognito = false;
   args_->GetBoolean(0, &incognito);
 
   RemovePreference(prefs::kProxy, incognito);
   return true;
 }