Attachment 356764 Details for Bug 192375 – Patch

[patch] Patch

bug-192375-20181206164206.patch (text/plain), 14.16 KB, created by Vivek Seth on 2018-12-06 16:42:08 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Vivek Seth

Created: 2018-12-06 16:42:08 PST

Size: 14.16 KB

patch

obsolete

>Subversion Revision: 238859
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 99e917573d2bdd4e977c1ff1e214e0158ec16463..2f6d13b563e53c70a627a8579b6018f296cb4480 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-12-04  Vivek Seth  <v_seth@apple.com>
>+
>+        HTTPS Upgrade: Figure out if/how to tell clients that the HTTPS upgrade happened
>+        https://bugs.webkit.org/show_bug.cgi?id=192375
>+        <rdar://problem/45851159>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Use simulated redirect to tell clients that HTTPS Upgrade happened.
>+
>+        * NetworkProcess/NetworkLoad.cpp:
>+        (WebKit::NetworkLoad::continueWillSendRequest):
>+        * NetworkProcess/NetworkLoad.h:
>+        * NetworkProcess/NetworkLoadChecker.cpp:
>+        (WebKit::NetworkLoadChecker::NetworkLoadChecker):
>+        (WebKit::NetworkLoadChecker::checkRequest):
>+        * NetworkProcess/NetworkLoadChecker.h:
>+        * NetworkProcess/NetworkResourceLoader.cpp:
>+        (WebKit::NetworkResourceLoader::cleanup):
>+
> 2018-12-04  Youenn Fablet  <youenn@apple.com>
> 
>         Device orientation may be wrong on page reload after crash
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>index 99fb8a5c6e0ff0c2438b87c2d25834e82a558d10..09075c407257584699644d69260d793a29a8631a 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -81,7 +81,7 @@ NetworkLoadChecker::NetworkLoadChecker(FetchOptions&& options, PAL::SessionID se
> 
> NetworkLoadChecker::~NetworkLoadChecker() = default;
> 
>-void NetworkLoadChecker::check(ResourceRequest&& request, ContentSecurityPolicyClient* client, ValidationHandler&& handler)
>+void NetworkLoadChecker::check(ResourceRequest&& request, ContentSecurityPolicyClient* client, LoadType loadType, ValidationHandler&& handler)
> {
>     ASSERT(!isChecking());
> 
>@@ -95,7 +95,7 @@ void NetworkLoadChecker::check(ResourceRequest&& request, ContentSecurityPolicyC
>         m_dntHeaderValue = "1";
>         request.setHTTPHeaderField(HTTPHeaderName::DNT, m_dntHeaderValue);
>     }
>-    checkRequest(WTFMove(request), client, WTFMove(handler));
>+    checkRequest(WTFMove(request), client, loadType, WTFMove(handler));
> }
> 
> void NetworkLoadChecker::prepareRedirectedRequest(ResourceRequest& request)
>@@ -109,7 +109,7 @@ static inline NetworkLoadChecker::RedirectionRequestOrError redirectionError(con
>     return makeUnexpected(ResourceError { String { }, 0, redirectResponse.url(), WTFMove(errorMessage), ResourceError::Type::AccessControl });
> }
> 
>-void NetworkLoadChecker::checkRedirection(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse, ContentSecurityPolicyClient* client, RedirectionValidationHandler&& handler)
>+void NetworkLoadChecker::checkRedirection(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse, ContentSecurityPolicyClient* client, LoadType loadType, RedirectionValidationHandler&& handler)
> {
>     ASSERT(!isChecking());
> 
>@@ -139,7 +139,7 @@ void NetworkLoadChecker::checkRedirection(ResourceRequest&& request, ResourceReq
>     m_previousURL = WTFMove(m_url);
>     m_url = redirectRequest.url();
> 
>-    checkRequest(WTFMove(redirectRequest), client, [handler = WTFMove(handler), request = WTFMove(request), redirectResponse = WTFMove(redirectResponse)](auto&& result) mutable {
>+    checkRequest(WTFMove(redirectRequest), client, loadType, [handler = WTFMove(handler), request = WTFMove(request), redirectResponse = WTFMove(redirectResponse)](auto&& result) mutable {
>         if (!result.has_value()) {
>             handler(makeUnexpected(WTFMove(result.error())));
>             return;
>@@ -213,20 +213,18 @@ bool NetworkLoadChecker::applyHTTPSUpgradeIfNeeded(ResourceRequest& request)
>     newURL.setProtocol("https"_s);
>     request.setURL(newURL);
>     return true;
>-
>-    return false;
> }
> #endif // ENABLE(HTTPS_UPGRADE)
> 
>-void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurityPolicyClient* client, ValidationHandler&& handler)
>+void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurityPolicyClient* client, LoadType loadType, ValidationHandler&& handler)
> {
>+    ResourceRequest originalRequest = request;
> 
> #if ENABLE(HTTPS_UPGRADE)
>-    if (request.requester() == ResourceRequest::Requester::Main) {
>-        if (applyHTTPSUpgradeIfNeeded(request))
>-            ASSERT(request.url().protocolIs("https"));
>+    if ((loadType == LoadType::MainFrame) && applyHTTPSUpgradeIfNeeded(request)) {
>+        ASSERT(request.url().protocolIs("https"));
>+        RELEASE_LOG_IF_ALLOWED("checkRequest - Upgrade URL from HTTP to HTTPS");
>     }
>-
> #endif // ENABLE(HTTPS_UPGRADE)
> 
>     if (auto* contentSecurityPolicy = this->contentSecurityPolicy()) {
>@@ -241,7 +239,7 @@ void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurity
>     }
> 
> #if ENABLE(CONTENT_EXTENSIONS)
>-    processContentExtensionRulesForLoad(WTFMove(request), [this, handler = WTFMove(handler)](auto result) mutable {
>+    processContentExtensionRulesForLoad(WTFMove(request), [this, handler = WTFMove(handler), originalRequest = WTFMove(originalRequest)](auto result) mutable {
>         if (!result.has_value()) {
>             ASSERT(result.error().isCancellation());
>             handler(makeUnexpected(WTFMove(result.error())));
>@@ -251,9 +249,21 @@ void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurity
>             handler(this->accessControlErrorForValidationHandler("Blocked by content extension"_s));
>             return;
>         }
>+
>+        // If request has been modified, NetworkResourceLoader will trigger a redirect.
>+        if (result.value().request.url() != originalRequest.url()) {
>+            handler(WTFMove(result.value().request));
>+            return;
>+        }
>+
>         this->continueCheckingRequest(WTFMove(result.value().request), WTFMove(handler));
>     });
> #else
>+    // If request has been modified, NetworkResourceLoader will trigger a redirect.
>+    if (request.url() != originalRequest.url()) {
>+        handler(WTFMove(request));
>+        return;
>+    }
>     continueCheckingRequest(WTFMove(request), WTFMove(handler));
> #endif
> }
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>index 6bde17ccd65b1caf3bda6934a28c9773c0927b06..6bf80efb093fa610ae3330d097226062e5e55f56 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -50,9 +50,10 @@ public:
>     NetworkLoadChecker(WebCore::FetchOptions&&, PAL::SessionID, uint64_t pageID, uint64_t frameID, WebCore::HTTPHeaderMap&&, URL&&, RefPtr<WebCore::SecurityOrigin>&&, WebCore::PreflightPolicy, String&& referrer, bool shouldCaptureExtraNetworkLoadMetrics = false);
>     ~NetworkLoadChecker();
> 
>+    enum class LoadType { MainFrame, Other };
>     using RequestOrError = Expected<WebCore::ResourceRequest, WebCore::ResourceError>;
>     using ValidationHandler = CompletionHandler<void(RequestOrError&&)>;
>-    void check(WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, ValidationHandler&&);
>+    void check(WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, LoadType, ValidationHandler&&);
> 
>     struct RedirectionTriplet {
>         WebCore::ResourceRequest request;
>@@ -61,7 +62,7 @@ public:
>     };
>     using RedirectionRequestOrError = Expected<RedirectionTriplet, WebCore::ResourceError>;
>     using RedirectionValidationHandler = CompletionHandler<void(RedirectionRequestOrError&&)>;
>-    void checkRedirection(WebCore::ResourceRequest&& request, WebCore::ResourceRequest&& redirectRequest, WebCore::ResourceResponse&& redirectResponse, WebCore::ContentSecurityPolicyClient*, RedirectionValidationHandler&&);
>+    void checkRedirection(WebCore::ResourceRequest&& request, WebCore::ResourceRequest&& redirectRequest, WebCore::ResourceResponse&& redirectResponse, WebCore::ContentSecurityPolicyClient*, LoadType, RedirectionValidationHandler&&);
> 
>     void prepareRedirectedRequest(WebCore::ResourceRequest&);
> 
>@@ -89,7 +90,7 @@ private:
>     bool isChecking() const { return !!m_corsPreflightChecker; }
>     bool isRedirected() const { return m_redirectCount; }
> 
>-    void checkRequest(WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, ValidationHandler&&);
>+    void checkRequest(WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, LoadType, ValidationHandler&&);
> 
>     bool isAllowedByContentSecurityPolicy(const WebCore::ResourceRequest&, WebCore::ContentSecurityPolicyClient*);
> 
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index 82a85501d2c5c67f95e1341ceb71907edeb42f19..125ecffa461eed1381a30b4d575ce8b7826939cb 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -171,6 +171,18 @@ bool NetworkResourceLoader::isSynchronous() const
>     return !!m_synchronousLoadData;
> }
> 
>+static ResourceResponse simulatedRedirectResponse(const URL& fromURL, const URL& toURL)
>+{
>+    ResourceResponse redirectResponse;
>+    redirectResponse.setURL(fromURL);
>+    redirectResponse.setHTTPStatusCode(302);
>+    redirectResponse.setHTTPVersion("HTTP/1.1"_s);
>+    redirectResponse.setHTTPHeaderField(HTTPHeaderName::Location, toURL.string());
>+    redirectResponse.setHTTPHeaderField(HTTPHeaderName::CacheControl, "no-store"_s);
>+
>+    return redirectResponse;
>+}
>+
> void NetworkResourceLoader::start()
> {
>     ASSERT(RunLoop::isMain());
>@@ -186,7 +198,11 @@ void NetworkResourceLoader::start()
>     m_wasStarted = true;
> 
>     if (m_networkLoadChecker) {
>-        m_networkLoadChecker->check(ResourceRequest { originalRequest() }, this, [this] (auto&& result) {
>+        NetworkLoadChecker::LoadType requestLoadType = NetworkLoadChecker::LoadType::Other;
>+        if (isMainFrameLoad())
>+            requestLoadType = NetworkLoadChecker::LoadType::MainFrame;
>+
>+        m_networkLoadChecker->check(ResourceRequest { originalRequest() }, this, requestLoadType, [this] (auto&& result) {
>             if (!result.has_value()) {
>                 if (!result.error().isCancellation())
>                     this->didFailLoading(result.error());
>@@ -194,6 +210,17 @@ void NetworkResourceLoader::start()
>             }
> 
>             auto currentRequest = result.value();
>+
>+            if (isMainFrameLoad() && (originalRequest().url() != currentRequest.url())) {
>+                ResourceResponse redirectResponse = simulatedRedirectResponse(originalRequest().url(), currentRequest.url());
>+                RELEASE_LOG_IF_ALLOWED("NetworkResourceLoader: simulated redirect sent because request URL was modified.");
>+
>+                ResourceRequest originalRequestCopy = originalRequest();
>+                m_isWaitingContinueWillSendRequestForCachedRedirect = true;
>+                willSendRedirectedRequest(WTFMove(originalRequestCopy), WTFMove(currentRequest), WTFMove(redirectResponse));
>+                return;
>+            }
>+
>             if (this->canUseCache(currentRequest)) {
>                 RELEASE_LOG_IF_ALLOWED("start: Checking cache for resource (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier, this->isMainResource(), this->isSynchronous());
>                 this->retrieveCacheEntry(currentRequest);
>@@ -627,8 +654,12 @@ void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request,
>         m_cache->storeRedirect(request, redirectResponse, redirectRequest, maxAgeCap);
> 
>     if (m_networkLoadChecker) {
>+        NetworkLoadChecker::LoadType requestLoadType = NetworkLoadChecker::LoadType::Other;
>+        if (isMainFrameLoad())
>+            requestLoadType = NetworkLoadChecker::LoadType::MainFrame;
>+
>         m_networkLoadChecker->storeRedirectionIfNeeded(request, redirectResponse);
>-        m_networkLoadChecker->checkRedirection(WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse), this, [protectedThis = makeRef(*this), this, storedCredentialsPolicy = m_networkLoadChecker->storedCredentialsPolicy()](auto&& result) mutable {
>+        m_networkLoadChecker->checkRedirection(WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse), this, requestLoadType, [protectedThis = makeRef(*this), this, storedCredentialsPolicy = m_networkLoadChecker->storedCredentialsPolicy()](auto&& result) mutable {
>             if (!result.has_value()) {
>                 if (result.error().isCancellation())
>                     return;
>diff --git a/Source/WebKit/NetworkProcess/PingLoad.cpp b/Source/WebKit/NetworkProcess/PingLoad.cpp
>index 983a42ac8bc25fcaa94f0b0c915d6fae020ef1c3..4bf3864931c837cc84b3b06e385de116984df314 100644
>--- a/Source/WebKit/NetworkProcess/PingLoad.cpp
>+++ b/Source/WebKit/NetworkProcess/PingLoad.cpp
>@@ -56,7 +56,7 @@ PingLoad::PingLoad(NetworkResourceLoadParameters&& parameters, WTF::CompletionHa
>     // Set a very generous timeout, just in case.
>     m_timeoutTimer.startOneShot(60000_s);
> 
>-    m_networkLoadChecker->check(ResourceRequest { m_parameters.request }, nullptr, [this] (auto&& result) {
>+    m_networkLoadChecker->check(ResourceRequest { m_parameters.request }, nullptr, NetworkLoadChecker::LoadType::Other, [this] (auto&& result) {
>         if (!result.has_value()) {
>             this->didFinish(result.error());
>             return;
>@@ -94,7 +94,7 @@ void PingLoad::loadRequest(ResourceRequest&& request)
> 
> void PingLoad::willPerformHTTPRedirection(ResourceResponse&& redirectResponse, ResourceRequest&& request, RedirectCompletionHandler&& completionHandler)
> {
>-    m_networkLoadChecker->checkRedirection(ResourceRequest { }, WTFMove(request), WTFMove(redirectResponse), nullptr, [this, completionHandler = WTFMove(completionHandler)] (auto&& result) mutable {
>+    m_networkLoadChecker->checkRedirection(ResourceRequest { }, WTFMove(request), WTFMove(redirectResponse), nullptr, NetworkLoadChecker::LoadType::Other, [this, completionHandler = WTFMove(completionHandler)] (auto&& result) mutable {
>         if (!result.has_value()) {
>             completionHandler({ });
>             this->didFinish(result.error());

Actions: View | Formatted Diff | Diff

Attachments on bug 192375: 356527 | 356651 | 356764 | 356838 | 356844 | 356935 | 356937 | 356978 | 357008 | 357009 | 357066 | 357078 | 357091 | 357094 | 357163