Sanitize https:// URLs before sending them to PAC scripts.

net/proxy/proxy_service_unittest.cc

Index: net/proxy/proxy_service_unittest.cc
diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc
index a0db685aaba34234fca5ac5a7afcf567a214b7eb..ff4efdcc03d3f29e884c42e931411d039feb1bdd 100644
--- a/net/proxy/proxy_service_unittest.cc
+++ b/net/proxy/proxy_service_unittest.cc
@@ -3480,4 +3480,213 @@ TEST_F(ProxyServiceTest, SynchronousWithFixedConfiguration) {
   EXPECT_EQ(0u, factory->pending_requests().size());
 }
 
+// This test checks that URLs are sanitized before being passed on to the proxy
+// resolver (i.e. PAC scripts).
+TEST_F(ProxyServiceTest, SanitizeUrlsBeforeResolving) {
+  MockProxyConfigService* config_service =
+      new MockProxyConfigService("http://foopy/proxy.pac");
+  MockAsyncProxyResolver resolver;
+  MockAsyncProxyResolverFactory* factory =
+      new MockAsyncProxyResolverFactory(false);
+
+  ProxyService service(base::WrapUnique(config_service),
+                       base::WrapUnique(factory), nullptr);
+
+  // Request 1 ----------------------------------------------------------------
+
+  // This URL includes a lot of different parts (some of these are expected to
+  // be stripped).
+  GURL url1("http://username:password@www.google.com:8080/path?query#fragment");
+  GURL sanitized_url1("http://www.google.com:8080/path?query");
+
+  ProxyInfo info1;
+  TestCompletionCallback callback1;
+  int rv = service.ResolveProxy(url1, std::string(), LOAD_NORMAL, &info1,
+                                callback1.callback(), nullptr, nullptr,
+                                BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  // First step is to download the PAC script.
+  EXPECT_EQ(GURL("http://foopy/proxy.pac"),
+            factory->pending_requests()[0]->script_data()->url());
+  factory->pending_requests()[0]->CompleteNowWithForwarder(OK, &resolver);
+
+  ASSERT_EQ(1u, resolver.pending_requests().size());
+  // Ensure that the sanitized URL was sent to the proxy resolver (not the full
+  // URL).
+  EXPECT_EQ(sanitized_url1, resolver.pending_requests()[0]->url());
+
+  // Complete the request.
+  resolver.pending_requests()[0]->results()->UsePacString("DIRECT");
+  resolver.pending_requests()[0]->CompleteNow(OK);
+  EXPECT_EQ(OK, callback1.WaitForResult());
+  EXPECT_TRUE(info1.is_direct());
+
+  // Request 2 ----------------------------------------------------------------
+
+  // Start another request. This time the URL is https://, so a different
+  // sanitization strategy should be used.
+  GURL url2(
+      "https://username:password@www.google.com:8080/path?query#fragment");
+  GURL sanitized_url2("https://www.google.com:8080/");
+
+  ProxyInfo info2;
+  TestCompletionCallback callback2;
+  rv = service.ResolveProxy(url2, std::string(), LOAD_NORMAL, &info2,
+                            callback2.callback(), nullptr, nullptr,
+                            BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  ASSERT_EQ(1u, resolver.pending_requests().size());
+  // Ensure that the sanitized URL was sent to the proxy resolver (not the full
+  // URL).
+  EXPECT_EQ(sanitized_url2, resolver.pending_requests()[0]->url());
+
+  // Complete the request.
+  resolver.pending_requests()[0]->results()->UsePacString("DIRECT");
+  resolver.pending_requests()[0]->CompleteNow(OK);
+  EXPECT_EQ(OK, callback2.WaitForResult());
+  EXPECT_TRUE(info2.is_direct());
+
+  // Request 3 ----------------------------------------------------------------
+
+  // Change the sanitization policy.
+  service.set_sanitize_url_for_pac_script_policy(
+      SanitizeUrlForPacScriptPolicy::UNSAFE);
+
+  // Start another request. This is the same as request 2, however the expected
+  // sanitized URL is different (includes the path and query).
+  GURL url3 = url2;
+  GURL sanitized_url3("https://www.google.com:8080/path?query");
+
+  ProxyInfo info3;
+  TestCompletionCallback callback3;
+  rv = service.ResolveProxy(url3, std::string(), LOAD_NORMAL, &info3,
+                            callback3.callback(), nullptr, nullptr,
+                            BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  ASSERT_EQ(1u, resolver.pending_requests().size());
+  // Ensure that the sanitized URL was sent to the proxy resolver (not the full
+  // URL).
+  EXPECT_EQ(sanitized_url3, resolver.pending_requests()[0]->url());
+
+  // Complete the request.
+  resolver.pending_requests()[0]->results()->UsePacString("DIRECT");
+  resolver.pending_requests()[0]->CompleteNow(OK);
+  EXPECT_EQ(OK, callback3.WaitForResult());
+  EXPECT_TRUE(info3.is_direct());
+}
+
+// Tests SanitizeUrlForPacScript() using input URLs that have a
+// non-cryptographic scheme (i.e. http://). The sanitized result is consistent
+// regardless of the stripping mode selected.
+TEST_F(ProxyServiceTest, SanitizeUrlForPacScriptNonCryptographic) {
+  const struct {
+    const char* raw_url;
+    const char* sanitized_url;
+  } kTests[] = {
+      // Embedded identity is stripped.
+      {
+          "http://foo:bar@example.com/", "http://example.com/",
+      },
+      {
+          "ftp://foo:bar@example.com/", "ftp://example.com/",
+      },
+      // Reference fragment is stripped.
+      {
+          "http://example.com/blah#hello", "http://example.com/blah",
+      },
+      // Query parameters are NOT stripped.
+      {
+          "http://example.com/foo/bar/baz?hello",
+          "http://example.com/foo/bar/baz?hello",
+      },
+      // Fragment is stripped, but path and query are left intact.
+      {
+          "http://foo:bar@example.com/foo/bar/baz?hello#sigh",
+          "http://example.com/foo/bar/baz?hello",
+      },
+      // Port numbers are not affected.
+      {
+          "http://example.com:88/hi", "http://example.com:88/hi",
+      },
+  };
+
+  for (const auto& test : kTests) {
+    // The result of SanitizeUrlForPacScript() is the same regardless of the
+    // second parameter (sanitization mode), since the input URLs do not use a
+    // cryptographic scheme.
+    GURL raw_url(test.raw_url);
+    ASSERT_TRUE(raw_url.is_valid());
+    EXPECT_FALSE(raw_url.SchemeIsCryptographic());
+
+    EXPECT_EQ(GURL(test.sanitized_url),
+              SanitizeUrlForPacScript(raw_url,
+                                      SanitizeUrlForPacScriptPolicy::UNSAFE));
+
+    EXPECT_EQ(
+        GURL(test.sanitized_url),
+        SanitizeUrlForPacScript(raw_url, SanitizeUrlForPacScriptPolicy::SAFE));
+  }
+}
+
+// Tests SanitizeUrlForPacScript() using input URLs that have a cryptographic
+// schemes (i.e. https://). The sanitized result differs depending
+// on the sanitization mode chosen.
+TEST_F(ProxyServiceTest, SanitizeUrlForPacScriptCryptographic) {
+  const struct {
+    // Input URL.
+    const char* raw_url;
+
+    // Output URL when stripping of cryptographic URLs is disabled.
+    const char* sanitized_url_unstripped;
+
+    // Output URL when stripping of cryptographic URLs is enabled.
+    const char* sanitized_url;
+  } kTests[] = {
+      // Embedded identity is always stripped.
+      {
+          "https://foo:bar@example.com/", "https://example.com/",
+          "https://example.com/",
+      },
+      // Fragments are always stripped, but stripping path is conditional on the
+      // mode.
+      {
+          "https://example.com/blah#hello", "https://example.com/blah",
+          "https://example.com/",
+      },
+      // Stripping the query is conditional on the mode.
+      {
+          "https://example.com/foo/bar/baz?hello",
+          "https://example.com/foo/bar/baz?hello", "https://example.com/",
+      },
+      // The embedded identity and fragment is always stripped, however path and
+      // query are conditional on the stripping mode.
+      {
+          "https://foo:bar@example.com/foo/bar/baz?hello#sigh",
+          "https://example.com/foo/bar/baz?hello", "https://example.com/",
+      },
+      // The URL's port should not be stripped.
+      {
+          "https://example.com:88/hi", "https://example.com:88/hi",
+          "https://example.com:88/",
+      },
+  };
+
+  for (const auto& test : kTests) {
+    GURL raw_url(test.raw_url);
+    ASSERT_TRUE(raw_url.is_valid());
+    EXPECT_TRUE(raw_url.SchemeIsCryptographic());
+
+    EXPECT_EQ(GURL(test.sanitized_url_unstripped),
+              SanitizeUrlForPacScript(raw_url,
+                                      SanitizeUrlForPacScriptPolicy::UNSAFE));
+
+    EXPECT_EQ(
+        GURL(test.sanitized_url),
+        SanitizeUrlForPacScript(raw_url, SanitizeUrlForPacScriptPolicy::SAFE));
+  }
+}
+
 }  // namespace net