Do not copy reference fragments for overridden redirects.

net/url_request/url_request_http_job.cc

Index: net/url_request/url_request_http_job.cc
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 41b15158d5728f0022466d97b8991e4b4de8b755..b38044ca808cea72ce2f49016e0f4c957ee92471 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -1034,6 +1034,28 @@ Filter* URLRequestHttpJob::SetupFilter() const {
       ? Filter::Factory(encoding_types, *filter_context_) : NULL;
 }
 
+bool URLRequestHttpJob::IsRedirectResponse(GURL* location,
+                                           int* http_status_code) {
+  if (!URLRequestJob::IsRedirectResponse(location, http_status_code))
+    return false;
+  const GURL& url = request_->url();
+
+  // Move the reference fragment of the old location to the new one if the
+  // new one has none. This duplicates mozilla's behavior.
+  // If |allowed_unsafe_redirect_url_| is set, then we assume that the redirect
+  // URL has been overridden by the network delegate, so the redirection URL
+  // must not be modified.
+  if (url.is_valid() && url.has_ref() && !location->has_ref() &&
+      !allowed_unsafe_redirect_url_.is_valid()) {

[mmenke, 2014/03/27 14:41:46]

This partially breaks the reason for having allowed_unsafe_redirect_url_
separate from the headers.  It should be compared to location.

I also think this is a completely non-obvious behavior, not at all implied by
"allowed_unsafe_redirect_url_".  I don't currently have a suggestion to fix
that, but I really don't want confusion over the API to lead to obscure future
bugs, 5 years down the road.

+    GURL::Replacements replacements;
+    // Reference the |ref| directly out of the original URL to avoid a malloc.
+    replacements.SetRef(url.spec().data(),
+                        url.parsed_for_possibly_invalid_spec().ref);
+    *location = location->ReplaceComponents(replacements);
+  }
+  return true;
+}
+
 bool URLRequestHttpJob::IsSafeRedirect(const GURL& location) {
   // HTTP is always safe.
   // TODO(pauljensen): Remove once crbug.com/146591 is fixed.
@@ -1041,14 +1063,9 @@ bool URLRequestHttpJob::IsSafeRedirect(const GURL& location) {
       (location.scheme() == "http" || location.scheme() == "https")) {
     return true;
   }
-  // Delegates may mark an URL as safe for redirection.
+  // Delegates may mark a URL as safe for redirection.
   if (allowed_unsafe_redirect_url_.is_valid()) {
-    GURL::Replacements replacements;
-    replacements.ClearRef();
-    if (allowed_unsafe_redirect_url_.ReplaceComponents(replacements) ==
-        location.ReplaceComponents(replacements)) {
-      return true;
-    }
+    return allowed_unsafe_redirect_url_ == location;
   }
   // Query URLRequestJobFactory as to whether |location| would be safe to
   // redirect to.