Changeset 251786 in webkit


Ignore:
Timestamp:
Oct 30, 2019 11:32:59 AM (5 years ago)
Author:
Chris Dumez
Message:

REGRESSION (r238252): HTTP POST is losing application/x-www-form-urlencoded body if there's a redirect to different host
​https://bugs.webkit.org/show_bug.cgi?id=201950
<rdar://problem/55577782>

Reviewed by Alex Christensen.

Source/WebKit:

The resource request body was getting lost on cross-site redirects. This was caused by the fact that a cross-site
redirect would cause a process-swap and the request to start again from a new process. This would work fine if
the request does not have a body. However, we have an optimization in place which avoids encoding the request body
whenever it is sent over IPC. Because the WebResourceLoader::WillSendRequest IPC would not encode the request body,
any decision to process swap as a result of this IPC (i.e. redirect) would cause the new request in the new process
to be missing its body. To address the issue, we now make sure to pass the request body in the WillSendRequest IPC
and reconsile the request with its body on the recipient side.

Test: http/tests/misc/form-submit-file-cross-site-redirect.html

  • NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::continueWillSendRedirectedRequest):

  • NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:

(WebKit::ServiceWorkerFetchTask::didReceiveRedirectResponse):

  • WebProcess/Network/WebResourceLoader.cpp:

(WebKit::WebResourceLoader::willSendRequest):

  • WebProcess/Network/WebResourceLoader.h:
  • WebProcess/Network/WebResourceLoader.messages.in:

LayoutTests:

