[CURL] Should handle redirects in WebCore
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Oct 2017 19:00:17 +0000 (19:00 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Oct 2017 19:00:17 +0000 (19:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=21242

Patch by Basuke Suzuki <Basuke.Suzuki@sony.com> on 2017-10-02
Reviewed by Alex Christensen.

* platform/network/ResourceHandle.cpp:
* platform/network/curl/CurlContext.cpp:
(WebCore::CurlHandle::enableAutoReferer): Deleted.
* platform/network/curl/CurlContext.h:
* platform/network/curl/CurlRequest.cpp:
(WebCore::CurlRequest::setupTransfer):
(WebCore::CurlRequest::didReceiveHeader):
(WebCore::CurlRequest::didReceiveData):
* platform/network/curl/ResourceHandleCurl.cpp:
(WebCore::ResourceHandle::start):
(WebCore::ResourceHandle::continueDidReceiveResponse):
(WebCore::ResourceHandle::continueWillSendRequest):
* platform/network/curl/ResourceHandleCurlDelegate.cpp:
(WebCore::ResourceHandleCurlDelegate::curlDidReceiveResponse):
(WebCore::ResourceHandleCurlDelegate::shouldRedirectAsGET):
(WebCore::ResourceHandleCurlDelegate::willSendRequest):
(WebCore::ResourceHandleCurlDelegate::continueWillSendRequest):
(WebCore::ResourceHandleCurlDelegate::continueAfterWillSendRequest):
* platform/network/curl/ResourceHandleCurlDelegate.h:
* platform/network/curl/ResourceResponse.h:
* platform/network/curl/ResourceResponseCurl.cpp:
(WebCore::ResourceResponse::shouldRedirect):
(WebCore::ResourceResponse::isMovedPermanently const):
(WebCore::ResourceResponse::isFound const):
(WebCore::ResourceResponse::isSeeOther const):
(WebCore::ResourceResponse::isRedirection const): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222728 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/network/ResourceHandle.cpp
Source/WebCore/platform/network/curl/CurlContext.cpp
Source/WebCore/platform/network/curl/CurlContext.h
Source/WebCore/platform/network/curl/CurlRequest.cpp
Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.cpp
Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.h
Source/WebCore/platform/network/curl/ResourceResponse.h
Source/WebCore/platform/network/curl/ResourceResponseCurl.cpp

index 0902852..56cd2c5 100644 (file)
@@ -1,3 +1,37 @@
+2017-10-02  Basuke Suzuki  <Basuke.Suzuki@sony.com>
+
+        [CURL] Should handle redirects in WebCore
+        https://bugs.webkit.org/show_bug.cgi?id=21242
+
+        Reviewed by Alex Christensen.
+
+        * platform/network/ResourceHandle.cpp:
+        * platform/network/curl/CurlContext.cpp:
+        (WebCore::CurlHandle::enableAutoReferer): Deleted.
+        * platform/network/curl/CurlContext.h:
+        * platform/network/curl/CurlRequest.cpp:
+        (WebCore::CurlRequest::setupTransfer):
+        (WebCore::CurlRequest::didReceiveHeader):
+        (WebCore::CurlRequest::didReceiveData):
+        * platform/network/curl/ResourceHandleCurl.cpp:
+        (WebCore::ResourceHandle::start):
+        (WebCore::ResourceHandle::continueDidReceiveResponse):
+        (WebCore::ResourceHandle::continueWillSendRequest):
+        * platform/network/curl/ResourceHandleCurlDelegate.cpp:
+        (WebCore::ResourceHandleCurlDelegate::curlDidReceiveResponse):
+        (WebCore::ResourceHandleCurlDelegate::shouldRedirectAsGET):
+        (WebCore::ResourceHandleCurlDelegate::willSendRequest):
+        (WebCore::ResourceHandleCurlDelegate::continueWillSendRequest):
+        (WebCore::ResourceHandleCurlDelegate::continueAfterWillSendRequest):
+        * platform/network/curl/ResourceHandleCurlDelegate.h:
+        * platform/network/curl/ResourceResponse.h:
+        * platform/network/curl/ResourceResponseCurl.cpp:
+        (WebCore::ResourceResponse::shouldRedirect):
+        (WebCore::ResourceResponse::isMovedPermanently const):
+        (WebCore::ResourceResponse::isFound const):
+        (WebCore::ResourceResponse::isSeeOther const):
+        (WebCore::ResourceResponse::isRedirection const): Deleted.
+
 2017-10-02  Antti Koivisto  <antti@apple.com>
 
         Crashes with guard malloc under RenderFullScreen::unwrapRenderer
index bb71e1d..45da881 100644 (file)
@@ -170,7 +170,7 @@ void ResourceHandle::didReceiveResponse(ResourceResponse&& response)
     }
 }
 
