Don't send Content-Type when redirecting from a POST.

http://code.google.com/p/chromium/issues/detail?id=843

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

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

3 files changed, 100 insertions, 0 deletions

diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index 7e30b37..c22101c 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -9,10 +9,12 @@
 #include "base/process_util.h"
 #include "base/singleton.h"
 #include "base/stats_counters.h"
+#include "base/string_util.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/upload_data.h"
+#include "net/http/http_util.h"
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_job_manager.h"
 
@@ -294,6 +296,36 @@ void URLRequest::OrphanJob() {
   job_ = NULL;
 }
 
+// static
+std::string URLRequest::StripPostSpecificHeaders(const std::string& headers) {
+  // These are headers that may be attached to a POST.
+  static const char* const kPostHeaders[] = {
+      "content-type",
+      "content-length",
+      "origin"
+  };
+
+  std::string stripped_headers;
+  net::HttpUtil::HeadersIterator it(headers.begin(), headers.end(), "\r\n");
+  
+  while (it.GetNext()) {
+    bool is_post_specific = false;
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kPostHeaders); ++i) {
+      if (LowerCaseEqualsASCII(it.name_begin(), it.name_end(),
+                               kPostHeaders[i])) {
+        is_post_specific = true;
+        break;
+      }
+    }
+    if (!is_post_specific) {
+      // Assume that name and values are on the same line.
+      stripped_headers.append(it.name_begin(), it.values_end());
+      stripped_headers.append("\r\n");
+    }
+  }
+  return stripped_headers;
+}
+
 int URLRequest::Redirect(const GURL& location, int http_status_code) {
   // TODO(darin): treat 307 redirects of POST requests very carefully.  we
   // should prompt the user before re-submitting the POST body.
@@ -312,12 +344,25 @@ int URLRequest::Redirect(const GURL& location, int http_status_code) {
   // NOTE: even though RFC 2616 says to preserve the request method when
   // following a 302 redirect, normal browsers don't do that.  instead, they
   // all convert a POST into a GET in response to a 302, and so shall we.
+  bool was_post = method_ == "POST";
   url_ = location;
   method_ = "GET";
   upload_ = 0;
   status_ = URLRequestStatus();
   --redirect_limit_;
 
+  if (was_post) {
+    // If being switched from POST to GET, must remove headers that were
+    // specific to the POST and don't have meaning in GET. For example
+    // the inclusion of a multipart Content-Type header in GET can cause
+    // problems with some servers:
+    // http://code.google.com/p/chromium/issues/detail?id=843
+    //
+    // TODO(eroman): It would be better if this data was structured into
+    // specific fields/flags, rather than a stew of extra headers.
+    extra_request_headers_ = StripPostSpecificHeaders(extra_request_headers_);
+  }
+
   if (!final_upload_progress_) {
     final_upload_progress_ = job_->GetUploadProgress();
   }
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index 1f5a06e..b01aa9c 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -437,6 +437,10 @@ class URLRequest {
   // been orphaned.
   void OrphanJob();
 
+  // Discard headers which have meaning in POST (Content-Length, Content-Type,
+  // Origin).
+  static std::string StripPostSpecificHeaders(const std::string& headers);
+
   scoped_refptr<URLRequestJob> job_;
   scoped_refptr<net::UploadData> upload_;
   GURL url_;
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index ab5c72d..ece803e 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -58,6 +58,17 @@ std::string TestNetResourceProvider(int key) {
   return "header";
 }
 
+// Do a case-insensitive search through |haystack| for |needle|.
+bool ContainsString(const std::string& haystack, const char* needle) {
+  std::string::const_iterator it =
+      std::search(haystack.begin(),
+                  haystack.end(),
+                  needle,
+                  needle + strlen(needle),
+                  CaseInsensitiveCompare<char>());
+  return it != haystack.end();
+}
+
 }  // namespace
 
 TEST(URLRequestTest, GetTest_NoCache) {
@@ -767,3 +778,43 @@ TEST(URLRequestTest, BasicAuth) {
   }
 }
 
+// In this test, we do a POST which the server will 302 redirect.
+// The subsequent transaction should use GET, and should not send the
+// Content-Type header.
+// http://code.google.com/p/chromium/issues/detail?id=843
+TEST(URLRequestTest, Post302RedirectGet) {
+  TestServer server(L"net/data/url_request_unittest");
+  TestDelegate d;
+  TestURLRequest req(server.TestServerPage("files/redirect-to-echoall"), &d);
+  req.set_method("POST");
+
+  // Set headers (some of which are specific to the POST).
+  // ("Content-Length: 10" is just a junk value to make sure it gets stripped).
+  req.SetExtraRequestHeaders(
+    "Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryAADeAA+NAAWMAAwZ\r\n"
+    "Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5\r\n"
+    "Accept-Language: en-US,en\r\n"
+    "Accept-Charset: ISO-8859-1,*,utf-8\r\n"
+    "Content-Length: 10\r\n"
+    "Origin: http://localhost:1337/"
+  );
+  req.Start();
+  MessageLoop::current()->Run();
+
+  std::string mime_type;
+  req.GetMimeType(&mime_type);
+  EXPECT_EQ("text/html", mime_type);
+
+  const std::string& data = d.data_received();
+
+  // Check that the post-specific headers were stripped:
+  EXPECT_FALSE(ContainsString(data, "Content-Length:"));
+  EXPECT_FALSE(ContainsString(data, "Content-Type:"));
+  EXPECT_FALSE(ContainsString(data, "Origin:"));
+
+  // These extra request headers should not have been stripped.
+  EXPECT_TRUE(ContainsString(data, "Accept:"));
+  EXPECT_TRUE(ContainsString(data, "Accept-Language:"));
+  EXPECT_TRUE(ContainsString(data, "Accept-Charset:"));
+}
+