Add tests and documentation for when net::ProxyConfig::fallback_proxies is not...

Add tests and documentation for when net::ProxyConfig::fallback_proxies is not a SOCKS proxy, and which proxy WebSocket URLs map to.


Change-Id: I28bd80069fb3e9fa14ed2c3ea71aa1dc164d5f11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825724
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702070}

net/proxy_resolution/proxy_config.cc

@@ -177,6 +177,27 @@ ProxyList* ProxyConfig::ProxyRules::MapUrlSchemeToProxyListNoFallback(
 
 const ProxyList* ProxyConfig::ProxyRules::GetProxyListForWebSocketScheme()
     const {
+  // Follow the recommendation from RFC 6455 section 4.1.3:
+  //
+  //       NOTE: Implementations that do not expose explicit UI for
+  //       selecting a proxy for WebSocket connections separate from other
+  //       proxies are encouraged to use a SOCKS5 [RFC1928] proxy for
+  //       WebSocket connections, if available, or failing that, to prefer
+  //       the proxy configured for HTTPS connections over the proxy
+  //       configured for HTTP connections.
+  //
+  // This interpretation is a bit different from the RFC, in
+  // that it favors both SOCKSv4 and SOCKSv5.
+  //
+  // When the net::ProxyRules came from system proxy settings,
+  // "fallback_proxies" will be empty, or a a single SOCKS
+  // proxy, making this ordering match the RFC.
+  //
+  // However for other configurations it is possible for
+  // "fallback_proxies" to be a list of any ProxyServer,
+  // including non-SOCKS. In this case "fallback_proxies" is
+  // still prioritized over proxies_for_http and
+  // proxies_for_https.
   if (!fallback_proxies.IsEmpty())
     return &fallback_proxies;
   if (!proxies_for_https.IsEmpty())

net/proxy_resolution/proxy_config_unittest.cc

@@ -505,20 +505,53 @@ TEST_F(ProxyConfigWebSocketTest, UsesProxy) {
   EXPECT_EQ("PROXY proxy:3128", ToPacString());
 }
 
-// See RFC6455 Section 4.1. item 3, "_Proxy Usage_".
-TEST_F(ProxyConfigWebSocketTest, PrefersSocks) {
+// See RFC6455 Section 4.1. item 3, "_Proxy Usage_". Note that this favors a
+// SOCKSv4 proxy (although technically the spec only notes SOCKSv5).
+TEST_F(ProxyConfigWebSocketTest, PrefersSocksV4) {
   ParseFromString(
       "http=proxy:3128 ; https=sslproxy:3128 ; socks=socksproxy:1080");
   Apply(WsUrl());
   EXPECT_EQ("SOCKS socksproxy:1080", ToPacString());
 }
 
+// See RFC6455 Section 4.1. item 3, "_Proxy Usage_".
+TEST_F(ProxyConfigWebSocketTest, PrefersSocksV5) {
+  ParseFromString(
+      "http=proxy:3128 ; https=sslproxy:3128 ; socks=socks5://socksproxy:1080");
+  Apply(WsUrl());
+  EXPECT_EQ("SOCKS5 socksproxy:1080", ToPacString());
+}
+
 TEST_F(ProxyConfigWebSocketTest, PrefersHttpsToHttp) {
   ParseFromString("http=proxy:3128 ; https=sslproxy:3128");
   Apply(WssUrl());
   EXPECT_EQ("PROXY sslproxy:3128", ToPacString());
 }
 
+// Tests when a proxy-per-url-scheme configuration was used, and proxies are
+// specified for http://, https://, and a fallback proxy (non-SOCKS).
+// Even though the fallback proxy is not SOCKS, it is still favored over the
+// proxy for http://* and https://*.
+TEST_F(ProxyConfigWebSocketTest, PrefersNonSocksFallbackOverHttps) {
+  // The notation for "socks=" is abused to set the "fallback proxy".
+  ParseFromString(
+      "http=proxy:3128 ; https=sslproxy:3128; socks=https://httpsproxy");
+  EXPECT_EQ("HTTPS httpsproxy:443", rules_.fallback_proxies.ToPacString());
+  Apply(WssUrl());
+  EXPECT_EQ("HTTPS httpsproxy:443", ToPacString());
+}
+
+// Tests when a proxy-per-url-scheme configuration was used, and the fallback
+// proxy is a non-SOCKS proxy, and no proxy was given for https://* or
+// http://*. The fallback proxy is used.
+TEST_F(ProxyConfigWebSocketTest, UsesNonSocksFallbackProxy) {
+  // The notation for "socks=" is abused to set the "fallback proxy".
+  ParseFromString("ftp=ftpproxy:3128; socks=https://httpsproxy");
+  EXPECT_EQ("HTTPS httpsproxy:443", rules_.fallback_proxies.ToPacString());
+  Apply(WssUrl());
+  EXPECT_EQ("HTTPS httpsproxy:443", ToPacString());
+}
+
 TEST_F(ProxyConfigWebSocketTest, PrefersHttpsEvenForWs) {
   ParseFromString("http=proxy:3128 ; https=sslproxy:3128");
   Apply(WsUrl());