Translate ONC ProxySettings <-> Shill ProxyConfig

chromeos/network/onc/onc_utils.cc

Index: chromeos/network/onc/onc_utils.cc
diff --git a/chromeos/network/onc/onc_utils.cc b/chromeos/network/onc/onc_utils.cc
index 2e4d384c4985fdde5ef2b0fb49f789fa373654f8..d89857ab4216eb0a3061ac6d10c7fc31f93ccfde 100644
--- a/chromeos/network/onc/onc_utils.cc
+++ b/chromeos/network/onc/onc_utils.cc
@@ -17,11 +17,15 @@
 #include "chromeos/network/onc/onc_utils.h"
 #include "chromeos/network/onc/onc_validator.h"
 #include "components/device_event_log/device_event_log.h"
+#include "components/proxy_config/proxy_config_dictionary.h"
 #include "crypto/encryptor.h"
 #include "crypto/hmac.h"
 #include "crypto/symmetric_key.h"
+#include "net/base/host_port_pair.h"
 #include "net/cert/pem_tokenizer.h"
 #include "net/cert/x509_certificate.h"
+#include "net/proxy/proxy_bypass_rules.h"
+#include "net/proxy/proxy_server.h"
 
 using namespace ::onc;
 
@@ -738,5 +742,219 @@ bool IsRecommendedValue(const base::DictionaryValue* onc,
           recommended_keys->end());
 }
 
