Support bypassList in Proxy Settings API.

chrome/browser/extensions/extension_proxy_api.cc

Index: chrome/browser/extensions/extension_proxy_api.cc
diff --git a/chrome/browser/extensions/extension_proxy_api.cc b/chrome/browser/extensions/extension_proxy_api.cc
index f816528b6b769a23ceb7109a23cdb197965da0bd..235d0ee460fb4eb4ce6f4cd1efff34a8a5d2e2cf 100644
--- a/chrome/browser/extensions/extension_proxy_api.cc
+++ b/chrome/browser/extensions/extension_proxy_api.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/extensions/extension_proxy_api.h"
 
 #include "base/string_util.h"
+#include "base/string_tokenizer.h"
 #include "base/values.h"
 #include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chrome/browser/profiles/profile.h"
@@ -49,6 +50,7 @@ const char kProxyCfgPacScriptUrl[] = "url";
 const char kProxyCfgRules[] = "rules";
 const char kProxyCfgRuleHost[] = "host";
 const char kProxyCfgRulePort[] = "port";
+const char kProxyCfgBypassList[] = "bypassList";
 const char kProxyCfgScheme[] = "scheme";
 
 COMPILE_ASSERT(SCHEME_MAX == SCHEME_SOCKS, SCHEME_MAX_must_equal_SCHEME_SOCKS);
@@ -58,6 +60,32 @@ COMPILE_ASSERT(arraysize(scheme_name) == SCHEME_MAX + 1,
                scheme_name_array_is_wrong_size);
 COMPILE_ASSERT(SCHEME_ALL == 0, singleProxy_must_be_first_option);
 
+bool TokenizeToStringList(
+    const std::string& in, const std::string& delims, ListValue** out) {
+  scoped_ptr<ListValue> result(new ListValue);
+  StringTokenizer entries(in, delims);
+  while (entries.GetNext()) {
+    result->Append(Value::CreateStringValue(entries.token()));
+  }
+  *out = result.release();
+  return true;
+}
+
+bool JoinStringList(
+    ListValue* list, const std::string& joiner, std::string* out) {
+  std::string result;
+  for (size_t i = 0; i < list->GetSize(); ++i) {
+    if (!result.empty())
+      result.append(joiner);
+    std::string entry;
+    if (!list->GetString(i, &entry))
+      return false;
+    result.append(entry);
+  }
+  *out = result;
+  return true;
+}
+
 // Converts a proxy server description |dict| as passed by the API caller
 // (e.g. for the http proxy in the rules element) and converts it to a
 // ProxyServer. Returns true if successful.
@@ -146,6 +174,25 @@ bool GetProxyRules(DictionaryValue* proxy_rules, std::string* out) {
   return true;
 }
 
+// Retrieves the "bypassList" from a proxy "rules" element passed by the API
+// caller in a proxy configuration and returns it as a joined string.

[eroman, 2011/02/11 04:52:28]

The wording of this comment doesn't seem right.

[battre, 2011/02/11 15:06:03]

Done.