-#if !PLATFORM(COCOA) && !USE(CFURLCONNECTION) && !USE(SOUP)
+#if !PLATFORM(COCOA) && !USE(CFURLCONNECTION) && !USE(SOUP) && !USE(CURL)
 // ResourceHandle never uses async client calls on these platforms yet.
 void ResourceHandle::continueWillSendRequest(ResourceRequest&&)
 {
index ef5c9de..bfae5a8 100644 (file)
@@ -425,11 +425,6 @@ void CurlHandle::enableFollowLocation()
     curl_easy_setopt(m_handle, CURLOPT_MAXREDIRS, maxNumberOfRedirectCount);
 }
 
-void CurlHandle::enableAutoReferer()
-{
-    curl_easy_setopt(m_handle, CURLOPT_AUTOREFERER, 1L);
-}
-
 void CurlHandle::enableHttpAuthentication(long option)
 {
     curl_easy_setopt(m_handle, CURLOPT_HTTPAUTH, option);
index c7cf7c0..153f2f3 100644 (file)
@@ -248,7 +248,6 @@ public:
     void enableAllowedProtocols();
 
     void enableFollowLocation();
-    void enableAutoReferer();
 
     void enableHttpAuthentication(long);
     void setHttpAuthUserPass(const String&, const String&);
index ad425f0..4fa2723 100644 (file)
@@ -169,8 +169,6 @@ CURL* CurlRequest::setupTransfer()
     m_curlHandle->enableAcceptEncoding();
     m_curlHandle->enableTimeout();
 
-    m_curlHandle->enableAutoReferer();
-    m_curlHandle->enableFollowLocation();
     m_curlHandle->enableProxyIfExists();
     m_curlHandle->enableCookieJarIfExists();
 
@@ -275,11 +273,7 @@ size_t CurlRequest::didReceiveHeader(String&& header)
         return receiveBytes;
     }
 
-    // If the FOLLOWLOCATION option is enabled for the curl handle then
-    // curl will follow the redirections internally. Thus this header callback
-    // will be called more than one time with the line starting "HTTP" for one job.
-
-    m_response.url = m_curlHandle->getEffectiveURL();
+    m_response.url = m_request.url();
     m_response.statusCode = statusCode;
 
     if (auto length = m_curlHandle->getContentLength())
@@ -308,13 +302,6 @@ size_t CurlRequest::didReceiveData(Ref<SharedBuffer>&& buffer)
 
     auto receiveBytes = buffer->size();
 
