Attachment 356527 Details for Bug 192375 – Patch

[patch] Patch

bug-192375-20181204135053.patch (text/plain), 9.52 KB, created by Vivek Seth on 2018-12-04 13:50:57 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Vivek Seth

Created: 2018-12-04 13:50:57 PST

Size: 9.52 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/NetworkLoad.cpp b/Source/WebKit/NetworkProcess/NetworkLoad.cpp
>index 19fe849163793bafa9f3e08a9217cef0d5e38a10..a91c3b3f620a1e8a365202b1bc76b15515f07a20 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoad.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoad.cpp
>@@ -114,7 +114,10 @@ void NetworkLoad::continueWillSendRequest(WebCore::ResourceRequest&& newRequest)
> #endif
> 
>     auto redirectCompletionHandler = std::exchange(m_redirectCompletionHandler, nullptr);
>-    ASSERT(redirectCompletionHandler);
>+
>+    if (!redirectCompletionHandler)
>+        return;
>+
>     if (m_currentRequest.isNull()) {
>         NetworkLoadMetrics emptyMetrics;
>         didCompleteWithError(cancelledError(m_currentRequest), emptyMetrics);
>@@ -123,8 +126,7 @@ void NetworkLoad::continueWillSendRequest(WebCore::ResourceRequest&& newRequest)
>         return;
>     }
> 
>-    if (redirectCompletionHandler)
>-        redirectCompletionHandler(ResourceRequest(m_currentRequest));
>+    redirectCompletionHandler(ResourceRequest(m_currentRequest));
> }
> 
> bool NetworkLoad::shouldCaptureExtraNetworkLoadMetrics() const
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoad.h b/Source/WebKit/NetworkProcess/NetworkLoad.h
>index b53fdbdcf8353a300846bcf53614822587dc3bf4..81361119ff212d7404f2c0ac09273903bee6da1f 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoad.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoad.h
>@@ -65,11 +65,13 @@ public:
> 
>     String description() const;
> 
>+    // NetworkDataTaskClient
>+    void willPerformHTTPRedirection(WebCore::ResourceResponse&&, WebCore::ResourceRequest&&, RedirectCompletionHandler&&) final;
>+
> private:
>     void initialize(NetworkSession&);
> 
>     // NetworkDataTaskClient
>-    void willPerformHTTPRedirection(WebCore::ResourceResponse&&, WebCore::ResourceRequest&&, RedirectCompletionHandler&&) final;
>     void didReceiveChallenge(WebCore::AuthenticationChallenge&&, ChallengeCompletionHandler&&) final;
>     void didReceiveResponse(WebCore::ResourceResponse&&, ResponseCompletionHandler&&) final;
>     void didReceiveData(Ref<WebCore::SharedBuffer>&&) final;
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>index 99fb8a5c6e0ff0c2438b87c2d25834e82a558d10..87bf9f5f7ca1c4e2f5308552177daa3f61ee90c7 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -54,7 +54,13 @@ static inline bool isSameOrigin(const URL& url, const SecurityOrigin* origin)
> }
> 
> NetworkLoadChecker::NetworkLoadChecker(FetchOptions&& options, PAL::SessionID sessionID, uint64_t pageID, uint64_t frameID, HTTPHeaderMap&& originalRequestHeaders, URL&& url, RefPtr<SecurityOrigin>&& sourceOrigin, PreflightPolicy preflightPolicy, String&& referrer, bool shouldCaptureExtraNetworkLoadMetrics)
>-    : m_options(WTFMove(options))
>+#if ENABLE(HTTPS_UPGRADE)
>+    : didUpgradeCallback()
>+    ,
>+#else
>+    :
>+#endif // ENABLE(HTTPS_UPGRADE)
>+    m_options(WTFMove(options))
>     , m_sessionID(sessionID)
>     , m_pageID(pageID)
>     , m_frameID(frameID)
>@@ -222,11 +228,17 @@ void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurity
> {
> 
> #if ENABLE(HTTPS_UPGRADE)
>-    if (request.requester() == ResourceRequest::Requester::Main) {
>-        if (applyHTTPSUpgradeIfNeeded(request))
>+    bool didUpgradeRequest = false;
>+    ResourceRequest oldRequest = request;
>+
>+    // Only upgade if there is a way to notify parent.
>+    if (didUpgradeCallback && request.requester() == ResourceRequest::Requester::Main) {
>+        didUpgradeRequest = applyHTTPSUpgradeIfNeeded(request);
>+        if (didUpgradeRequest) {
>             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 +253,12 @@ 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)
>+#if ENABLE(HTTPS_UPGRADE)
>+    , oldRequest, request, didUpgradeRequest
>+#endif // ENABLE(HTTPS_UPGRADE)
>+    ](auto result) mutable {
>+
>         if (!result.has_value()) {
>             ASSERT(result.error().isCancellation());
>             handler(makeUnexpected(WTFMove(result.error())));
>@@ -251,6 +268,16 @@ void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurity
>             handler(this->accessControlErrorForValidationHandler("Blocked by content extension"_s));
>             return;
>         }
>+
>+#if ENABLE(HTTPS_UPGRADE)
>+        if (didUpgradeCallback && didUpgradeRequest) {
>+            didUpgradeCallback(oldRequest, request, [this, request = WTFMove(result.value().request), handler = WTFMove(handler)]() mutable {
>+                this->continueCheckingRequest(WTFMove(request), WTFMove(handler));
>+            });
>+            return;
>+        }
>+#endif // ENABLE(HTTPS_UPGRADE)
>+
>         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..710a697155ba1e503e9c27342a40a8822e608685 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -84,6 +84,10 @@ public:
> 
>     void enableContentExtensionsCheck() { m_checkContentExtensions = true; }
> 
>+#if ENABLE(HTTPS_UPGRADE)
>+    WTF::Function<void(WebCore::ResourceRequest, WebCore::ResourceRequest, WTF::Function<void(void)>)> didUpgradeCallback;
>+#endif // ENABLE(HTTPS_UPGRADE)
>+
> private:
>     WebCore::ContentSecurityPolicy* contentSecurityPolicy();
>     bool isChecking() const { return !!m_corsPreflightChecker; }
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index 82a85501d2c5c67f95e1341ceb71907edeb42f19..c31fdc7856dac78861309a9d12b4553070439eb4 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -120,6 +120,21 @@ 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());
>+#if ENABLE(HTTPS_UPGRADE)
>+        m_networkLoadChecker->didUpgradeCallback = [this](ResourceRequest originalRequest, ResourceRequest currentRequest, WTF::Function<void(void)> callback) {
>+            ResourceResponse redirectResponse { };
>+            redirectResponse.setURL(originalRequest.url());
>+            redirectResponse.setHTTPStatusCode(302);
>+            redirectResponse.setHTTPVersion("HTTP/1.1");
>+            redirectResponse.setHTTPHeaderField(String("Location"), currentRequest.url().string());
>+            redirectResponse.setHTTPHeaderField(String("Cache-Control"), String("no-store"));
>+
>+            RELEASE_LOG_IF_ALLOWED("NetworkResourceLoader: didUpgradeCallback called and simulated redirect sent");
>+
>+            willSendRedirectedRequest(WTFMove(originalRequest), WTFMove(currentRequest), WTFMove(redirectResponse));
>+            callback();
>+        };
>+#endif // ENABLE(HTTPS_UPGRADE)
>         if (m_parameters.cspResponseHeaders)
>             m_networkLoadChecker->setCSPResponseHeaders(ContentSecurityPolicyResponseHeaders { m_parameters.cspResponseHeaders.value() });
> #if ENABLE(CONTENT_EXTENSIONS)
>@@ -345,6 +360,10 @@ void NetworkResourceLoader::cleanup(LoadResult result)
> 
>     m_networkLoad = nullptr;
> 
>+#if ENABLE(HTTPS_UPGRADE)
>+    m_networkLoadChecker->didUpgradeCallback = nullptr;
>+#endif // ENABLE(HTTPS_UPGRADE)
>+
>     // This will cause NetworkResourceLoader to be destroyed and therefore we do it last.
>     m_connection->didCleanupResourceLoader(*this);
> }

Actions: View | Formatted Diff | Diff

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