Add layout test coverage.

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

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r251780 r251786  
     12019-10-30  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION (r238252): HTTP POST is losing application/x-www-form-urlencoded body if there's a redirect to different host
     4        https://bugs.webkit.org/show_bug.cgi?id=201950
     5        <rdar://problem/55577782>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Add layout test coverage.
     10
     11        * http/tests/misc/form-submit-file-cross-site-redirect-expected.txt: Added.
     12        * http/tests/misc/form-submit-file-cross-site-redirect.html: Added.
     13
    1142019-10-30  Antti Koivisto  <antti@apple.com>
    215

  • trunk/LayoutTests/platform/win/TestExpectations

    r251746 r251786  
    214214fast/forms/file/recover-file-input-in-unposted-form.html [ Skip ]
    215215http/tests/misc/form-submit-file-cross-site.html [ Skip ]
     216http/tests/misc/form-submit-file-cross-site-redirect.html [ Skip ]
    216217http/tests/xmlhttprequest/post-blob-content-type-async.html [ Skip ]
    217218http/tests/xmlhttprequest/post-blob-content-type-sync.html [ Skip ]

  • trunk/Source/WebKit/ChangeLog

    r251784 r251786  
     12019-10-30  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION (r238252): HTTP POST is losing application/x-www-form-urlencoded body if there's a redirect to different host
     4        https://bugs.webkit.org/show_bug.cgi?id=201950
     5        <rdar://problem/55577782>
     6
     7        Reviewed by Alex Christensen.
     8
     9        The resource request body was getting lost on cross-site redirects. This was caused by the fact that a cross-site
     10        redirect would cause a process-swap and the request to start again from a new process. This would work fine if
     11        the request does not have a body. However, we have an optimization in place which avoids encoding the request body
     12        whenever it is sent over IPC. Because the WebResourceLoader::WillSendRequest IPC would not encode the request body,
     13        any decision to process swap as a result of this IPC (i.e. redirect) would cause the new request in the new process
     14        to be missing its body. To address the issue, we now make sure to pass the request body in the WillSendRequest IPC
     15        and reconsile the request with its body on the recipient side.
     16
     17        Test: http/tests/misc/form-submit-file-cross-site-redirect.html
     18
     19        * NetworkProcess/NetworkResourceLoader.cpp:
     20        (WebKit::NetworkResourceLoader::continueWillSendRedirectedRequest):
     21        * NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:
     22        (WebKit::ServiceWorkerFetchTask::didReceiveRedirectResponse):
     23        * WebProcess/Network/WebResourceLoader.cpp:
     24        (WebKit::WebResourceLoader::willSendRequest):
     25        * WebProcess/Network/WebResourceLoader.h:
     26        * WebProcess/Network/WebResourceLoader.messages.in:
     27
    1282019-10-30  Jer Noble  <jer.noble@apple.com>
    229

  • trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

    r251710 r251786  
    724724        networkSession->handleAdClickAttributionConversion(WTFMove(*adClickConversion), request.url(), redirectRequest);
    725725
    726     send(Messages::WebResourceLoader::WillSendRequest(redirectRequest, sanitizeResponseIfPossible(WTFMove(redirectResponse), ResourceResponse::SanitizationType::Redirection)));
     726    // We send the request body separately because the ResourceRequest body normally does not get encoded when sent over IPC, as an optimization.
     727    // However, we really need the body here because a redirect cross-site may cause a process-swap and the request to start again in a new WebContent process.
     728    send(Messages::WebResourceLoader::WillSendRequest(redirectRequest, IPC::FormDataReference { redirectRequest.httpBody() }, sanitizeResponseIfPossible(WTFMove(redirectResponse), ResourceResponse::SanitizationType::Redirection)));
    727729}
    728730

  • trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp

    r251434 r251786  
    112112    auto newRequest = m_currentRequest.redirectedRequest(response, m_loader.parameters().shouldClearReferrerOnHTTPSToHTTPRedirect);
    113113
    114     sendToClient(Messages::WebResourceLoader::WillSendRequest { newRequest, response });
     114    sendToClient(Messages::WebResourceLoader::WillSendRequest { newRequest, IPC::FormDataReference { newRequest.httpBody() }, response });
    115115}
    116116

  • trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp

    r251585 r251786  
    2828
    2929#include "DataReference.h"
     30#include "FormDataReference.h"
    3031#include "Logging.h"
    3132#include "NetworkProcessConnection.h"
    … …  
    9394}
    9495
    95 void WebResourceLoader::willSendRequest(ResourceRequest&& proposedRequest, ResourceResponse&& redirectResponse)
     96void WebResourceLoader::willSendRequest(ResourceRequest&& proposedRequest, IPC::FormDataReference&& proposedRequestBody, ResourceResponse&& redirectResponse)
    9697{
    9798    Ref<WebResourceLoader> protectedThis(*this);
     99
     100    // Make the request whole again as we do not normally encode the request's body when sending it over IPC, for performance reasons.
     101    proposedRequest.setHTTPBody(proposedRequestBody.takeData());
    98102
    99103    LOG(Network, "(WebProcess) WebResourceLoader::willSendRequest to '%s'", proposedRequest.url().string().latin1().data());

  • trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.h

    r251124 r251786  
    3838namespace IPC {
    3939class DataReference;
     40class FormDataReference;
    4041}
    4142
    … …  
    8081    uint64_t messageSenderDestinationID() const override;
    8182
    82     void willSendRequest(WebCore::ResourceRequest&&, WebCore::ResourceResponse&&);
     83    void willSendRequest(WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&&);
    8384    void didSendData(uint64_t bytesSent, uint64_t totalBytesToBeSent);
    8485    void didReceiveResponse(const WebCore::ResourceResponse&, bool needsContinueDidReceiveResponseMessage);

  • trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.messages.in

    r251124 r251786  
    2222
    2323messages -> WebResourceLoader LegacyReceiver {
    24     WillSendRequest(WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse)
     24    WillSendRequest(WebCore::ResourceRequest request, IPC::FormDataReference requestBody, WebCore::ResourceResponse redirectResponse)
    2525    DidSendData(uint64_t bytesSent, uint64_t totalBytesToBeSent)
    2626    DidReceiveResponse(WebCore::ResourceResponse response, bool needsContinueDidReceiveResponseMessage)
Note: See TracChangeset for help on using the changeset viewer.