Split out HttpUtil::SpecForRequest() into a more generic function of net_util.h.

This was a TODO, since that function is useful outside of HTTP.

In the process, I uncovered some test cases in proxy_service that are passing in invalid URLs (by virtue of the extra DCHECK). This doesn't make much sense to me to support that, so I have changed them.

Review URL: http://codereview.chromium.org/160558

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22359 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 94 insertions, 21 deletions

diff --git a/net/base/net_util.cc b/net/base/net_util.cc
index 7afd4c5..492f7a8 100644
--- a/net/base/net_util.cc
+++ b/net/base/net_util.cc
@@ -1297,4 +1297,13 @@ std::wstring FormatUrl(const GURL& url,
   return url_string;
 }
 
+GURL SimplifyUrlForRequest(const GURL& url) {
+  DCHECK(url.is_valid());
+  GURL::Replacements replacements;
+  replacements.ClearUsername();
+  replacements.ClearPassword();
+  replacements.ClearRef();
+  return url.ReplaceComponents(replacements);
+}
+
 }  // namespace net
diff --git a/net/base/net_util.h b/net/base/net_util.h
index 4320e1c..0e247b8 100644
--- a/net/base/net_util.h
+++ b/net/base/net_util.h
@@ -225,6 +225,11 @@ inline std::wstring FormatUrl(const GURL& url, const std::wstring& languages) {
   return FormatUrl(url, languages, true, UnescapeRule::SPACES, NULL, NULL);
 }
 
+// Strip the portions of |url| that aren't core to the network request.
+//   - user name / password
+//   - reference section
+GURL SimplifyUrlForRequest(const GURL& url);
+
 }  // namespace net
 
 #endif  // NET_BASE_NET_UTIL_H__
diff --git a/net/base/net_util_unittest.cc b/net/base/net_util_unittest.cc
index feb4be7..726529e 100644
--- a/net/base/net_util_unittest.cc
+++ b/net/base/net_util_unittest.cc
@@ -1269,3 +1269,47 @@ TEST(NetUtilTest, FormatUrlParsed) {
   EXPECT_EQ(L"query", formatted.substr(parsed.query.begin, parsed.query.len));
   EXPECT_EQ(L"ref", formatted.substr(parsed.ref.begin, parsed.ref.len));
 }
+
+TEST(NetUtilTest, SimplifyUrlForRequest) {
+  struct {
+    const char* input_url;
+    const char* expected_simplified_url;
+  } tests[] = {
+    {
+      // Reference section should be stripped.
+      "http://www.google.com:78/foobar?query=1#hash",
+      "http://www.google.com:78/foobar?query=1",
+    },
+    {
+      // Reference section can itself contain #.
+      "http://192.168.0.1?query=1#hash#10#11#13#14",
+      "http://192.168.0.1?query=1",
+    },
+    { // Strip username/password.
+      "http://user:pass@google.com",
+      "http://google.com/",
+    },
+    { // Strip both the reference and the username/password.
+      "http://user:pass@google.com:80/sup?yo#X#X",
+      "http://google.com/sup?yo",
+    },
+    { // Try an HTTPS URL -- strip both the reference and the username/password.
+      "https://user:pass@google.com:80/sup?yo#X#X",
+      "https://google.com:80/sup?yo",
+    },
+    { // Try an FTP URL -- strip both the reference and the username/password.
+      "ftp://user:pass@google.com:80/sup?yo#X#X",
+      "ftp://google.com:80/sup?yo",
+    },
+    { // Try an standard URL with unknow scheme.
+      "foobar://user:pass@google.com:80/sup?yo#X#X",
+      "foobar://google.com:80/sup?yo",
+    },
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
+    SCOPED_TRACE(StringPrintf("Test[%d]: %s", i, tests[i].input_url));
+    GURL input_url(GURL(tests[i].input_url));
+    GURL expected_url(GURL(tests[i].expected_simplified_url));
+    EXPECT_EQ(expected_url, net::SimplifyUrlForRequest(input_url));
+  }
+}
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index 5e81dab..4381117 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -12,6 +12,7 @@
 #include "base/logging.h"
 #include "base/string_piece.h"
 #include "base/string_util.h"
+#include "net/base/net_util.h"
 
 using std::string;
 
