Changeset 222728 in webkit

Timestamp:

Oct 2, 2017 12:00:17 PM (7 years ago)

Author:

commit-queue@webkit.org

Message:

[CURL] Should handle redirects in WebCore
​https://bugs.webkit.org/show_bug.cgi?id=21242

Patch by Basuke Suzuki <Basuke Suzuki> 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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/network/ResourceHandle.cpp (modified) (1 diff)
• platform/network/curl/CurlContext.cpp (modified) (1 diff)
• platform/network/curl/CurlContext.h (modified) (1 diff)
• platform/network/curl/CurlRequest.cpp (modified) (3 diffs)
• platform/network/curl/ResourceHandleCurl.cpp (modified) (2 diffs)
• platform/network/curl/ResourceHandleCurlDelegate.cpp (modified) (2 diffs)
• platform/network/curl/ResourceHandleCurlDelegate.h (modified) (3 diffs)
• platform/network/curl/ResourceResponse.h (modified) (1 diff)
• platform/network/curl/ResourceResponseCurl.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/platform/network/ResourceHandle.cpp

@@ -171 +171 @@
 }
 
-#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&&)

trunk/Source/WebCore/platform/network/curl/CurlContext.cpp

@@ -426 +426 @@
 }
 
-void CurlHandle::enableAutoReferer()
-{
-    curl_easy_setopt(m_handle, CURLOPT_AUTOREFERER, 1L);
-}
-
 void CurlHandle::enableHttpAuthentication(long option)
 {

trunk/Source/WebCore/platform/network/curl/CurlContext.h

@@ -249 +249 @@
 
     void enableFollowLocation();
-    void enableAutoReferer();
 
     void enableHttpAuthentication(long);

trunk/Source/WebCore/platform/network/curl/CurlRequest.cpp

@@ -170 +170 @@
     m_curlHandle->enableTimeout();
 
-    m_curlHandle->enableAutoReferer();
-    m_curlHandle->enableFollowLocation();
     m_curlHandle->enableProxyIfExists();
     m_curlHandle->enableCookieJarIfExists();
@@ -276 +274 @@
     }
 
-    // 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;
 
@@ -308 +302 @@
 
     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) {

trunk/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp

@@ -66 +66 @@
         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();
@@ -251 +258 @@
 }
 
+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
 

trunk/Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.cpp

@@ -202 +202 @@
     }
 
-    // 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;
     }
 
@@ -288 +275 @@
     CurlCacheManager::getInstance().didFail(*m_handle);
     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);
 }
 

trunk/Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.h

@@ -56 +56 @@
     void dispatchSynchronousJob();
 
+    void continueWillSendRequest(ResourceRequest&&);
+
 private:
     // Called from main thread.
@@ -70 +72 @@
     void curlDidFailWithError(const ResourceError&) override;
 
+    bool shouldRedirectAsGET(const ResourceRequest&, bool crossOrigin);
+    void willSendRequest();
+    void continueAfterWillSendRequest(ResourceRequest&&);
+
     void handleDataURL();
 
@@ -76 +82 @@
     std::unique_ptr<MultipartHandle> m_multipartHandle;
     unsigned m_authFailureCount { 0 };
-    // Used by worker thread.
+    int m_redirectCount { 0 };
+
     ResourceRequest m_firstRequest;
     ResourceRequest m_currentRequest;

trunk/Source/WebCore/platform/network/curl/ResourceResponse.h

@@ -53 +53 @@
     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;

trunk/Source/WebCore/platform/network/curl/ResourceResponseCurl.cpp

@@ -134 +134 @@
 }
 
-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);
 }