Attachment 357009 Details for Bug 192375 – Patch

[patch] Patch

bug-192375-20181210154458.patch (text/plain), 18.95 KB, created by Vivek Seth on 2018-12-10 15:44:59 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Vivek Seth

Created: 2018-12-10 15:44:59 PST

Size: 18.95 KB

patch

obsolete

>Subversion Revision: 239056
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 59a34486c9b03fc31db93f5b89b935344863f587..e3f8e958e881f0263d2a446c0f3d19f29584675b 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-12-10  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-10  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r239023.
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>index 288d695280da529009ebb22baef707689c005935..d1ef7c8d5cb07e4149a9cda2c23c616e8a7cf529 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -45,7 +45,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)
>@@ -56,6 +56,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) {
>@@ -132,11 +133,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) });
>+            }
>+        );
>     });
> }
> 
>@@ -177,14 +185,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)
>@@ -196,29 +207,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()) {
>@@ -233,23 +240,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();
>@@ -394,7 +425,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 99d8b7f3e83b54104fb9493eb5b20961c34a4ee3..445fb6871b784c03b2cb188d78a3c5ff0d17eb33 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -30,6 +30,7 @@
> #include <WebCore/ResourceError.h>
> #include <wtf/CompletionHandler.h>
> #include <wtf/Expected.h>
>+#include <wtf/Variant.h>
> 
> namespace WebCore {
> class ContentSecurityPolicy;
>@@ -42,18 +43,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&&);
>@@ -89,13 +93,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 {
>@@ -106,6 +111,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;
>@@ -134,10 +144,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 f7cf027e37a70fd17c406a4d170fe96030a89ffd..0f010003fc2f55b6f6efafe48bb08d05b8d6808b 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -111,7 +111,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)
>@@ -179,20 +180,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 f8e91eda4c9478c0015d5b296dbcb37c9627ae10..e71cbf4a3a587ebfb17b32f0324894e0f0d4c949 100644
>--- a/Source/WebKit/NetworkProcess/PingLoad.cpp
>+++ b/Source/WebKit/NetworkProcess/PingLoad.cpp
>@@ -57,11 +57,17 @@ PingLoad::PingLoad(NetworkResourceLoadParameters&& parameters, CompletionHandler
>     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