Attachment 357008 Details for Bug 192375 – Patch

[patch] Patch

bug-192375-20181210153817.patch (text/plain), 18.98 KB, created by Vivek Seth on 2018-12-10 15:38:18 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Vivek Seth

Created: 2018-12-10 15:38:18 PST

Size: 18.98 KB

patch

obsolete

>Subversion Revision: 238859
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 99e917573d2bdd4e977c1ff1e214e0158ec16463..9d1fb2b5adcf8e5fb78824e1c3cd7efb24f6749e 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,28 @@
>+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/NetworkLoadChecker.cpp:
>+        (WebKit::NetworkLoadChecker::NetworkLoadChecker):
>+        (WebKit::NetworkLoadChecker::checkRedirection):
>+        (WebKit::NetworkLoadChecker::accessControlErrorForValidationHandler):
>+        (WebKit::NetworkLoadChecker::applyHTTPSUpgradeIfNeeded const):
>+        (WebKit::NetworkLoadChecker::checkRequest):
>+        (WebKit::simulatedRedirectResponse):
>+        (WebKit::NetworkLoadChecker::continueCheckingRequestOrDoSimulatedRedirect):
>+        (WebKit::NetworkLoadChecker::checkCORSRequestWithPreflight):
>+        (WebKit::NetworkLoadChecker::applyHTTPSUpgradeIfNeeded): Deleted.
>+        * NetworkProcess/NetworkLoadChecker.h:
>+        * NetworkProcess/NetworkResourceLoader.cpp:
>+        (WebKit::NetworkResourceLoader::start):
>+        * NetworkProcess/PingLoad.cpp:
>+
> 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..a51b768ec4310bae0dce3b57eb96928b62b144c7 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -53,7 +53,7 @@ static inline bool isSameOrigin(const URL& url, const SecurityOrigin* origin)
>     return url.protocolIsData() || url.protocolIsBlob() || !origin || origin->canRequest(url);
> }
> 
>-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)
>+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, LoadType requestLoadType)
>     : m_options(WTFMove(options))
>     , m_sessionID(sessionID)
>     , m_pageID(pageID)
>@@ -64,6 +64,7 @@ NetworkLoadChecker::NetworkLoadChecker(FetchOptions&& options, PAL::SessionID se
>     , m_preflightPolicy(preflightPolicy)
>     , m_referrer(WTFMove(referrer))
>     , m_shouldCaptureExtraNetworkLoadMetrics(shouldCaptureExtraNetworkLoadMetrics)
>+    , m_requestLoadType(requestLoadType)
> {
>     m_isSameOriginRequest = isSameOrigin(m_url, m_origin.get());
>     switch (options.credentials) {
>@@ -140,11 +141,18 @@ void NetworkLoadChecker::checkRedirection(ResourceRequest&& request, ResourceReq
>     m_url = redirectRequest.url();
> 
>     checkRequest(WTFMove(redirectRequest), client, [handler = WTFMove(handler), request = WTFMove(request), redirectResponse = WTFMove(redirectResponse)](auto&& result) mutable {
>-        if (!result.has_value()) {
>-            handler(makeUnexpected(WTFMove(result.error())));
>-            return;
>-        }
>-        handler(RedirectionTriplet { WTFMove(request), WTFMove(result.value()), WTFMove(redirectResponse) });
>+        WTF::switchOn(result,
>+            [&handler] (ResourceError& error) mutable {
>+                handler(makeUnexpected(WTFMove(error)));
>+            },
>+            [&handler, &request, &redirectResponse] (RedirectionTriplet& triplet) mutable {
>+                // FIXME: if checkRequest returns a RedirectionTriplet, it means the requested URL has changed and we should update the redirectResponse to match.
>+                handler(RedirectionTriplet { WTFMove(request), WTFMove(triplet.redirectRequest), WTFMove(redirectResponse) });
>+            },
>+            [&handler, &request, &redirectResponse] (ResourceRequest& redirectRequest) mutable {
>+                handler(RedirectionTriplet { WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse) });
>+            }
>+        );
>     });
> }
> 
>@@ -185,14 +193,17 @@ ResourceError NetworkLoadChecker::validateResponse(ResourceResponse& response)
>     return { };
> }
> 
>-auto NetworkLoadChecker::accessControlErrorForValidationHandler(String&& message) -> RequestOrError
>+auto NetworkLoadChecker::accessControlErrorForValidationHandler(String&& message) -> RequestOrRedirectionTripletOrError
> {
>-    return makeUnexpected(ResourceError { String { }, 0, m_url, WTFMove(message), ResourceError::Type::AccessControl });
>+    return ResourceError { String { }, 0, m_url, WTFMove(message), ResourceError::Type::AccessControl };
> }
> 
> #if ENABLE(HTTPS_UPGRADE)
>-bool NetworkLoadChecker::applyHTTPSUpgradeIfNeeded(ResourceRequest& request)
>+void NetworkLoadChecker::applyHTTPSUpgradeIfNeeded(ResourceRequest& request) const
> {
>+    if (m_requestLoadType != LoadType::MainFrame)
>+        return;
>+
>     // Use dummy list for now.
>     static NeverDestroyed<HashSet<String>> upgradableHosts = std::initializer_list<String> {
>         "www.bbc.com"_s, // (source: https://whynohttps.com)
>@@ -204,29 +215,25 @@ bool NetworkLoadChecker::applyHTTPSUpgradeIfNeeded(ResourceRequest& request)
> 
>     // Only upgrade http urls.
>     if (!url.protocolIs("http"))
>-        return false;
>+        return;
> 
>     if (!upgradableHosts.get().contains(url.host().toString()))
>-        return false;
>+        return;
> 
>     auto newURL = url;
>     newURL.setProtocol("https"_s);
>     request.setURL(newURL);
>-    return true;
> 
>-    return false;
>+    RELEASE_LOG_IF_ALLOWED("applyHTTPSUpgradeIfNeeded - Upgrade URL from HTTP to HTTPS");
> }
> #endif // ENABLE(HTTPS_UPGRADE)
> 
> 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))
>-            ASSERT(request.url().protocolIs("https"));
>-    }
>-
>+    applyHTTPSUpgradeIfNeeded(request);
> #endif // ENABLE(HTTPS_UPGRADE)
> 
>     if (auto* contentSecurityPolicy = this->contentSecurityPolicy()) {
>@@ -241,23 +248,47 @@ 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())));
>+            handler(WTFMove(result.error()));
>             return;
>         }
>         if (result.value().status.blockedLoad) {
>             handler(this->accessControlErrorForValidationHandler("Blocked by content extension"_s));
>             return;
>         }
>-        this->continueCheckingRequest(WTFMove(result.value().request), WTFMove(handler));
>+
>+        continueCheckingRequestOrDoSimulatedRedirect(WTFMove(originalRequest), WTFMove(result.value().request), WTFMove(handler));
>     });
> #else
>-    continueCheckingRequest(WTFMove(request), WTFMove(handler));
>+    continueCheckingRequestOrDoSimulatedRedirect(WTFMove(originalRequest), WTFMove(request), WTFMove(handler));
> #endif
> }
> 
>+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 NetworkLoadChecker::continueCheckingRequestOrDoSimulatedRedirect(ResourceRequest&& originalRequest, ResourceRequest&& currentRequest, ValidationHandler&& handler)
>+{
>+    // If main frame load and request has been modified, trigger a simulated redirect.
>+    if (m_requestLoadType == LoadType::MainFrame && currentRequest.url() != originalRequest.url()) {
>+        ResourceResponse redirectResponse = simulatedRedirectResponse(originalRequest.url(), currentRequest.url());
>+        handler(RedirectionTriplet { WTFMove(originalRequest), WTFMove(currentRequest), WTFMove(redirectResponse) });
>+        return;
>+    }
>+    this->continueCheckingRequest(WTFMove(currentRequest), WTFMove(handler));
>+}
>+
> bool NetworkLoadChecker::isAllowedByContentSecurityPolicy(const ResourceRequest& request, WebCore::ContentSecurityPolicyClient* client)
> {
>     auto* contentSecurityPolicy = this->contentSecurityPolicy();
>@@ -402,7 +433,7 @@ void NetworkLoadChecker::checkCORSRequestWithPreflight(ResourceRequest&& request
>         RELEASE_LOG_IF_ALLOWED("checkCORSRequestWithPreflight - makeCrossOriginAccessRequestWithPreflight preflight complete, success: %d forRedirect? %d", error.isNull(), isRedirected);
> 
>         if (!error.isNull()) {
>-            handler(makeUnexpected(WTFMove(error)));
>+            handler(WTFMove(error));
>             return;
>         }
> 
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>index 6bde17ccd65b1caf3bda6934a28c9773c0927b06..80966014a68c4f0609062661231c906d0449fc54 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -33,6 +33,7 @@
> #include <WebCore/SecurityPolicyViolationEvent.h>
> #include <wtf/CompletionHandler.h>
> #include <wtf/Expected.h>
>+#include <wtf/Variant.h>
> #include <wtf/WeakPtr.h>
> 
> namespace WebCore {
>@@ -47,18 +48,21 @@ class NetworkCORSPreflightChecker;
> 
> class NetworkLoadChecker : public CanMakeWeakPtr<NetworkLoadChecker> {
> 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&&);
>+    NetworkLoadChecker(WebCore::FetchOptions&&, PAL::SessionID, uint64_t pageID, uint64_t frameID, WebCore::HTTPHeaderMap&&, URL&&, RefPtr<WebCore::SecurityOrigin>&&, WebCore::PreflightPolicy, String&& referrer, bool shouldCaptureExtraNetworkLoadMetrics = false, LoadType requestLoadType = LoadType::Other);
>+    ~NetworkLoadChecker();
> 
>     struct RedirectionTriplet {
>         WebCore::ResourceRequest request;
>         WebCore::ResourceRequest redirectRequest;
>         WebCore::ResourceResponse redirectResponse;
>     };
>+
>+    using RequestOrRedirectionTripletOrError = Variant<WebCore::ResourceRequest, RedirectionTriplet, WebCore::ResourceError>;
>+    using ValidationHandler = CompletionHandler<void(RequestOrRedirectionTripletOrError&&)>;
>+    void check(WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, ValidationHandler&&);
>+
>     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&&);
>@@ -94,13 +98,14 @@ private:
>     bool isAllowedByContentSecurityPolicy(const WebCore::ResourceRequest&, WebCore::ContentSecurityPolicyClient*);
> 
>     void continueCheckingRequest(WebCore::ResourceRequest&&, ValidationHandler&&);
>+    void continueCheckingRequestOrDoSimulatedRedirect(WebCore::ResourceRequest&& originalRequest, WebCore::ResourceRequest&& currentRequest, ValidationHandler&&);
> 
>     bool doesNotNeedCORSCheck(const URL&) const;
>     void checkCORSRequest(WebCore::ResourceRequest&&, ValidationHandler&&);
>     void checkCORSRedirectedRequest(WebCore::ResourceRequest&&, ValidationHandler&&);
>     void checkCORSRequestWithPreflight(WebCore::ResourceRequest&&, ValidationHandler&&);
> 
>-    RequestOrError accessControlErrorForValidationHandler(String&&);
>+    RequestOrRedirectionTripletOrError accessControlErrorForValidationHandler(String&&);
> 
> #if ENABLE(CONTENT_EXTENSIONS)
>     struct ContentExtensionResult {
>@@ -111,6 +116,11 @@ private:
>     using ContentExtensionCallback = CompletionHandler<void(ContentExtensionResultOrError)>;
>     void processContentExtensionRulesForLoad(WebCore::ResourceRequest&&, ContentExtensionCallback&&);
> #endif
>+
>+#if ENABLE(HTTPS_UPGRADE)
>+    void applyHTTPSUpgradeIfNeeded(WebCore::ResourceRequest&) const;
>+#endif // ENABLE(HTTPS_UPGRADE)
>+
>     WebCore::FetchOptions m_options;
>     WebCore::StoredCredentialsPolicy m_storedCredentialsPolicy;
>     PAL::SessionID m_sessionID;
>@@ -139,10 +149,7 @@ private:
>     bool m_shouldCaptureExtraNetworkLoadMetrics { false };
>     WebCore::NetworkLoadInformation m_loadInformation;
> 
>-#if ENABLE(HTTPS_UPGRADE)
>-    static bool applyHTTPSUpgradeIfNeeded(WebCore::ResourceRequest&);
>-#endif // ENABLE(HTTPS_UPGRADE)
>-
>+    LoadType m_requestLoadType;
> };
> 
> }
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index 82a85501d2c5c67f95e1341ceb71907edeb42f19..d7dccaa590f50731c6272f58aad0dcf2945cfa7f 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -119,7 +119,8 @@ 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());
>+        NetworkLoadChecker::LoadType requestLoadType = isMainFrameLoad() ? NetworkLoadChecker::LoadType::MainFrame : NetworkLoadChecker::LoadType::Other;
>+        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(), requestLoadType);
>         if (m_parameters.cspResponseHeaders)
>             m_networkLoadChecker->setCSPResponseHeaders(ContentSecurityPolicyResponseHeaders { m_parameters.cspResponseHeaders.value() });
> #if ENABLE(CONTENT_EXTENSIONS)
>@@ -187,20 +188,26 @@ void NetworkResourceLoader::start()
> 
>     if (m_networkLoadChecker) {
>         m_networkLoadChecker->check(ResourceRequest { originalRequest() }, this, [this] (auto&& result) {
>-            if (!result.has_value()) {
>-                if (!result.error().isCancellation())
>-                    this->didFailLoading(result.error());
>-                return;
>-            }
>-
>-            auto currentRequest = result.value();
>-            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);
>-                return;
>-            }
>-
>-            this->startNetworkLoad(WTFMove(result.value()), FirstLoad::Yes);
>+            WTF::switchOn(result,
>+                [this] (ResourceError& error) {
>+                    if (!error.isCancellation())
>+                        this->didFailLoading(error);
>+                },
>+                [this] (NetworkLoadChecker::RedirectionTriplet& triplet) {
>+                    this->m_isWaitingContinueWillSendRequestForCachedRedirect = true;
>+                    willSendRedirectedRequest(WTFMove(triplet.request), WTFMove(triplet.redirectRequest), WTFMove(triplet.redirectResponse));
>+                    RELEASE_LOG_IF_ALLOWED("NetworkResourceLoader: simulated redirect sent because request URL was modified.");
>+                },
>+                [this] (ResourceRequest& request) {
>+                    if (this->canUseCache(request)) {
>+                        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(request);
>+                        return;
>+                    }
>+
>+                    this->startNetworkLoad(WTFMove(request), FirstLoad::Yes);
>+                }
>+            );
>         });
>         return;
>     }
>diff --git a/Source/WebKit/NetworkProcess/PingLoad.cpp b/Source/WebKit/NetworkProcess/PingLoad.cpp
>index 983a42ac8bc25fcaa94f0b0c915d6fae020ef1c3..f50464a3a68bc39db1880441462c27e7516dc41f 100644
>--- a/Source/WebKit/NetworkProcess/PingLoad.cpp
>+++ b/Source/WebKit/NetworkProcess/PingLoad.cpp
>@@ -57,11 +57,17 @@ PingLoad::PingLoad(NetworkResourceLoadParameters&& parameters, WTF::CompletionHa
>     m_timeoutTimer.startOneShot(60000_s);
> 
>     m_networkLoadChecker->check(ResourceRequest { m_parameters.request }, nullptr, [this] (auto&& result) {
>-        if (!result.has_value()) {
>-            this->didFinish(result.error());
>-            return;
>-        }
>-        this->loadRequest(WTFMove(result.value()));
>+        WTF::switchOn(result,
>+            [this] (ResourceError& error) {
>+                this->didFinish(error);
>+            },
>+            [this] (NetworkLoadChecker::RedirectionTriplet& triplet) {
>+                this->loadRequest(WTFMove(triplet.redirectRequest));
>+            },
>+            [this] (ResourceRequest& request) {
>+                this->loadRequest(WTFMove(request));
>+            }
>+        );
>     });
> }
>

Actions: View | Formatted Diff | Diff

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