⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 237639 in webkit


Ignore:
Timestamp:
Oct 31, 2018, 9:23:27 AM (8 years ago)
Author:
Chris Dumez
Message:

[PSON] When process-swapping for a POST request the HTTP body gets dropped
​https://bugs.webkit.org/show_bug.cgi?id=191046
<rdar://problem/45229732>

Reviewed by Alex Christensen.

Source/WebKit:

For performance reasons, the ResourceRequest IPC encoder does not encode the request's HTTP body (aka form data).
When we decide to process-swap for a POST request in WebPageProxy::decidePolicyForNavigationAction(), the request
we pass the new WebProcess thus no longer has a HTTP body and the load will likely fail in the new process.

To address the issue, we now pass the request body along with the request when sending the DecidePolicyForNavigationActionAsync
/ DecidePolicyForNavigationActionSync IPC from the WebProcess to the UIProcess. No action is needed for the
LoadRequest IPC to the new WebProcess since the LoadParameters' IPC encoder takes care of encoding the request's
body already.

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::decidePolicyForNavigationActionAsync):
(WebKit::WebPageProxy::decidePolicyForNavigationActionSync):

  • UIProcess/WebPageProxy.h:
  • UIProcess/WebPageProxy.messages.in:
  • WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:

(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):

LayoutTests:

