Attachment 356844 Details for Bug 192375 – Patch

[patch] Patch

bug-192375-20181207152551.patch (text/plain), 9.57 KB, created by Vivek Seth on 2018-12-07 15:25:53 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Vivek Seth

Created: 2018-12-07 15:25:53 PST

Size: 9.57 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..7758c38d71faa0fcd8fe9fba4b50cf03dfb2b725 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -191,8 +191,11 @@ auto NetworkLoadChecker::accessControlErrorForValidationHandler(String&& message
> }
> 
> #if ENABLE(HTTPS_UPGRADE)
>-bool NetworkLoadChecker::applyHTTPSUpgradeIfNeeded(ResourceRequest& request)
>+bool NetworkLoadChecker::applyHTTPSUpgradeIfNeeded(ResourceRequest& request) const
> {
>+    if (m_requestLoadType != LoadType::MainFrame)
>+        return false;
>+
>     // Use dummy list for now.
>     static NeverDestroyed<HashSet<String>> upgradableHosts = std::initializer_list<String> {
>         "www.bbc.com"_s, // (source: https://whynohttps.com)
>@@ -213,20 +216,27 @@ 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)
> {
>+    ResourceRequest originalRequest = request;
>+
>+    auto continueCheckingRequestOrAllowSimulatedRedirect = [this](ResourceRequest&& originalRequest, ResourceRequest&& currentRequest, ValidationHandler&& handler) {
>+        // If main frame load and request has been modified, allow NetworkResourceLoader to trigger a redirect.
>+        if ((m_requestLoadType != LoadType::MainFrame) && (currentRequest.url() != originalRequest.url())) {
>+            handler(WTFMove(currentRequest));
>+            return;
>+        }
>+        this->continueCheckingRequest(WTFMove(currentRequest), WTFMove(handler));
>+    };
> 
> #if ENABLE(HTTPS_UPGRADE)
>-    if (request.requester() == ResourceRequest::Requester::Main) {
>-        if (applyHTTPSUpgradeIfNeeded(request))
>-            ASSERT(request.url().protocolIs("https"));
>+    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 +251,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), continueCheckingRequestOrAllowSimulatedRedirect = WTFMove(continueCheckingRequestOrAllowSimulatedRedirect)](auto result) mutable {
>         if (!result.has_value()) {
>             ASSERT(result.error().isCancellation());
>             handler(makeUnexpected(WTFMove(result.error())));
>@@ -251,10 +261,11 @@ void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurity
>             handler(this->accessControlErrorForValidationHandler("Blocked by content extension"_s));
>             return;
>         }
>-        this->continueCheckingRequest(WTFMove(result.value().request), WTFMove(handler));
>+
>+        continueCheckingRequestOrAllowSimulatedRedirect(WTFMove(originalRequest), WTFMove(result.value().request), WTFMove(handler));
>     });
> #else
>-    continueCheckingRequest(WTFMove(request), WTFMove(handler));
>+    continueCheckingRequestOrAllowSimulatedRedirect(WTFMove(originalRequest), WTFMove(request), WTFMove(handler));
> #endif
> }
> 
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>index 6bde17ccd65b1caf3bda6934a28c9773c0927b06..2b6e0fbd64e3daebeba2a242a0e091bea81e8686 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -50,6 +50,7 @@ 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&&);
>@@ -84,6 +85,8 @@ public:
> 
>     void enableContentExtensionsCheck() { m_checkContentExtensions = true; }
> 
>+    void setRequestLoadType(LoadType requestLoadType) { m_requestLoadType = requestLoadType; }
>+
> private:
>     WebCore::ContentSecurityPolicy* contentSecurityPolicy();
>     bool isChecking() const { return !!m_corsPreflightChecker; }
>@@ -139,8 +142,10 @@ private:
>     bool m_shouldCaptureExtraNetworkLoadMetrics { false };
>     WebCore::NetworkLoadInformation m_loadInformation;
> 
>+    LoadType m_requestLoadType { LoadType::Other };
>+
> #if ENABLE(HTTPS_UPGRADE)
>-    static bool applyHTTPSUpgradeIfNeeded(WebCore::ResourceRequest&);
>+    bool applyHTTPSUpgradeIfNeeded(WebCore::ResourceRequest&) const;
> #endif // ENABLE(HTTPS_UPGRADE)
> 
> };
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index 82a85501d2c5c67f95e1341ceb71907edeb42f19..991d4941b15ab2d63263fe3f0fba7cf52fe242e9 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -122,6 +122,8 @@ NetworkResourceLoader::NetworkResourceLoader(NetworkResourceLoadParameters&& par
>         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());
>         if (m_parameters.cspResponseHeaders)
>             m_networkLoadChecker->setCSPResponseHeaders(ContentSecurityPolicyResponseHeaders { m_parameters.cspResponseHeaders.value() });
>+        if (isMainFrameLoad())
>+            m_networkLoadChecker->setRequestLoadType(NetworkLoadChecker::LoadType::MainFrame);
> #if ENABLE(CONTENT_EXTENSIONS)
>         m_networkLoadChecker->setContentExtensionController(URL { m_parameters.mainDocumentURL }, m_parameters.userContentControllerIdentifier);
> #endif
>@@ -171,6 +173,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());
>@@ -194,6 +208,16 @@ 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.");
>+
>+                m_isWaitingContinueWillSendRequestForCachedRedirect = true;
>+                willSendRedirectedRequest(ResourceRequest(originalRequest()), 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);

Actions: View | Formatted Diff | Diff

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