Address a TODO for properly stripping references from request URL.

(rfind of # isn't quite right, as reference might contain hashes).

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

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

4 files changed, 65 insertions, 10 deletions

diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 58af549..24cd33f 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -141,16 +141,11 @@ HttpNetworkTransaction::~HttpNetworkTransaction() {
 }
 
 void HttpNetworkTransaction::BuildRequestHeaders() {
-  std::string path;
-  if (using_proxy_) {
-    // TODO(darin): GURL should have a method for this.
-    path = request_->url.spec();
-    size_t ref_pos = path.rfind('#');
-    if (ref_pos != std::string::npos)
-      path.erase(ref_pos);
-  } else {
-    path = request_->url.PathForRequest();
-  }
+  // For proxy use the full url. Otherwise just the absolute path.
+  // This strips out any reference/username/password.
+  std::string path = using_proxy_ ?
+      HttpUtil::SpecForRequest(request_->url) :
+      HttpUtil::PathForRequest(request_->url);
 
   request_headers_ = request_->method + " " + path +
       " HTTP/1.1\r\nHost: " + request_->url.host();
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index e67ea32..e204e67 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -53,6 +53,24 @@ static size_t FindStringEnd(const string& line, size_t start, char delim) {
 //-----------------------------------------------------------------------------
 
 // static
+std::string HttpUtil::PathForRequest(const GURL& url) {
+  DCHECK(url.is_valid() && (url.SchemeIs("http") || url.SchemeIs("https")));
+  if (url.has_query())
+    return url.path() + "?" + url.query();
+  return url.path(); 
+}
+
+// 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();
+}
+
+// static
 size_t HttpUtil::FindDelimiter(const string& line, size_t search_start,
                                char delimiter) {
   do {
diff --git a/net/http/http_util.h b/net/http/http_util.h
index 70488c3..5262070 100644
--- a/net/http/http_util.h
+++ b/net/http/http_util.h
@@ -6,6 +6,7 @@
 #define NET_HTTP_HTTP_UTIL_H_
 
 #include "base/string_tokenizer.h"
+#include "googleurl/src/gurl.h"
 
 // This is a macro to support extending this string literal at compile time.
 // Please excuse me polluting your global namespace!
@@ -15,6 +16,15 @@ namespace net {
 
 class HttpUtil {
  public:
+   // Returns the absolute path of the URL, to be used for the http request.
+   // The absolute path starts with a '/' and may contain a query.
+   static std::string PathForRequest(const GURL& url);
+
+   // Returns the absolute URL, to be used for the http request. This url is
+   // made up of the protocol, host, [port], path, [query]. Everything else
+   // is stripped (username, password, reference).
+   static std::string SpecForRequest(const GURL& url);
+
   // Locates the next occurance of delimiter in line, skipping over quoted
   // strings (e.g., commas will not be treated as delimiters if they appear
   // within a quoted string).  Returns the offset of the found delimiter or
diff --git a/net/http/http_util_unittest.cc b/net/http/http_util_unittest.cc
index 14c7724..a531228 100644
--- a/net/http/http_util_unittest.cc
+++ b/net/http/http_util_unittest.cc
@@ -412,3 +412,35 @@ TEST(HttpUtilTest, AssembleRawHeaders) {
   }
 }
 
+// Test SpecForRequest() and PathForRequest().
+TEST(HttpUtilTest, RequestUrlSanitize) {
+  struct {
+    const char* url;
+    const char* expected_spec;
+    const char* expected_path;
+  } tests[] = {
+    { // Check that #hash is removed.
+      "http://www.google.com:78/foobar?query=1#hash",
+      "http://www.google.com:78/foobar?query=1",
+      "/foobar?query=1"
+    },
+    { // The reference may itself contain # -- strip all of it.
+      "http://192.168.0.1?query=1#hash#10#11#13#14",
+      "http://192.168.0.1/?query=1",
+      "/?query=1"
+    },
+    { // Strip username/password.
+      "http://user:pass@google.com",
+      "http://google.com/",
+      "/"
+    }
+  };
+  for (size_t i = 0; i < arraysize(tests); ++i) {
+    GURL url(GURL(tests[i].url));
+    std::string expected_spec(tests[i].expected_spec);
+    std::string expected_path(tests[i].expected_path);
+
+    EXPECT_EQ(expected_spec, HttpUtil::SpecForRequest(url));
+    EXPECT_EQ(expected_path, HttpUtil::PathForRequest(url));
+  }
+}