-    // this shouldn't be necessary but apparently is. CURL writes the data
-    // of html page even if it is a redirect that was handled internally
-    // can be observed e.g. on gmail.com
-    auto statusCode = m_curlHandle->getResponseCode();
-    if (statusCode && (300 <= *statusCode) && (*statusCode < 400))
-        return receiveBytes;
-
     if (receiveBytes) {
         callDelegate([this, buffer = WTFMove(buffer)](CurlRequestDelegate* delegate) mutable {
             if (delegate)
index 7825646..2e7f0cf 100644 (file)
@@ -65,6 +65,13 @@ bool ResourceHandle::start()
     if (d->m_context && !d->m_context->isValid())
         return false;
 
+    // Only allow the POST and GET methods for non-HTTP requests.
+    const ResourceRequest& request = firstRequest();
+    if (!request.url().protocolIsInHTTPFamily() && request.httpMethod() != "GET" && request.httpMethod() != "POST") {
+        scheduleFailure(InvalidURLFailure); // Error must not be reported immediately
+        return true;
+    }
+
     d->m_delegate = adoptRef(new ResourceHandleCurlDelegate(this));
     return d->m_delegate->start();
 }
@@ -250,6 +257,20 @@ void ResourceHandle::platformLoadResourceSynchronously(NetworkingContext* contex
     response = client.response();
 }
 
+void ResourceHandle::continueDidReceiveResponse()
+{
+    notImplemented();
+}
+
+void ResourceHandle::continueWillSendRequest(ResourceRequest&& request)
+{
+    ASSERT(isMainThread());
+    ASSERT(!client() || client()->usesAsyncCallbacks());
+
+    if (d->m_delegate)
+        d->m_delegate->continueWillSendRequest(WTFMove(request));
+}
+
 } // namespace WebCore
 
 #endif
index 20b6d4b..fff0fef 100644 (file)
@@ -201,22 +201,9 @@ void ResourceHandleCurlDelegate::curlDidReceiveResponse(const CurlResponse& rece
             m_multipartHandle = std::make_unique<MultipartHandle>(m_handle, boundary);
     }
 
-    // HTTP redirection
-    if (response().isRedirection()) {
-        String location = response().httpHeaderField(HTTPHeaderName::Location);
-        if (!location.isEmpty()) {
-            URL newURL = URL(m_firstRequest.url(), location);
-
-            ResourceRequest redirectedRequest = m_firstRequest;
-            redirectedRequest.setURL(newURL);
-            ResourceResponse localResponse = response();
-            if (m_handle->client())
-                m_handle->client()->willSendRequest(m_handle, WTFMove(redirectedRequest), WTFMove(localResponse));
-
-            m_firstRequest.setURL(newURL);
-
-            return;
-        }
+    if (response().shouldRedirect()) {
+        willSendRequest();
+        return;
     }
 
     if (response().isUnauthorized()) {
@@ -289,6 +276,105 @@ void ResourceHandleCurlDelegate::curlDidFailWithError(const ResourceError& resou
     m_handle->client()->didFail(m_handle, resourceError);
 }
 
+bool ResourceHandleCurlDelegate::shouldRedirectAsGET(const ResourceRequest& request, bool crossOrigin)
+{
+    if ((request.httpMethod() == "GET") || (request.httpMethod() == "HEAD"))
+        return false;
+
+    if (!request.url().protocolIsInHTTPFamily())
+        return true;
+
+    if (response().isSeeOther())
+        return true;
+
+    if ((response().isMovedPermanently() || response().isFound()) && (request.httpMethod() == "POST"))
+        return true;
+
+    if (crossOrigin && (request.httpMethod() == "DELETE"))
+        return true;
+
+    return false;
+}
+
+void ResourceHandleCurlDelegate::willSendRequest()
+{
+    ASSERT(isMainThread());
+
+    static const int maxRedirects = 20;
+
+    if (m_redirectCount++ > maxRedirects) {
+        m_handle->client()->didFail(m_handle, ResourceError::httpError(CURLE_TOO_MANY_REDIRECTS, response().url()));
+        return;
+    }
+
+    String location = response().httpHeaderField(HTTPHeaderName::Location);
+    URL newURL = URL(m_firstRequest.url(), location);
+    bool crossOrigin = !protocolHostAndPortAreEqual(m_firstRequest.url(), newURL);
+
+    ResourceRequest newRequest = m_firstRequest;
+    newRequest.setURL(newURL);
+
+    if (shouldRedirectAsGET(newRequest, crossOrigin)) {
+        newRequest.setHTTPMethod("GET");
+        newRequest.setHTTPBody(nullptr);
+        newRequest.clearHTTPContentType();
+    }
+
+    // Should not set Referer after a redirect from a secure resource to non-secure one.
+    if (!newURL.protocolIs("https") && protocolIs(newRequest.httpReferrer(), "https") && m_handle->context()->shouldClearReferrerOnHTTPSToHTTPRedirect())
+        newRequest.clearHTTPReferrer();
+
+    m_user = newURL.user();
+    m_pass = newURL.pass();
+    newRequest.removeCredentials();
+
+    if (crossOrigin) {
+        // If the network layer carries over authentication headers from the original request
+        // in a cross-origin redirect, we want to clear those headers here. 
+        newRequest.clearHTTPAuthorization();
+        newRequest.clearHTTPOrigin();
+    }
+
+    ResourceResponse responseCopy = response();
+    if (m_handle->client()->usesAsyncCallbacks())
+        m_handle->client()->willSendRequestAsync(m_handle, WTFMove(newRequest), WTFMove(responseCopy));
+    else {
+        auto request = m_handle->client()->willSendRequest(m_handle, WTFMove(newRequest), WTFMove(responseCopy));
+        continueAfterWillSendRequest(WTFMove(request));
+    }
+}
+
+void ResourceHandleCurlDelegate::continueWillSendRequest(ResourceRequest&& request)
+{
+    ASSERT(isMainThread());
+
+    continueAfterWillSendRequest(WTFMove(request));
+}
+
+void ResourceHandleCurlDelegate::continueAfterWillSendRequest(ResourceRequest&& request)
+{
+    ASSERT(isMainThread());
+
+    // willSendRequest might cancel the load.
+    if (cancelledOrClientless() || !m_curlRequest)
+        return;
+
+    m_currentRequest = WTFMove(request);
+
+    bool isSyncRequest = m_curlRequest->isSyncRequest();
+    m_curlRequest->cancel();
+    m_curlRequest->setDelegate(nullptr);
+
+    m_curlRequest = createCurlRequest(m_currentRequest);
+
+    if (protocolHostAndPortAreEqual(m_currentRequest.url(), response().url())) {
+        auto credential = getCredential(m_currentRequest, true);
+        m_curlRequest->setUserPass(credential.first, credential.second);
+    }
+
+    m_curlRequest->start(isSyncRequest);
+}
+
 ResourceResponse& ResourceHandleCurlDelegate::response()
 {
     return m_handle->getInternal()->m_response;
index c252590..7364a54 100644 (file)
@@ -55,6 +55,8 @@ public:
 
     void dispatchSynchronousJob();
 
+    void continueWillSendRequest(ResourceRequest&&);
+
 private:
     // Called from main thread.
     ResourceResponse& response();
@@ -69,13 +71,18 @@ private:
     void curlDidComplete() override;
     void curlDidFailWithError(const ResourceError&) override;
 
+    bool shouldRedirectAsGET(const ResourceRequest&, bool crossOrigin);
+    void willSendRequest();
+    void continueAfterWillSendRequest(ResourceRequest&&);
+
     void handleDataURL();
 
     // Used by main thread.
     ResourceHandle* m_handle;
     std::unique_ptr<MultipartHandle> m_multipartHandle;
     unsigned m_authFailureCount { 0 };
-    // Used by worker thread.
+    int m_redirectCount { 0 };
+
     ResourceRequest m_firstRequest;
     ResourceRequest m_currentRequest;
     bool m_shouldUseCredentialStorage;
index 3ab7080..e2568c0 100644 (file)
@@ -52,7 +52,10 @@ public:
 
     void setDeprecatedNetworkLoadMetrics(const NetworkLoadMetrics& networkLoadMetrics) { m_networkLoadMetrics = networkLoadMetrics; }
 
-    bool isRedirection() const;
+    bool shouldRedirect();
+    bool isMovedPermanently() const;
+    bool isFound() const;
+    bool isSeeOther() const;
     bool isNotModified() const;
     bool isUnauthorized() const;
 
index 9bb3cff..89b76d4 100644 (file)
@@ -133,10 +133,35 @@ String ResourceResponse::platformSuggestedFilename() const
     return filenameFromHTTPContentDisposition(httpHeaderField(HTTPHeaderName::ContentDisposition));
 }
 
-bool ResourceResponse::isRedirection() const
+bool ResourceResponse::shouldRedirect()
 {
     auto statusCode = httpStatusCode();
-    return (300 <= statusCode) && (statusCode < 400) && (statusCode != 304);
+    if ((statusCode < 300) || (400 <= statusCode))
+        return false;
+
+    // Some 3xx status codes aren't actually redirects.
+    if (statusCode == 300 || statusCode == 304 || statusCode == 305 || statusCode == 306)
+        return false;
+
+    if (httpHeaderField(HTTPHeaderName::Location).isEmpty())
+        return false;
+
+    return true;
+}
+
+bool ResourceResponse::isMovedPermanently() const
+{
+    return (httpStatusCode() == 301);
+}
+
+bool ResourceResponse::isFound() const
+{
+    return (httpStatusCode() == 302);
+}
+
+bool ResourceResponse::isSeeOther() const
+{
+    return (httpStatusCode() == 303);
 }
 
 bool ResourceResponse::isNotModified() const