Add layout test coverage.

  • http/tests/misc/form-post-textplain-cross-site-expected.txt: Added.
  • http/tests/misc/form-post-textplain-cross-site.html: Added.
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r237620 r237639  
     12018-10-31  Chris Dumez  <cdumez@apple.com>
     2
     3        [PSON] When process-swapping for a POST request the HTTP body gets dropped
     4        https://bugs.webkit.org/show_bug.cgi?id=191046
     5        <rdar://problem/45229732>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Add layout test coverage.
     10
     11        * http/tests/misc/form-post-textplain-cross-site-expected.txt: Added.
     12        * http/tests/misc/form-post-textplain-cross-site.html: Added.
     13
    1142018-10-30  David Kilzer  <ddkilzer@apple.com>
    215

  • trunk/LayoutTests/http/tests/misc/form-post-textplain-expected.txt

    r132758 r237639  
    1 This is a test for 20795 and 100445, it makes sure that forms POSTed with a content-type of text/plain actually send data in text/plain
     1This test makes sure that forms POSTed with a content-type of text/plain actually send data in text/plain
    22
    33SUCCESS: Content-type is text/plain.

  • trunk/LayoutTests/http/tests/misc/resources/form-post-textplain.php

    r132758 r237639  
    33?>
    44<html>
    5 <head>
    6 <title>Regression test for bug 20795 and 100445</title>
    7 </head>
    85<body>
    96<p>
    10 This is a test for 20795 and 100445, it makes sure that forms POSTed with a content-type of text/plain actually send data in text/plain
     7This test makes sure that forms POSTed with a content-type of text/plain actually send data in text/plain
    118</p>
    129<?php

  • trunk/Source/WebKit/ChangeLog

    r237629 r237639  
     12018-10-31  Chris Dumez  <cdumez@apple.com>
     2
     3        [PSON] When process-swapping for a POST request the HTTP body gets dropped
     4        https://bugs.webkit.org/show_bug.cgi?id=191046
     5        <rdar://problem/45229732>
     6
     7        Reviewed by Alex Christensen.
     8
     9        For performance reasons, the ResourceRequest IPC encoder does not encode the request's HTTP body (aka form data).
     10        When we decide to process-swap for a POST request in WebPageProxy::decidePolicyForNavigationAction(), the request
     11        we pass the new WebProcess thus no longer has a HTTP body and the load will likely fail in the new process.
     12
     13        To address the issue, we now pass the request body along with the request when sending the DecidePolicyForNavigationActionAsync
     14        / DecidePolicyForNavigationActionSync IPC from the WebProcess to the UIProcess. No action is needed for the
     15        LoadRequest IPC to the new WebProcess since the LoadParameters' IPC encoder takes care of encoding the request's
     16        body already.
     17
     18        * UIProcess/WebPageProxy.cpp:
     19        (WebKit::WebPageProxy::decidePolicyForNavigationActionAsync):
     20        (WebKit::WebPageProxy::decidePolicyForNavigationActionSync):
     21        * UIProcess/WebPageProxy.h:
     22        * UIProcess/WebPageProxy.messages.in:
     23        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
     24        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
     25
    1262018-10-31  Antti Koivisto  <antti@apple.com>
    227

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r237624 r237639  
    6060#include "DrawingAreaProxy.h"
    6161#include "EventDispatcherMessages.h"
     62#include "FormDataReference.h"
    6263#include "FrameInfoData.h"
    6364#include "LoadParameters.h"
    … …  
    40624063#endif
    40634064
    4064 void WebPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, uint64_t listenerID)
     4065void WebPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, uint64_t listenerID)
    40654066{
    40664067    auto* frame = m_process->webFrame(frameID);
    40674068    MESSAGE_CHECK(frame);
    40684069
    4069     decidePolicyForNavigationAction(*frame, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, PolicyDecisionSender::create([this, protectedThis = makeRef(*this), frameID, listenerID] (auto... args) {
     4070    decidePolicyForNavigationAction(*frame, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, PolicyDecisionSender::create([this, protectedThis = makeRef(*this), frameID, listenerID] (auto... args) {
    40704071        m_process->send(Messages::WebPage::DidReceivePolicyDecision(frameID, listenerID, args...), m_pageID);
    40714072    }));
    40724073}
    40734074
    4074 void WebPageProxy::decidePolicyForNavigationAction(WebFrameProxy& frame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&& sender)
     4075void WebPageProxy::decidePolicyForNavigationAction(WebFrameProxy& frame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&& sender)
    40754076{
    40764077    LOG(Loading, "WebPageProxy::decidePolicyForNavigationAction - Original URL %s, current target URL %s", originalRequest.url().string().utf8().data(), request.url().string().utf8().data());
    40774078
    40784079    PageClientProtector protector(pageClient());
     4080
     4081    // Make the request whole again as we do not normally encode the request's body when sending it over IPC, for performance reasons.
     4082    request.setHTTPBody(requestBody.takeData());
    40794083
    40804084    auto transaction = m_pageLoadState.transaction();
    … …  
    41664170}
    41674171
    4168 void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
     4172void WebPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
    41694173{
    41704174    auto sender = PolicyDecisionSender::create(WTFMove(reply));
    … …  
    41814185        RELEASE_ASSERT(frame);
    41824186    }
    4183    
    4184     decidePolicyForNavigationAction(*frame, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, sender.copyRef());
     4187
     4188    decidePolicyForNavigationAction(*frame, frameSecurityOrigin, navigationID, WTFMove(navigationActionData), frameInfoData, originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, shouldSkipSafeBrowsingCheck, sender.copyRef());
    41854189
    41864190    // If the client did not respond synchronously, proceed with the load.

  • trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r237624 r237639  
    14511451    void didDestroyNavigation(uint64_t navigationID);
    14521452
    1453     void decidePolicyForNavigationAction(WebFrameProxy&, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&&);
    1454     void decidePolicyForNavigationActionAsync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, uint64_t listenerID);
    1455     void decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
     1453    void decidePolicyForNavigationAction(WebFrameProxy&, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Ref<PolicyDecisionSender>&&);
     1454    void decidePolicyForNavigationActionAsync(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, uint64_t listenerID);
     1455    void decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&&, const FrameInfoData&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData&, WebCore::ShouldSkipSafeBrowsingCheck, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
    14561456    void decidePolicyForNewWindowAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, NavigationActionData&&, WebCore::ResourceRequest&&, const String& frameName, uint64_t listenerID, const UserData&);
    14571457    void decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&);

  • trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in

    r237624 r237639  
    108108    # Policy messages
    109109    DecidePolicyForResponse(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, WebCore::ResourceResponse response, WebCore::ResourceRequest request, bool canShowMIMEType, uint64_t listenerID, WebKit::UserData userData)
    110     DecidePolicyForNavigationActionAsync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum:bool WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, uint64_t listenerID)
    111     DecidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum:bool WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck) -> (enum:uint8_t WebCore::PolicyAction policyAction, uint64_t newNavigationID, WebKit::DownloadID downloadID, std::optional<WebKit::WebsitePoliciesData> websitePolicies) Delayed
     110    DecidePolicyForNavigationActionAsync(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum:bool WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck, uint64_t listenerID)
     111    DecidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, struct WebCore::SecurityOriginData frameSecurityOrigin, uint64_t navigationID, struct WebKit::NavigationActionData navigationActionData, struct WebKit::FrameInfoData originatingFrameInfoData, uint64_t originatingPageID, WebCore::ResourceRequest originalRequest, WebCore::ResourceRequest request, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse, WebKit::UserData userData, enum:bool WebCore::ShouldSkipSafeBrowsingCheck shouldSkipSafeBrowsingCheck) -> (enum:uint8_t WebCore::PolicyAction policyAction, uint64_t newNavigationID, WebKit::DownloadID downloadID, std::optional<WebKit::WebsitePoliciesData> websitePolicies) Delayed
    112112    DecidePolicyForNewWindowAction(uint64_t frameID, struct WebCore::SecurityOriginData frameSecurityOrigin, struct WebKit::NavigationActionData navigationActionData, WebCore::ResourceRequest request, String frameName, uint64_t listenerID, WebKit::UserData userData)
    113113    UnableToImplementPolicy(uint64_t frameID, WebCore::ResourceError error, WebKit::UserData userData)

  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp

    r237467 r237639  
    3131#include "DrawingArea.h"
    3232#include "FindController.h"
     33#include "FormDataReference.h"
    3334#include "FrameInfoData.h"
    3435#include "InjectedBundle.h"
    … …  
    898899        std::optional<WebsitePoliciesData> websitePolicies;
    899900
    900         if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->frameID(), m_frame->isMainFrame(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), shouldSkipSafeBrowsingCheck), Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(policyAction, newNavigationID, downloadID, websitePolicies))) {
     901        if (!webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->frameID(), m_frame->isMainFrame(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, IPC::FormDataReference { request.httpBody() }, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), shouldSkipSafeBrowsingCheck), Messages::WebPageProxy::DecidePolicyForNavigationActionSync::Reply(policyAction, newNavigationID, downloadID, websitePolicies))) {
    901902            m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
    902903            return;
    … …  
    908909
    909910    ASSERT(policyDecisionMode == PolicyDecisionMode::Asynchronous);
    910     if (!webPage->send(Messages::WebPageProxy::DecidePolicyForNavigationActionAsync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), shouldSkipSafeBrowsingCheck, listenerID)))
     911    if (!webPage->send(Messages::WebPageProxy::DecidePolicyForNavigationActionAsync(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), documentLoader->navigationID(), navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, IPC::FormDataReference { request.httpBody() }, redirectResponse, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get()), shouldSkipSafeBrowsingCheck, listenerID)))
    911912        m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
    912913}
Note: See TracChangeset for help on using the changeset viewer.