+namespace {
+
+const char kSchemeFtp[] = "ftp";
+const char kSchemeHttp[] = "http";
+const char kSchemeHttps[] = "https";
+const char kSchemeSocks[] = "socks";
+
+net::ProxyServer ConvertOncProxyLocationToHostPort(
+    net::ProxyServer::Scheme default_proxy_scheme,
+    const base::DictionaryValue& onc_proxy_location) {
+  std::string host;
+  onc_proxy_location.GetStringWithoutPathExpansion(::onc::proxy::kHost, &host);
+  // Parse |host| according to the format [<scheme>"://"]<server>[":"<port>].
+  net::ProxyServer proxy_server =
+      net::ProxyServer::FromURI(host, default_proxy_scheme);
+  int port = 0;
+  onc_proxy_location.GetIntegerWithoutPathExpansion(::onc::proxy::kPort, &port);
+
+  // Replace the port parsed from |host| by the provided |port|.
+  return net::ProxyServer(
+      proxy_server.scheme(),
+      net::HostPortPair(proxy_server.host_port_pair().host(),
+                        static_cast<uint16>(port)));
+}
+
+void AppendProxyServerForScheme(const base::DictionaryValue& onc_manual,
+                                const std::string& onc_scheme,
+                                std::string* spec) {
+  const base::DictionaryValue* onc_proxy_location = nullptr;
+  if (!onc_manual.GetDictionaryWithoutPathExpansion(onc_scheme,
+                                                    &onc_proxy_location)) {
+    return;
+  }
+
+  net::ProxyServer::Scheme default_proxy_scheme = net::ProxyServer::SCHEME_HTTP;
+  std::string url_scheme;
+  if (onc_scheme == ::onc::proxy::kFtp) {
+    url_scheme = kSchemeFtp;
+  } else if (onc_scheme == ::onc::proxy::kHttp) {
+    url_scheme = kSchemeHttp;
+  } else if (onc_scheme == ::onc::proxy::kHttps) {
+    url_scheme = kSchemeHttps;
+  } else if (onc_scheme == ::onc::proxy::kSocks) {
+    default_proxy_scheme = net::ProxyServer::SCHEME_SOCKS4;
+    url_scheme = kSchemeSocks;
+  } else {
+    NOTREACHED();
+  }
+
+  net::ProxyServer proxy_server = ConvertOncProxyLocationToHostPort(
+      default_proxy_scheme, *onc_proxy_location);
+
+  ProxyConfigDictionary::EncodeAndAppendProxyServer(url_scheme, proxy_server,
+                                                    spec);
+}
+
+net::ProxyBypassRules ConvertOncExcludeDomainsToBypassRules(
+    const base::ListValue& onc_exclude_domains) {
+  net::ProxyBypassRules rules;
+  for (base::ListValue::const_iterator it = onc_exclude_domains.begin();
+       it != onc_exclude_domains.end(); ++it) {
+    std::string rule;
+    (*it)->GetAsString(&rule);
+    rules.AddRuleFromString(rule);
+  }
+  return rules;
+}
+
+}  // namespace
+
+scoped_ptr<base::DictionaryValue> ConvertOncProxySettingsToProxyConfig(
+    const base::DictionaryValue& onc_proxy_settings) {
+  std::string type;
+  onc_proxy_settings.GetStringWithoutPathExpansion(::onc::proxy::kType, &type);
+  scoped_ptr<base::DictionaryValue> proxy_dict;
+
+  if (type == ::onc::proxy::kDirect) {
+    proxy_dict.reset(ProxyConfigDictionary::CreateDirect());
+  } else if (type == ::onc::proxy::kWPAD) {
+    proxy_dict.reset(ProxyConfigDictionary::CreateAutoDetect());
+  } else if (type == ::onc::proxy::kPAC) {
+    std::string pac_url;
+    onc_proxy_settings.GetStringWithoutPathExpansion(::onc::proxy::kPAC,
+                                                     &pac_url);
+    GURL url(pac_url);
+    DCHECK(url.is_valid())
+        << "PAC field is invalid for this ProxySettings.Type";

[pneubeck (no reviews), 2015/07/07 10:18:47]

i think this error message is wrong.
should instead be:
  Invalid URL in ProxySettings.PAC

(although this part should better be verified in the validator)

[stevenjb, 2015/07/07 18:30:36]

Done.

+    proxy_dict.reset(ProxyConfigDictionary::CreatePacScript(url.spec(), false));
+  } else if (type == ::onc::proxy::kManual) {
+    const base::DictionaryValue* manual_dict = nullptr;
+    onc_proxy_settings.GetDictionaryWithoutPathExpansion(::onc::proxy::kManual,
+                                                         &manual_dict);
+    std::string manual_spec;
+    AppendProxyServerForScheme(*manual_dict, ::onc::proxy::kFtp, &manual_spec);
+    AppendProxyServerForScheme(*manual_dict, ::onc::proxy::kHttp, &manual_spec);
+    AppendProxyServerForScheme(*manual_dict, ::onc::proxy::kSocks,
+                               &manual_spec);
+    AppendProxyServerForScheme(*manual_dict, ::onc::proxy::kHttps,
+                               &manual_spec);
+
+    const base::ListValue* exclude_domains = nullptr;
+    net::ProxyBypassRules bypass_rules;
+    if (onc_proxy_settings.GetListWithoutPathExpansion(
+            ::onc::proxy::kExcludeDomains, &exclude_domains)) {
+      bypass_rules.AssignFrom(
+          ConvertOncExcludeDomainsToBypassRules(*exclude_domains));
+    }
+    proxy_dict.reset(ProxyConfigDictionary::CreateFixedServers(
+        manual_spec, bypass_rules.ToString()));
+  } else {
+    NOTREACHED();
+  }
+  return proxy_dict.Pass();
+}
+
+scoped_ptr<base::DictionaryValue> ConvertProxyConfigToOncProxySettings(
+    const base::DictionaryValue& proxy_config_value) {
+  // Create a ProxyConfigDictionary from the DictionaryValye.

[pneubeck (no reviews), 2015/07/07 10:18:47]

Valye -> Value

[stevenjb, 2015/07/07 18:30:36]

Done.

+  scoped_ptr<ProxyConfigDictionary> proxy_config(
+      new ProxyConfigDictionary(&proxy_config_value));
+
+  // Create the result DictionaryValue and populate it.
+  scoped_ptr<base::DictionaryValue> proxy_settings(new base::DictionaryValue);
+  ProxyPrefs::ProxyMode mode;
+  if (!proxy_config->GetMode(&mode))
+    return nullptr;
+  switch (mode) {
+    case ProxyPrefs::MODE_DIRECT: {
+      proxy_settings->SetStringWithoutPathExpansion(::onc::proxy::kType,
+                                                    ::onc::proxy::kDirect);
+      break;
+    }
+    case ProxyPrefs::MODE_AUTO_DETECT: {
+      proxy_settings->SetStringWithoutPathExpansion(::onc::proxy::kType,
+                                                    ::onc::proxy::kWPAD);
+      break;
+    }
+    case ProxyPrefs::MODE_PAC_SCRIPT: {
+      proxy_settings->SetStringWithoutPathExpansion(::onc::proxy::kType,
+                                                    ::onc::proxy::kPAC);
+      std::string pac_url;
+      proxy_config->GetPacUrl(&pac_url);
+      proxy_settings->SetStringWithoutPathExpansion(::onc::proxy::kPAC,
+                                                    pac_url);
+      break;
+    }
+    case ProxyPrefs::MODE_FIXED_SERVERS: {
+      proxy_settings->SetString(::onc::proxy::kType, ::onc::proxy::kManual);
+      // Convert the 'server' string into dictionary entries.
+      std::string servers_string;
+      proxy_config->GetProxyServer(&servers_string);
+      std::vector<std::string> servers;

[pneubeck (no reviews), 2015/07/07 10:18:47]

instead of the following parsing you could instead use
  net::ProxyConfig::ProxyRules::ParseFromString

But as I understand it, you don't have to parse the general grammar described
there
(https://code.google.com/p/chromium/codesearch#chromium/src/net/proxy/proxy_co...)
but instead you only have to reverse the above function
ConvertOncProxySettingsToProxyConfig.

You should emphasize that in the header, that this function is not expecting
arbitrary ProxyConfig dictionaries but only the ones created by
ConvertOncProxySettingsToProxyConfig.

[stevenjb, 2015/07/07 18:30:36]

I wasn't aware of ProxyRules, I think that is a better way to go in case we ever
set a proxy configuration in Shill through a non ONC path.

+      Tokenize(servers_string, ";", &servers);
+      if (!servers.empty()) {

[pneubeck (no reviews), 2015/07/07 10:18:48]

according to the ONC spec the Manual subdictionary is required if Type is
Manual.

please check whether it would be more correct to at least insert the empty
Manual dict.

[stevenjb, 2015/07/07 18:30:36]

Check with who? :P I will always add it.

+        scoped_ptr<base::DictionaryValue> manual(new base::DictionaryValue);
+        for (std::string server : servers) {

[pneubeck (no reviews), 2015/07/07 10:18:47]

const&

[stevenjb, 2015/07/07 18:30:36]

Acknowledged.

+          size_t index = server.find_first_of('=');

[pneubeck (no reviews), 2015/07/07 10:18:47]

optional nit: 'const size_t'

[stevenjb, 2015/07/07 18:30:36]

Acknowledged.

+          std::string scheme = server.substr(0, index);
+          std::string url = server.substr(index + 1);
+          std::string onc_scheme;
+          std::string default_prefix = scheme;

[pneubeck (no reviews), 2015/07/07 10:18:47]

the default prefix for everything but Socks is  net::ProxyServer::SCHEME_HTTP
and not for example FTP for FTP.

IIRC, the meaning of this is:
FTP, HTTP and HTTPS usually are transported over an http connection to the
proxy.
Whereas for socks, the socks protocol itself is used.

[stevenjb, 2015/07/07 18:30:36]

Acknowledged.

+          if (scheme == kSchemeFtp) {
+            onc_scheme = ::onc::proxy::kFtp;
+          } else if (scheme == kSchemeHttp) {
+            onc_scheme = ::onc::proxy::kHttp;
+          } else if (scheme == kSchemeHttps) {
+            onc_scheme = ::onc::proxy::kHttps;
+          } else if (scheme == kSchemeSocks) {
+            onc_scheme = ::onc::proxy::kSocks;
+            default_prefix = "socks4";
+          }
+          if (!onc_scheme.empty()) {

[pneubeck (no reviews), 2015/07/07 10:18:48]

onc_scheme.empty() seems to be a severe error that we at least should log.

[stevenjb, 2015/07/07 18:30:36]

Acknowledged.

+            // If the prefix matches the default prefix for the scheme, skip it.
+            default_prefix += "://";
+            if (url.substr(0, default_prefix.length()) == default_prefix)
+              url = url.substr(default_prefix.length());
+            index = url.find_last_of(':');
+            std::string host = url.substr(0, index);
+            std::string port_str = url.substr(index + 1);
+            int port = 0;
+            base::StringToInt(port_str, &port);
+            scoped_ptr<base::DictionaryValue> url_dict(
+                new base::DictionaryValue);
+            url_dict->SetStringWithoutPathExpansion(::onc::proxy::kHost, host);
+            url_dict->SetIntegerWithoutPathExpansion(::onc::proxy::kPort, port);
+            manual->SetWithoutPathExpansion(onc_scheme, url_dict.release());
+          }
+        }
+        proxy_settings->SetWithoutPathExpansion(::onc::proxy::kManual,
+                                                manual.release());
+      }
+
+      // Convert the 'bypass_list' string into dictionary entries.
+      std::string bypass_list_string;
+      proxy_config->GetBypassList(&bypass_list_string);
+      std::vector<std::string> bypass_list;
+      Tokenize(bypass_list_string, ";", &bypass_list);
+      scoped_ptr<base::ListValue> exclude_domains(new base::ListValue);
+      for (std::string entry : bypass_list)

[pneubeck (no reviews), 2015/07/07 10:18:47]

const&

[stevenjb, 2015/07/07 18:30:36]

Using ProxyBypassRules now.

+        exclude_domains->AppendString(entry);
+      if (!exclude_domains->empty()) {
+        proxy_settings->SetWithoutPathExpansion(::onc::proxy::kExcludeDomains,
+                                                exclude_domains.release());
+      }
+      break;
+    }
+    default: {
+      LOG(ERROR) << "Unexpected proxy mode in Shill config: " << mode;
+      return nullptr;
+    }
+  }
+  return proxy_settings.Pass();
+}
+
 }  // namespace onc
 }  // namespace chromeos