Attachment 356651 Details for Bug 192375 – Patch

[patch] Patch

bug-192375-20181205132945.patch (text/plain), 8.46 KB, created by Vivek Seth on 2018-12-05 13:29:48 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Vivek Seth

Created: 2018-12-05 13:29:48 PST

Size: 8.46 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..37a197287162d6b60aaf2b516264b8984d0f9a7d 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -193,6 +193,9 @@ auto NetworkLoadChecker::accessControlErrorForValidationHandler(String&& message
> #if ENABLE(HTTPS_UPGRADE)
> bool NetworkLoadChecker::applyHTTPSUpgradeIfNeeded(ResourceRequest& request)
> {
>+    if (request.requester() != ResourceRequest::Requester::Main)
>+        return false;
>+
>     // Use dummy list for now.
>     static NeverDestroyed<HashSet<String>> upgradableHosts = std::initializer_list<String> {
>         "www.bbc.com"_s, // (source: https://whynohttps.com)
>@@ -220,13 +223,16 @@ bool NetworkLoadChecker::applyHTTPSUpgradeIfNeeded(ResourceRequest& request)
> 
> void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurityPolicyClient* client, ValidationHandler&& handler)
> {
>+    ResourceRequest originalRequest = request;
> 
> #if ENABLE(HTTPS_UPGRADE)
>-    if (request.requester() == ResourceRequest::Requester::Main) {
>-        if (applyHTTPSUpgradeIfNeeded(request))
>+    // Only upgade if there is a way to notify parent.
>+    if (m_didUpdateRequestURL) {
>+        if (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 +247,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](auto result) mutable {
>         if (!result.has_value()) {
>             ASSERT(result.error().isCancellation());
>             handler(makeUnexpected(WTFMove(result.error())));
>@@ -251,6 +257,16 @@ void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurity
>             handler(this->accessControlErrorForValidationHandler("Blocked by content extension"_s));
>             return;
>         }
>+
>+        bool didModifyRequest = (result.value().request != originalRequest);
>+        if (m_didUpdateRequestURL && didModifyRequest) {
>+            auto currentRequest = result.value().request;
>+            m_didUpdateRequestURL(originalRequest, currentRequest, [this, request = WTFMove(result.value().request), handler = WTFMove(handler)]() mutable {
>+                this->continueCheckingRequest(WTFMove(request), WTFMove(handler));
>+            });
>+            return;
>+        }
>+
>         this->continueCheckingRequest(WTFMove(result.value().request), WTFMove(handler));
>     });
> #else
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>index 6bde17ccd65b1caf3bda6934a28c9773c0927b06..c884228221a46f267224d6a3255679fb5cf3978a 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -84,6 +84,8 @@ public:
> 
>     void enableContentExtensionsCheck() { m_checkContentExtensions = true; }
> 
>+    void setDidUpdateRequestURL(WTF::Function<void(WebCore::ResourceRequest&, WebCore::ResourceRequest&, WTF::Function<void(void)>&&)>&& didUpdateRequestURL) { m_didUpdateRequestURL = WTFMove(didUpdateRequestURL); };
>+
> private:
>     WebCore::ContentSecurityPolicy* contentSecurityPolicy();
>     bool isChecking() const { return !!m_corsPreflightChecker; }
>@@ -139,6 +141,8 @@ private:
>     bool m_shouldCaptureExtraNetworkLoadMetrics { false };
>     WebCore::NetworkLoadInformation m_loadInformation;
> 
>+    WTF::Function<void(WebCore::ResourceRequest&, WebCore::ResourceRequest&, WTF::Function<void(void)>&&)> m_didUpdateRequestURL;
>+
> #if ENABLE(HTTPS_UPGRADE)
>     static bool applyHTTPSUpgradeIfNeeded(WebCore::ResourceRequest&);
> #endif // ENABLE(HTTPS_UPGRADE)
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index 82a85501d2c5c67f95e1341ceb71907edeb42f19..e904a15ebcdf589267c25ca4822f9bc1a67f98ba 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -120,6 +120,20 @@ NetworkResourceLoader::NetworkResourceLoader(NetworkResourceLoadParameters&& par
> 
>     if (synchronousReply || parameters.shouldRestrictHTTPResponseAccess) {
>         m_networkLoadChecker = std::make_unique<NetworkLoadChecker>(FetchOptions { m_parameters.options }, m_parameters.sessionID, m_parameters.webPageID, m_parameters.webFrameID, HTTPHeaderMap { m_parameters.originalRequestHeaders }, URL { m_parameters.request.url() }, m_parameters.sourceOrigin.copyRef(), m_parameters.preflightPolicy, originalRequest().httpReferrer(), shouldCaptureExtraNetworkLoadMetrics());
>+        m_networkLoadChecker->setDidUpdateRequestURL([this](ResourceRequest& originalRequest, ResourceRequest& currentRequest, WTF::Function<void(void)>&& callback) {
>+            ResourceResponse redirectResponse;
>+            redirectResponse.setURL(originalRequest.url());
>+            redirectResponse.setHTTPStatusCode(302);
>+            redirectResponse.setHTTPVersion("HTTP/1.1"_s);
>+            redirectResponse.setHTTPHeaderField(HTTPHeaderName::Location, currentRequest.url().string());
>+            redirectResponse.setHTTPHeaderField(HTTPHeaderName::CacheControl, "no-store"_s);
>+
>+            RELEASE_LOG_IF_ALLOWED("NetworkResourceLoader: m_didUpdateRequestURL called and simulated redirect sent");
>+
>+            willSendRedirectedRequest(WTFMove(originalRequest), WTFMove(currentRequest), WTFMove(redirectResponse));
>+            m_didSendSimulatedRedirectCallback = WTFMove(callback);
>+        });
>+
>         if (m_parameters.cspResponseHeaders)
>             m_networkLoadChecker->setCSPResponseHeaders(ContentSecurityPolicyResponseHeaders { m_parameters.cspResponseHeaders.value() });
> #if ENABLE(CONTENT_EXTENSIONS)
>@@ -704,6 +718,12 @@ void NetworkResourceLoader::restartNetworkLoad(WebCore::ResourceRequest&& newReq
> 
> void NetworkResourceLoader::continueWillSendRequest(ResourceRequest&& newRequest, bool isAllowedToAskUserForCredentials)
> {
>+    auto didSendSimulatedRedirectCallback = std::exchange(m_didSendSimulatedRedirectCallback, nullptr);
>+    if (didSendSimulatedRedirectCallback) {
>+        didSendSimulatedRedirectCallback();
>+        return;
>+    }
>+
>     if (m_shouldRestartLoad) {
>         m_shouldRestartLoad = false;
>         restartNetworkLoad(WTFMove(newRequest));
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.h b/Source/WebKit/NetworkProcess/NetworkResourceLoader.h
>index c2d5d776b73ffb7601dfe58ff1649348cfe8a84e..9cbb0c182ef41008efa2f0b98fce2ec1820c98ad 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.h
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.h
>@@ -212,6 +212,8 @@ private:
>     ResponseCompletionHandler m_responseCompletionHandler;
> 
>     std::optional<NetworkActivityTracker> m_networkActivityTracker;
>+
>+    WTF::Function<void(void)> m_didSendSimulatedRedirectCallback;
> };
> 
> } // namespace WebKit

Actions: View | Formatted Diff | Diff

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