To fix the cross-site post submission bug.

content/renderer/render_view_impl.cc

 // Copyright (c) 2012 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/renderer/render_view_impl.h"
 
+#include "webkit/glue/weburlloader_impl.h"
+
 #include <algorithm>
 #include <cmath>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/debug/trace_event.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
@@ skipping 161 matching lines @@
 #include "ui/gfx/point.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/size_conversions.h"
 #include "v8/include/v8.h"
 #include "webkit/appcache/web_application_cache_host_impl.h"
 #include "webkit/base/file_path_string_conversions.h"
 #include "webkit/dom_storage/dom_storage_types.h"
 #include "webkit/glue/alt_error_page_resource_fetcher.h"
 #include "webkit/glue/dom_operations.h"
 #include "webkit/glue/glue_serialize.h"
+#include "webkit/glue/resource_request_body.h"
 #include "webkit/glue/web_intent_service_data.h"
 #include "webkit/glue/webdropdata.h"
 #include "webkit/glue/webkit_constants.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/glue/weburlresponse_extradata_impl.h"
 #include "webkit/gpu/webgraphicscontext3d_in_process_impl.h"
 #include "webkit/media/webmediaplayer_impl.h"
 #include "webkit/media/webmediaplayer_ms.h"
 #include "webkit/plugins/npapi/plugin_list.h"
 #include "webkit/plugins/npapi/webplugin_delegate.h"
@@ skipping 115 matching lines @@
 using WebKit::WebVector;
 using WebKit::WebView;
 using WebKit::WebWidget;
 using WebKit::WebWindowFeatures;
 using appcache::WebApplicationCacheHostImpl;
 using base::Time;
 using base::TimeDelta;
 
 using webkit_glue::AltErrorPageResourceFetcher;
 using webkit_glue::ResourceFetcher;
+using webkit_glue::ResourceRequestBody;
 using webkit_glue::WebPreferences;
 using webkit_glue::WebURLResponseExtraDataImpl;
 
 #if defined(OS_ANDROID)
 using WebKit::WebContentDetectionResult;
 using WebKit::WebFloatPoint;
 using WebKit::WebFloatRect;
 using WebKit::WebHitTestResult;
 #endif
+using content::ViewMsg_Request;
 
 namespace content {
 
+  // Will find a proper place to move to.
+  ViewMsg_Request::ViewMsg_Request() {
+  }
+
+  ViewMsg_Request::~ViewMsg_Request() {
+  }
+
 //-----------------------------------------------------------------------------
 
 typedef std::map<WebKit::WebView*, RenderViewImpl*> ViewMap;
 static base::LazyInstance<ViewMap> g_view_map = LAZY_INSTANCE_INITIALIZER;
 
 // Time, in seconds, we delay before sending content state changes (such as form
 // state and scroll position) to the browser. We delay sending changes to avoid
 // spamming the browser.
 // To avoid having tab/session restore require sending a message to get the
 // current content state during tab closing we use a shorter timeout for the
@@ skipping 780 matching lines @@
 
     if (!params.extra_headers.empty()) {
       for (net::HttpUtil::HeadersIterator i(params.extra_headers.begin(),
                                             params.extra_headers.end(), "\n");
            i.GetNext(); ) {
         request.addHTTPHeaderField(WebString::fromUTF8(i.name()),
                                    WebString::fromUTF8(i.values()));
       }
     }
 
-    if (params.is_post) {
-      request.setHTTPMethod(WebString::fromUTF8("POST"));
-
-      // Set post data.
+    // Deal With Cross-Process Post Submission
+    if(params.is_post) {
       WebHTTPBody http_body;
       http_body.initialize();
-      http_body.appendData(WebData(
-          reinterpret_cast<const char*>(
-              &params.browser_initiated_post_data.front()),
-          params.browser_initiated_post_data.size()));
+      const std::vector<ResourceRequestBody::Element>* uploads =
+          params.request.request_body->elements();
+      std::vector<ResourceRequestBody::Element>::const_iterator iter;
+      for (iter = uploads->begin(); iter != uploads->end(); ++iter) {
+        switch (iter->type()) {
+          case ResourceRequestBody::Element::TYPE_BYTES: {
+            http_body.appendData(WebData(iter->bytes(),
+                                         static_cast<int>(iter->length())));
+            break;
+          }
+          case ResourceRequestBody::Element::TYPE_FILE: {
+            http_body.appendFileRange(
+                WebString::fromUTF8(iter->path().value()),
+                static_cast<long long>(iter->offset()),
+                static_cast<long long>(iter->length()),
+                iter->expected_modification_time().ToDoubleT());
+            break;
+          }
+          case ResourceRequestBody::Element::TYPE_FILE_FILESYSTEM: {
+            // Since I cannot found a cross-site post submission
+            // in this type which does not use XHR,
+            // this part have't been testes.

[michaeln, 2012/11/02 00:08:23]

Q: do cross-site posts ever cross storage partition boundaries?

[irobert, 2012/11/02 17:22:55]

I need to chat with Albert about it.

On 2012/11/02 00:08:23, michaeln wrote:

+            http_body.appendURLRange(
+                iter->url(),
+                static_cast<long long>(iter->offset()),
+                static_cast<long long>(iter->length()),
+                iter->expected_modification_time().ToDoubleT());
+            break;
+          }
+          case ResourceRequestBody::Element:: TYPE_BLOB: {
+            // Since I cannot found a cross-site post submission
+            // in this type which does not use XHR,
+            // this part have't been testes.
+            http_body.appendBlob(iter->url());
+            break;
+          }
+          default:
+            NOTREACHED();
+        }
+      }
       request.setHTTPBody(http_body);
+      request.setHTTPMethod(WebString::fromUTF8(params.request.method));
+      std::string type = params.request.headers.find("Content-Type")->second;
+      request.setHTTPHeaderField(
+          WebString::fromUTF8("Content-Type"),
+          WebString::fromUTF8(type));
     }
 
     main_frame->loadRequest(request);
   }
-
   // In case LoadRequest failed before DidCreateDataSource was called.
   pending_navigation_params_.reset();
 }
 
 bool RenderViewImpl::IsBackForwardToStaleEntry(
     const ViewMsg_Navigate_Params& params,
     bool is_reload) {
   // Make sure this isn't a back/forward to an entry we have already cropped
   // or replaced from our history, before the browser knew about it.  If so,
   // a new navigation has committed in the mean time, and we can ignore this.
@@ skipping 496 matching lines @@
                              const Referrer& referrer,
                              WebNavigationPolicy policy) {
   Send(new ViewHostMsg_OpenURL(
       routing_id_,
       url,
       referrer,
       NavigationPolicyToDisposition(policy),
       frame->identifier()));
 }
 
