Preserve non-POST methods on 301/302 requests.

net/url_request/url_request.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/url_request/url_request.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/singleton.h"
@@ skipping 677 matching lines @@
   }
 
   if (!location.is_valid())
     return ERR_INVALID_URL;
 
   if (!job_->IsSafeRedirect(location)) {
     DVLOG(1) << "disallowing redirect: unsafe protocol";
     return ERR_UNSAFE_REDIRECT;
   }
 
-  bool strip_post_specific_headers = false;
-  if (http_status_code != 307) {
-    // 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.  For
-    // 307 redirects, browsers preserve the method.  The RFC says to prompt the
-    // user to confirm the generation of a new POST request, but IE omits this
-    // prompt and so shall we.
-    strip_post_specific_headers = method_ == "POST";
+  // 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.  For
+  // 307 redirects, browsers preserve the method.  The RFC says to prompt the
+  // user to confirm the generation of a new requests, other than GET and HEAD

[wtc, 2011/10/25 18:04:48]

Nit: "a new requests" has a grammatical error.  I guess you
meant to remove "a".

BUT, this comment on 307 redirects is confusing because the
http_status_code != 307 test is gone in the new code.  This
comment should be updated to match the new code.  For example,
the special case for 303, where all methods are converted
to GET, should be explained.

We should reference httpbis if we're implementing the new
httpbis rules here.

[wtc, 2011/10/25 18:33:39]

mmenke: it is less important to reference httpbis.

It is more important to make the comments describe the new
code.  The new code tests for 301, 302, and 303.  So the
comments should describe why we handle those three status
codes this way.  We probably can remove the old comment
about 307.

[mmenke, 2011/10/25 18:54:00]

How's this:

  // For 303 redirects, all request methods are converted to GETs, as per RFC
  // 2616.  The latest httpbis draft also allows POST requests to be converted
  // to GETs when following 301/302 redirects for historical reasons.  All
  // normal browsers do this and so shall we.  The RFC says to prompt the user
  // to confirm the generation of new requests, other than GET and HEAD
  // requests, but IE omits these prompts and so shall we.
  // See: 
http://greenbytes.de/tech/webdav/draft-ietf-httpbis-p2-semantics-latest.html#...

+  // requests, but IE omits these prompts and so shall we.
+  bool was_post = method_ == "POST";
+  if (http_status_code == 303 ||
+      ((http_status_code == 301 || http_status_code == 302) && was_post)) {
     method_ = "GET";
     upload_ = NULL;
+    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
+      StripPostSpecificHeaders(&extra_request_headers_);
+    }
   }
 
   // Suppress the referrer if we're redirecting out of https.
   if (GURL(referrer_).SchemeIsSecure() && !location.SchemeIsSecure())
     referrer_.clear();
 
   url_chain_.push_back(location);
   --redirect_limit_;
 
-  if (strip_post_specific_headers) {
-    // 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
-    StripPostSpecificHeaders(&extra_request_headers_);
-  }
-
   if (!final_upload_progress_)
     final_upload_progress_ = job_->GetUploadProgress();
 
   PrepareToRestart();
   Start();
   return OK;
 }
 
 const URLRequestContext* URLRequest::context() const {
   return context_.get();
@@ skipping 151 matching lines @@
 
 void URLRequest::SetUnblockedOnDelegate() {
   if (!blocked_on_delegate_)
     return;
   blocked_on_delegate_ = false;
   load_state_param_.clear();
   net_log_.EndEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_DELEGATE, NULL);
 }
 
 }  // namespace net