Preserving Content-Type header from http request in OpenURL path.

content/browser/frame_host/render_frame_host_manager.cc

 // Copyright 2013 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 "content/browser/frame_host/render_frame_host_manager.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 471 matching lines @@
   std::vector<GURL> rest_of_chain = transfer_url_chain;
   rest_of_chain.pop_back();
 
   transferring_render_frame_host->frame_tree_node()
       ->navigator()
       ->RequestTransferURL(
           transferring_render_frame_host, transfer_url, nullptr, rest_of_chain,
           referrer, page_transition, global_request_id,
           should_replace_current_entry,
           transfer_navigation_handle_->IsPost() ? "POST" : "GET",
-          transfer_navigation_handle_->resource_request_body());
+          transfer_navigation_handle_->resource_request_body(),
+          std::string() /* extra_headers */);

[Łukasz Anforowicz, 2016/09/22 21:23:16]

We are preserving the post body, because we know the renderer might want to
inspect it (i.e. in XSSAuditor).  We probably need to also preserve the extra
headers, because without Content-Type, the renderer might not be able to
properly inspect the post body.

I hope this can be done in another CL, because 1) current CL is sufficient for
fixing https://crbug.com/646261 (once we take care of CURRENT_TAB window
disposition) and 2) preserving extra headers requires more changes [i.e. we
preserve the post body via transfer_navigation_handle, as seen above]).

Hmmm... if the above is reasonable, then I probably should add a TODO here.
How does that sound?

[Łukasz Anforowicz, 2016/09/27 18:58:36]

It turns out that XSSAuditor works fine, even if we don't send the Content-Type
header here - XSSAuditor essentially ignores the Content-Type header. 
XSSAuditor can do this, because XSSAuditor looks for substrings in the whole
request body (i.e. without splitting it out into individual fields) - this will
work even if the body contains extra test coming from "multipart/form-data"
encoding.

In particular:

- Note how XSSAuditor calls httpBody->flattenToString() [1] which returns the
whole body as a string (omitting strings, only returning verbatim bytes included
in the body [2])

- Note how XSSAuditor searches [3] for substrings in the whole body.

[1]
https://chromium.googlesource.com/chromium/src/+/239fdac4d03219287e6f8aef8e7f...

[2]
https://chromium.googlesource.com/chromium/src/+/009459a350906e2c49d20f4f7ee1...

[3]
https://chromium.googlesource.com/chromium/src/+/239fdac4d03219287e6f8aef8e7f...

[Tom Sepez, 2016/09/27 19:59:10]

That's correct.

[Charlie Reis, 2016/09/30 21:31:55]

So, do you still intend to pass headers here, or it is safe not to?  I feel like
we should document it either way.

[Łukasz Anforowicz, 2016/09/30 23:16:59]

I think it is safe to not pass the headers (this has been confirmed for
XSSAuditor by tsepez@;  I am not aware of other renderer-side code that might
need to inspect headers of the original request *after* a transfer has been
made).

I've added a comment here.

 
   // The transferring request was only needed during the RequestTransferURL
   // call, so it is safe to clear at this point.
   cross_site_transferring_request_.reset();
 
   // If the navigation continued, the NavigationHandle should have been
   // transfered to a RenderFrameHost. In the other cases, it should be cleared.
   transfer_navigation_handle_.reset();
 
   // If the navigation in the new renderer did not start, inform the
@@ skipping 2175 matching lines @@
                                              resolved_url)) {
     DCHECK(!dest_instance ||
            dest_instance == render_frame_host_->GetSiteInstance());
     return false;
   }
 
   return true;
 }
 
 }  // namespace content