@@ -63,11 +64,7 @@ std::string HttpUtil::PathForRequest(const GURL& url) {
 // static
 std::string HttpUtil::SpecForRequest(const GURL& url) {
   DCHECK(url.is_valid() && (url.SchemeIs("http") || url.SchemeIs("https")));
-  GURL::Replacements replacements;
-  replacements.ClearUsername();
-  replacements.ClearPassword();
-  replacements.ClearRef();
-  return url.ReplaceComponents(replacements).spec();
+  return SimplifyUrlForRequest(url).spec();
 }
 
 // static
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index cdf269c..ab18956 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -12,6 +12,7 @@
 #include "base/string_util.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_errors.h"
+#include "net/base/net_util.h"
 #include "net/proxy/proxy_config_service_fixed.h"
 #include "net/proxy/proxy_script_fetcher.h"
 #if defined(OS_WIN)
@@ -63,18 +64,6 @@ class ProxyResolverNull : public ProxyResolver {
   virtual void SetPacScriptByUrlInternal(const GURL& pac_url) {}
 };
 
-// Strip away any reference fragments and the username/password, as they
-// are not relevant to proxy resolution.
-static GURL SanitizeURLForProxyResolver(const GURL& url) {
-  // TODO(eroman): The following duplicates logic from
-  // HttpUtil::SpecForRequest. Should probably live in net_util.h
-  GURL::Replacements replacements;
-  replacements.ClearUsername();
-  replacements.ClearPassword();
-  replacements.ClearRef();
-  return url.ReplaceComponents(replacements);
-}
-
 // ProxyService::PacRequest ---------------------------------------------------
 
 class ProxyService::PacRequest
@@ -252,7 +241,9 @@ int ProxyService::ResolveProxy(const GURL& raw_url, ProxyInfo* result,
                                PacRequest** pac_request) {
   DCHECK(callback);
 
-  GURL url = SanitizeURLForProxyResolver(raw_url);
+  // Strip away any reference fragments and the username/password, as they
+  // are not relevant to proxy resolution.
+  GURL url = SimplifyUrlForRequest(raw_url);
 
   // Check if the request can be completed right away. This is the case when
   // using a direct connection, or when the config is bad.
diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc
index f13df7c..b6b62b6 100644
--- a/net/proxy/proxy_service_unittest.cc
+++ b/net/proxy/proxy_service_unittest.cc
@@ -236,6 +236,33 @@ TEST(ProxyServiceTest, PAC) {
   EXPECT_EQ("foopy:80", info.proxy_server().ToURI());
 }
 
+// Test that the proxy resolver does not see the URL's username/password
+// or its reference section.
+TEST(ProxyServiceTest, PAC_NoIdentityOrHash) {
+  MockProxyConfigService* config_service =
+      new MockProxyConfigService("http://foopy/proxy.pac");
+
+  MockAsyncProxyResolver* resolver = new MockAsyncProxyResolver;
+
+  ProxyService service(config_service, resolver);
+
+  GURL url("http://username:password@www.google.com/?ref#hash#hash");
+
+  ProxyInfo info;
+  TestCompletionCallback callback;
+  int rv = service.ResolveProxy(url, &info, &callback, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  EXPECT_EQ(GURL("http://foopy/proxy.pac"), resolver->pac_url());
+  ASSERT_EQ(1u, resolver->pending_requests().size());
+  // The URL should have been simplified, stripping the username/password/hash.
+  EXPECT_EQ(GURL("http://www.google.com/?ref"),
+                 resolver->pending_requests()[0]->url());
+
+  // We end here without ever completing the request -- destruction of
+  // ProxyService will cancel the outstanding request.
+}
+
 TEST(ProxyServiceTest, PAC_FailoverToDirect) {
   MockProxyConfigService* config_service =
       new MockProxyConfigService("http://foopy/proxy.pac");
@@ -589,7 +616,7 @@ TEST(ProxyServiceTest, ProxyBypassList) {
   {
     ProxyService service(new MockProxyConfigService(config),
                               new MockAsyncProxyResolver());
-    GURL test_url("local");
+    GURL test_url("http://local");
     TestCompletionCallback callback;
     int rv = service.ResolveProxy(test_url, &info, &callback, NULL);
     EXPECT_EQ(OK, rv);
@@ -811,7 +838,7 @@ TEST(ProxyServiceTest, PerProtocolProxyTests) {
     config.proxy_rules.ParseFromString("foopy1:8080");
     ProxyService service(new MockProxyConfigService(config),
                               new MockAsyncProxyResolver);
-    GURL test_url("www.microsoft.com");
+    GURL test_url("http://www.microsoft.com");
     ProxyInfo info;
     TestCompletionCallback callback;
     int rv = service.ResolveProxy(test_url, &info, &callback, NULL);
@@ -866,7 +893,7 @@ TEST(ProxyServiceTest, DefaultProxyFallbackToSOCKS) {
   {
     ProxyService service(new MockProxyConfigService(config),
                               new MockAsyncProxyResolver);
-    GURL test_url("www.microsoft.com");
+    GURL test_url("unknown://www.microsoft.com");
     ProxyInfo info;
     TestCompletionCallback callback;
     int rv = service.ResolveProxy(test_url, &info, &callback, NULL);