+// Returns true if successful (i.e. string could be delivered or no "bypassList"
+// exists in the |proxy_rules|).
+bool GetBypassList(DictionaryValue* proxy_rules, std::string* out) {
+  if (!proxy_rules)
+    return false;
+
+  ListValue* bypass_list;
+  if (!proxy_rules->HasKey(kProxyCfgBypassList)) {
+    *out = "";
+    return true;
+  }
+  if (!proxy_rules->GetList(kProxyCfgBypassList, &bypass_list))
+    return false;
+
+  return JoinStringList(bypass_list, ",", out);
+}
+
 }  // namespace
 
 void ProxySettingsFunction::ApplyPreference(const char* pref_path,
@@ -205,9 +252,12 @@ bool UseCustomProxySettingsFunction::RunImpl() {
                << "Setting custom proxy settings failed.";
     return false;
   }
-
-  // not supported, yet.
   std::string bypass_list;
+  if (proxy_rules && !GetBypassList(proxy_rules, &bypass_list)) {

[eroman, 2011/02/11 04:52:28]

There are string encoding problems in this file.

Note that strings originating from the javascript are wide. When calling
DictionaryValue:GetString() with a std::string, really we are getting UTF-8
encoded strings.

However a lot of this file is doing tokenizing assuming an ASCII input. And
certainly the net::* proxy parsing functions which are ultimately passed these
inputs, is similarly expecting ASCII input.

I generally recommend avoiding this pitfall by only ever extracting strings from
Value's using string16 -- this forces the code to have to deal with unicode of
the string, and avoids passing it around to functions that take ASCII inputs
(since you get compiletime failures!)

For now you might want to call DictionaryValue::GetStringAsASCII(), so the other
code doesn't need to be prepared to handle UTF8, and then just file some
TODOs/bugs on handling wide inputs.

You might be wandering whether it makes sense for javascript to be passing wide
strings -- yes it does. As far as the network stack is concerned, hostnames are
always ASCII. However as far as the user is concerned, international hostnames
can be represented using unicode (there is a translation layer to ASCII called
punycode).

This same issues has come up for PAC scripts, which similarly are javascript
code doing proxy stuff. We currently handle wide hostnames issued by PAC scripts
correctly (in the dnsResolve()/dnsResolveEx() bindings. However there is an
outstanding bug to create UTF16 aware parsing routines for proxy lists. See
http://crbug.com/47234.

[battre, 2011/02/11 15:06:03]

I have replaced all GetString calls with GetStringASCII where extension_api.json
prescribes an enum value and therefore guarantees an ASCII value.

For URLs I have used GetString(string16*), and bail out if IsStringASCII(string)
returns false. Otherwise I convert the string with UTF16ToASCII().

I have added TODO(battre)s with a reference to http://crbug.com/72692 in the
code.

So for now, the proxy settings API will accept only ASCII strings. I will do the
UTF-8 CodeLab and see how I can resolve this.

Done.

+    LOG(ERROR) << "Invalid 'bypassList' specified. "
+               << "Setting custom proxy settings failed.";
+    return false;
+  }
 
   DictionaryValue* result_proxy_config = NULL;
   switch (mode_enum) {
@@ -315,17 +365,33 @@ bool GetCurrentProxySettingsFunction::ConvertToApiFormat(
       break;
     }
     case ProxyPrefs::MODE_FIXED_SERVERS: {
-      // TODO(battre): Handle bypass list.
+      DictionaryValue* rules_dict = new DictionaryValue;
+
       std::string proxy_servers;
       if (!dict.GetProxyServer(&proxy_servers)) {
-        LOG(ERROR) << "Missing proxy servers";
+        LOG(ERROR) << "Missing proxy servers in configuration";
         return false;
       }
-      DictionaryValue* rules_dict = new DictionaryValue;
       if (!ParseRules(proxy_servers, rules_dict)) {
         LOG(ERROR) << "Could not parse proxy rules";
         return false;
       }
+
+      bool hasBypassList = dict.HasBypassList();
+      if (hasBypassList) {
+        std::string bypass_list_string;
+        if (!dict.GetBypassList(&bypass_list_string)) {
+          LOG(ERROR) << "Invalid bypassList in configuration";
+          return false;

[eroman, 2011/02/11 04:52:28]

See comment below.

[battre, 2011/02/11 15:06:03]

Done.

+        }
+        ListValue* bypass_list = NULL;
+        if (TokenizeToStringList(bypass_list_string, ",;", &bypass_list)) {
+          rules_dict->Set(kProxyCfgBypassList, bypass_list);
+        } else {
+          LOG(ERROR) << "Error parsing bypassList " << bypass_list_string;
+          return false;

[eroman, 2011/02/11 04:52:28]

Isn't this going to leak |rules_dict|?

I suggest putting rules_dict into a scoped_ptr<> to avoid that.

[battre, 2011/02/11 15:06:03]

Done.

+        }
+      }
       api_proxy_config->Set(kProxyCfgRules, rules_dict);
       break;
     }