+// Not sure whether to change the original OpenURL API by adding a
+// new argument. To avoid too much change, use OpenPostURL API do
+// send ViewHostMsg_OpenPostURL message to browser process.
+void RenderViewImpl::OpenPostURL(WebFrame* frame,
+                                 const GURL& url,
+                                 const Referrer& referrer,
+                                 WebNavigationPolicy policy,
+                                 const ViewMsg_Request& request) {
+  Send(new ViewHostMsg_OpenPostURL(
+      routing_id_,
+      url,
+      referrer,
+      NavigationPolicyToDisposition(policy),
+      frame->identifier(),
+      request));
+}
+
 // WebViewDelegate ------------------------------------------------------------
 
 void RenderViewImpl::LoadNavigationErrorPage(
     WebFrame* frame,
     const WebURLRequest& failed_request,
     const WebURLError& error,
     const std::string& html,
     bool replace) {
   std::string alt_html;
   const std::string* error_html;
@@ skipping 1040 matching lines @@
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kEnableStrictSiteIsolation) &&
       !frame->parent() && (is_content_initiated || is_redirect)) {
     WebString origin_str = frame->document().securityOrigin().toString();
     GURL frame_url(origin_str.utf8().data());
     // TODO(cevans): revisit whether this site check is still necessary once
     // crbug.com/101395 is fixed.
     if (!net::RegistryControlledDomainService::SameDomainOrHost(frame_url,
                                                                 url) ||
         frame_url.scheme() != url.scheme()) {
-      OpenURL(frame, url, referrer, default_policy);
-      return WebKit::WebNavigationPolicyIgnore;
+      WebString method = request.httpMethod();
+      if(method != WebString("POST")) {
+        OpenURL(frame, url, referrer, default_policy);
+        return WebKit::WebNavigationPolicyIgnore;
+      }
+      else {

[michaeln, 2012/11/02 00:08:23]

style nit : } else {

[irobert, 2012/11/02 17:22:55]

Done.

+        ViewMsg_Request params;
+
+        // Convert WebHTTPBody to ResourceRequestBody
+        scoped_refptr<ResourceRequestBody> request_body =
+            new ResourceRequestBody();
+        WebHTTPBody body = request.httpBody();
+        WebKit::WebHTTPBody::Element element;
+        for (int i=0; body.elementAt(i, element); i++) {
+          switch (element.type) {
+            case WebHTTPBody::Element::TypeData:
+              if (!element.data.isEmpty())
+                request_body->AppendBytes(
+                    element.data.data(), static_cast<int>(element.data.size()));
+              break;
+            case WebHTTPBody::Element::TypeFile: {
+              #if defined(OS_POSIX)
+                const FilePath::StringType kFilePath = FILE_PATH_LITERAL(

[michaeln, 2012/11/02 00:08:23]

use of FILE_PATH_LITERAL seems odd since the value here is not a literal?

[irobert, 2012/11/02 17:22:55]

Also looks odd to me. But I just followed the usage in the file
resource_request_body_unittest.cc:24.

On 2012/11/02 00:08:23, michaeln wrote:

+                    base::SysWideToNativeMB(UTF16ToWideHack(element.filePath)));
+              #elif defined(OS_WIN)
+                const FilePath::StringType kFilePath = FILE_PATH_LITERAL(
+                    UTF16ToWideHack(element.filePath));
+              #endif
+              if (element.fileLength == -1) {
+                request_body->AppendFileRange(
+                    FilePath(kFilePath), 0, kuint64max, base::Time());
+              } else {
+                request_body->AppendFileRange(
+                    FilePath(kFilePath),
+                    static_cast<uint64>(element.fileStart),
+                    static_cast<uint64>(element.fileLength),
+                    base::Time::FromDoubleT(element.modificationTime));
+              }
+              break;
+            }
+            case WebHTTPBody::Element::TypeURL: {
+              GURL gurl = GURL(element.url);
+              DCHECK(gurl.SchemeIsFileSystem());
+              request_body->AppendFileSystemFileRange(
+                  gurl,
+                  static_cast<uint64>(element.fileStart),
+                  static_cast<uint64>(element.fileLength),
+                  base::Time::FromDoubleT(element.modificationTime));
+              break;
+            }
+            case WebHTTPBody::Element::TypeBlob:
+              request_body->AppendBlob(GURL(element.blobURL));
+              break;
+            default:
+              NOTREACHED();
+          }
+          request_body->set_identifier(body.identifier());
+        }
+        params.request_body = request_body;
+        params.method = "POST";
+        params.url = url;
+        // Extract Header Info.
+        WebString ContentType =
+            request.httpHeaderField(WebString::fromUTF8("Content-Type"));
+        std::string ContentTypeStr (ContentType.utf8().data(),
+                                    ContentType.utf8().length());
+        params.headers.insert(std::pair<std::string,std::string>(

[michaeln, 2012/11/02 00:08:23]

if there's at most one header value in here, maybe we don't need a whole headers
struct for it and instead could just sent contentType as an explicit msg param

[irobert, 2012/11/02 17:22:55]

For Cross-Process Post Submission, I think Content-Type is the only important
header. The reason why I did not use explicit msg param is that I think it can
be reused for other problems which need to send all the header through IPC.

On 2012/11/02 00:08:23, michaeln wrote:

+                                  "Content-Type", ContentTypeStr));
+
+        OpenPostURL(frame, url, referrer, default_policy, params);
+        return default_policy;

[michaeln, 2012/11/02 00:08:23]

return default_policy? Is that right?

Seems like this s/b WebKit::WebNavigationPolicyIgnor since by calling
OpenPostURL we've just asked the browser process to handle this post in a
different renderer. By returning default, does will this POST happens twice,
once in this renderer and again in the other renderer per OpenPostURL
processing?

[irobert, 2012/11/02 17:22:55]

Yes. return WebKit::WebNavigationPolicyIgnor will break the cross-site Post
submission. I did not notice the POST happens twice. But I am still
investigating what is the right value to return.

On 2012/11/02 00:08:23, michaeln wrote:

[Charlie Reis, 2012/11/05 16:21:40]

Ignore is the correct thing to return.  There must be some other issue if that
breaks the submission, because returning the default policy means the old
renderer will continue with its POST submission as well.

+      }
     }
   }
 
   // If the browser is interested, then give it a chance to look at the request.
   if (is_content_initiated) {
     bool browser_handles_request =
         renderer_preferences_.browser_handles_non_local_top_level_requests &&
         IsNonLocalTopLevelNavigation(url, frame, type);
     if (!browser_handles_request) {
       browser_handles_request =
           renderer_preferences_.browser_handles_all_top_level_requests &&
           IsTopLevelNavigation(frame);
     }
 
     if (browser_handles_request) {
       // Reset these counters as the RenderView could be reused for the next
       // navigation.
       page_id_ = -1;
       last_page_id_sent_to_browser_ = -1;
       OpenURL(frame, url, referrer, default_policy);

[michaeln, 2012/11/02 00:08:23]

What is the difference between the cross-site handling above (where you've
extended things to also deal with POSTS) and this additional logic? Why does
this logic not have to deal with POSTS where the logic above does?

I'm not so familiar with this code, so can you explain it to me?

[irobert, 2012/11/02 17:22:55]

The above code deal with the case when flag --enable-strict-site-isolation is
enabled and the top-level navigation. My understanding of the bug is we only
need to deal with the strict site isolation case.

On 2012/11/02 00:08:23, michaeln wrote:

[Charlie Reis, 2012/11/05 16:21:40]

Hmm, that's not correct.  We'd like this new approach to work for any
cross-process navigation, including from a normal page to a hosted app (not just
--enable-strict-site-isolation).  We should make sure it behaves correctly in
other cases as well.

       return WebKit::WebNavigationPolicyIgnore;  // Suppress the load here.
     }
   }
 
   // Use the frame's original request's URL rather than the document's URL for
   // subsequent checks.  For a popup, the document's URL may become the opener
   // window's URL if the opener has called document.write().
   // See http://crbug.com/93517.
   GURL old_url(frame->dataSource()->request().url());
 
@@ skipping 3582 matching lines @@
 }
 #endif
 
 void RenderViewImpl::OnReleaseDisambiguationPopupDIB(
     TransportDIB::Handle dib_handle) {
   TransportDIB* dib = TransportDIB::CreateWithHandle(dib_handle);
   RenderProcess::current()->ReleaseTransportDIB(dib);
 }
 
 }  // namespace content