Allow hostnames and host:port strings for proxy_server_strings

Bug: 1512190
Change-Id: I6b56dc232492db691888d4cba8e3c14c88cebfeb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5131718
Reviewed-by: David Schinazi <dschinazi@chromium.org>
Commit-Queue: Ciara McMullin <ciaramcmullin@google.com>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Dustin Mitchell <djmitche@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1239142}

net/base/proxy_string_util.cc

@@ -45,49 +45,6 @@ ProxyServer::Scheme GetSchemeFromPacTypeInternal(std::string_view type) {
   return ProxyServer::SCHEME_INVALID;
 }
 
-ProxyServer FromSchemeHostAndPort(ProxyServer::Scheme scheme,
-                                  std::string_view host_and_port) {
-  // Trim leading/trailing space.
-  host_and_port = HttpUtil::TrimLWS(host_and_port);
-
-  if (scheme == ProxyServer::SCHEME_INVALID)
-    return ProxyServer();
-
-  if (scheme == ProxyServer::SCHEME_DIRECT) {
-    if (!host_and_port.empty())
-      return ProxyServer();  // Invalid -- DIRECT cannot have a host/port.
-    return ProxyServer::Direct();
-  }
-
-  url::Component username_component;
-  url::Component password_component;
-  url::Component hostname_component;
-  url::Component port_component;
-  url::ParseAuthority(host_and_port.data(),
-                      url::Component(0, host_and_port.size()),
-                      &username_component, &password_component,
-                      &hostname_component, &port_component);
-  if (username_component.is_valid() || password_component.is_valid() ||
-      hostname_component.is_empty()) {
-    return ProxyServer();
-  }
-
-  std::string_view hostname =
-      host_and_port.substr(hostname_component.begin, hostname_component.len);
-
-  // Reject inputs like "foo:". /url parsing and canonicalization code generally
-  // allows it and treats it the same as a URL without a specified port, but
-  // Chrome has traditionally disallowed it in proxy specifications.
-  if (port_component.is_valid() && port_component.is_empty())
-    return ProxyServer();
-  std::string_view port =
-      port_component.is_nonempty()
-          ? host_and_port.substr(port_component.begin, port_component.len)
-          : "";
-
-  return ProxyServer::FromSchemeHostAndPort(scheme, hostname, port);
-}
-
 std::string ConstructHostPortString(std::string_view hostname, uint16_t port) {
   DCHECK(!hostname.empty());
   DCHECK((hostname.front() == '[' && hostname.back() == ']') ||
@@ -125,7 +82,8 @@ ProxyServer PacResultElementToProxyServer(std::string_view pac_result_element) {
 
   // And everything to the right of the space is the
   // <host>[":" <port>].
-  return FromSchemeHostAndPort(scheme, pac_result_element.substr(space));
+  return ProxySchemeHostAndPortToProxyServer(scheme,
+                                             pac_result_element.substr(space));
 }
 
 std::string ProxyServerToPacResultElement(const ProxyServer& proxy_server) {
@@ -182,7 +140,7 @@ ProxyServer ProxyUriToProxyServer(std::string_view uri,
   }
 
   // Now parse the <host>[":"<port>].
-  return FromSchemeHostAndPort(scheme, uri);
+  return ProxySchemeHostAndPortToProxyServer(scheme, uri);
 }
 
 std::string ProxyServerToProxyUri(const ProxyServer& proxy_server) {
@@ -216,6 +174,53 @@ std::string ProxyServerToProxyUri(const ProxyServer& proxy_server) {
   }
 }
 
+ProxyServer ProxySchemeHostAndPortToProxyServer(
+    ProxyServer::Scheme scheme,
+    std::string_view host_and_port) {
+  // Trim leading/trailing space.
+  host_and_port = HttpUtil::TrimLWS(host_and_port);
+
+  if (scheme == ProxyServer::SCHEME_INVALID) {
+    return ProxyServer();
+  }
+
+  if (scheme == ProxyServer::SCHEME_DIRECT) {
+    if (!host_and_port.empty()) {
+      return ProxyServer();  // Invalid -- DIRECT cannot have a host/port.
+    }
+    return ProxyServer::Direct();
+  }
+
+  url::Component username_component;
+  url::Component password_component;
+  url::Component hostname_component;
+  url::Component port_component;
+  url::ParseAuthority(host_and_port.data(),
+                      url::Component(0, host_and_port.size()),
+                      &username_component, &password_component,
+                      &hostname_component, &port_component);
+  if (username_component.is_valid() || password_component.is_valid() ||
+      hostname_component.is_empty()) {
+    return ProxyServer();
+  }
+
+  std::string_view hostname =
+      host_and_port.substr(hostname_component.begin, hostname_component.len);
+
+  // Reject inputs like "foo:". /url parsing and canonicalization code generally
+  // allows it and treats it the same as a URL without a specified port, but
+  // Chrome has traditionally disallowed it in proxy specifications.
+  if (port_component.is_valid() && port_component.is_empty()) {
+    return ProxyServer();
+  }
+  std::string_view port =
+      port_component.is_nonempty()
+          ? host_and_port.substr(port_component.begin, port_component.len)
+          : "";
+
+  return ProxyServer::FromSchemeHostAndPort(scheme, hostname, port);
+}
+
 ProxyServer::Scheme GetSchemeFromUriScheme(std::string_view scheme) {
   if (base::EqualsCaseInsensitiveASCII(scheme, "http"))
     return ProxyServer::SCHEME_HTTP;

net/base/proxy_string_util.h

@@ -89,6 +89,9 @@ NET_EXPORT ProxyChain ProxyUriToProxyChain(std::string_view uri,
 NET_EXPORT ProxyServer
 ProxyUriToProxyServer(std::string_view uri, ProxyServer::Scheme default_scheme);
 NET_EXPORT std::string ProxyServerToProxyUri(const ProxyServer& proxy_server);
+NET_EXPORT ProxyServer
+ProxySchemeHostAndPortToProxyServer(ProxyServer::Scheme scheme,
+                                    std::string_view host_and_port);
 
 // Parses the proxy scheme from the non-standard URI scheme string
 // representation used in `ProxyUriToProxyServer()` and

services/network/ip_protection_config_cache_impl.cc

@@ -9,6 +9,7 @@
 #include "base/task/thread_pool.h"
 #include "base/time/time.h"
 #include "net/base/proxy_server.h"
+#include "net/base/proxy_string_util.h"
 #include "services/network/ip_protection_proxy_list_manager.h"
 #include "services/network/ip_protection_proxy_list_manager_impl.h"
 #include "services/network/ip_protection_token_cache_manager.h"
@@ -107,13 +108,12 @@ std::vector<net::ProxyChain>
 IpProtectionConfigCacheImpl::ConvertProxyServerStringsToProxyChainList(
     const std::vector<std::vector<std::string>>& proxy_server_strings) {
   std::vector<net::ProxyChain> proxy_chain_list;
-  for (const std::vector<std::string>& proxy_chain_hostnames :
-       proxy_server_strings) {
+  for (const std::vector<std::string>& proxy_chain : proxy_server_strings) {
     bool invalid_proxy_server = false;
     std::vector<net::ProxyServer> proxy_servers;
-    for (const auto& proxy : proxy_chain_hostnames) {
-      net::ProxyServer proxy_server = net::ProxyServer::FromSchemeHostAndPort(
-          net::ProxyServer::SCHEME_HTTPS, proxy, absl::nullopt);
+    for (const auto& proxy : proxy_chain) {
+      net::ProxyServer proxy_server = net::ProxySchemeHostAndPortToProxyServer(
+          net::ProxyServer::SCHEME_HTTPS, proxy);
       // If invalid proxy server, skip entire proxy chain.
       if (!proxy_server.is_valid()) {
         invalid_proxy_server = true;

services/network/ip_protection_config_cache_impl_unittest.cc

@@ -186,4 +186,43 @@ TEST_F(IpProtectionConfigCacheImplTest, GetProxyListFromManager) {
   EXPECT_EQ(ipp_config_cache_->GetProxyChainList(), proxy_chain_list);
 }
 
+// Token cache manager supports both hostnames and host:port for proxy servers
+// in list.
+TEST_F(IpProtectionConfigCacheImplTest, GetProxyChainList) {
+  const struct {
+    const net::ProxyServer::Scheme scheme;
+    const char* const host;
+    const std::optional<uint16_t> port;
+    const char* const proxy;
+  } tests[] = {
+      {net::ProxyServer::SCHEME_HTTPS, "a-proxy", 443, "a-proxy:443"},
+      {net::ProxyServer::SCHEME_HTTPS, "b-proxy", 443, "b-proxy:443"},
+
+      // No ports.
+      {net::ProxyServer::SCHEME_HTTPS, "a-proxy", absl::nullopt, "a-proxy"},
+      {net::ProxyServer::SCHEME_HTTPS, "b-proxy", absl::nullopt, "b-proxy"},
+
+      // Non-standard port.
+      {net::ProxyServer::SCHEME_HTTPS, "a-proxy", 10, "a-proxy:10"},
+      {net::ProxyServer::SCHEME_HTTPS, "b-proxy", 0, "b-proxy:0"},
+  };
+
+  for (size_t i = 0; i < std::size(tests); ++i) {
+    auto ip_protection_proxy_chain =
+        net::ProxyChain(net::ProxyServer::FromSchemeHostAndPort(
+                            tests[i].scheme, tests[i].host, tests[i].port))
+            .ForIpProtection();
+    const std::vector<net::ProxyChain> proxy_chain_list = {
+        std::move(ip_protection_proxy_chain)};
+    auto ipp_proxy_list_manager_ =
+        std::make_unique<MockIpProtectionProxyListManager>();
+    ipp_proxy_list_manager_->SetProxyList({{tests[i].proxy}});
+    ipp_config_cache_->SetIpProtectionProxyListManagerForTesting(
+        std::move(ipp_proxy_list_manager_));
+
+    ASSERT_TRUE(ipp_config_cache_->IsProxyListAvailable());
+    EXPECT_EQ(ipp_config_cache_->GetProxyChainList(), proxy_chain_list);
+  }
+}
+
 }  // namespace network

services/network/network_service_proxy_delegate_unittest.cc

@@ -334,7 +334,7 @@ TEST_F(NetworkServiceProxyDelegateTest,
 
   auto ipp_config_cache = std::make_unique<MockIpProtectionConfigCache>();
   ipp_config_cache->SetNextAuthToken(MakeAuthToken("Bearer: a-token"));
-  ipp_config_cache->SetProxyList({{"foo:80"}});
+  ipp_config_cache->SetProxyList({{"[foo]"}});
   delegate->SetIpProtectionConfigCache(std::move(ipp_config_cache));
 
   net::ProxyInfo result;