Attachment 356937 Details for Bug 192375 – Patch

[patch] Patch

bug-192375-20181209180844.patch (text/plain), 18.06 KB, created by Vivek Seth on 2018-12-09 18:08:45 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Vivek Seth

Created: 2018-12-09 18:08:45 PST

Size: 18.06 KB

patch

obsolete

>Subversion Revision: 238859
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 99e917573d2bdd4e977c1ff1e214e0158ec16463..9ed831464740a39974cf6ad217ae77e172cb14bd 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+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::applyHTTPSUpgradeIfNeeded const):
>+        (WebKit::NetworkLoadChecker::checkRequest):
>+        (WebKit::simulatedRedirectResponse):
>+        (WebKit::NetworkLoadChecker::continueCheckingRequestOrDoSimulatedRedirect):
>+        (WebKit::NetworkLoadChecker::continueCheckingRequest):
>+        (WebKit::NetworkLoadChecker::checkCORSRequest):
>+        (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..6f891dea5af5db31ea0b34be0e9d03e9c6124bdb 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) {
>@@ -139,7 +140,7 @@ void NetworkLoadChecker::checkRedirection(ResourceRequest&& request, ResourceReq
>     m_previousURL = WTFMove(m_url);
>     m_url = redirectRequest.url();
> 
>-    checkRequest(WTFMove(redirectRequest), client, [handler = WTFMove(handler), request = WTFMove(request), redirectResponse = WTFMove(redirectResponse)](auto&& result) mutable {
>+    checkRequest(WTFMove(redirectRequest), client, [handler = WTFMove(handler), request = WTFMove(request), redirectResponse = WTFMove(redirectResponse)](auto&& result, std::optional<WebCore::ResourceResponse>&& response) mutable {
>         if (!result.has_value()) {
>             handler(makeUnexpected(WTFMove(result.error())));
>             return;
>@@ -191,8 +192,11 @@ auto NetworkLoadChecker::accessControlErrorForValidationHandler(String&& message
> }
> 
> #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 +208,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()) {
>@@ -235,29 +235,53 @@ void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ContentSecurity
>             contentSecurityPolicy->upgradeInsecureRequestIfNeeded(request, type);
>         }
>         if (!isAllowedByContentSecurityPolicy(request, client)) {
>-            handler(accessControlErrorForValidationHandler("Blocked by Content Security Policy."_s));
>+            handler(accessControlErrorForValidationHandler("Blocked by Content Security Policy."_s), std::nullopt);
>             return;
>         }
>     }
> 
> #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(makeUnexpected(WTFMove(result.error())), std::nullopt);
>             return;
>         }
>         if (result.value().status.blockedLoad) {
>-            handler(this->accessControlErrorForValidationHandler("Blocked by content extension"_s));
>+            handler(this->accessControlErrorForValidationHandler("Blocked by content extension"_s), std::nullopt);
>             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(WTFMove(currentRequest), redirectResponse);
>+        return;
>+    }
>+    this->continueCheckingRequest(WTFMove(currentRequest), WTFMove(handler));
>+}
>+
> bool NetworkLoadChecker::isAllowedByContentSecurityPolicy(const ResourceRequest& request, WebCore::ContentSecurityPolicyClient* client)
> {
>     auto* contentSecurityPolicy = this->contentSecurityPolicy();
>@@ -306,13 +330,13 @@ void NetworkLoadChecker::continueCheckingRequest(ResourceRequest&& request, Vali
>     m_isSameOriginRequest = m_isSameOriginRequest && isSameOrigin(request.url(), m_origin.get());
> 
>     if (doesNotNeedCORSCheck(request.url())) {
>-        handler(WTFMove(request));
>+        handler(WTFMove(request), std::nullopt);
>         return;
>     }
> 
>     if (m_options.mode == FetchOptions::Mode::SameOrigin) {
>         String message = makeString("Unsafe attempt to load URL ", request.url().stringCenterEllipsizedToLength(), " from origin ", m_origin->toString(), ". Domains, protocols and ports must match.\n");
>-        handler(accessControlErrorForValidationHandler(WTFMove(message)));
>+        handler(accessControlErrorForValidationHandler(WTFMove(message)), std::nullopt);
>         return;
>     }
> 
>@@ -342,7 +366,7 @@ void NetworkLoadChecker::checkCORSRequest(ResourceRequest&& request, ValidationH
>         FALLTHROUGH;
>     case PreflightPolicy::Prevent:
>         updateRequestForAccessControl(request, *m_origin, m_storedCredentialsPolicy);
>-        handler(WTFMove(request));
>+        handler(WTFMove(request), std::nullopt);
>         break;
>     }
> }
>@@ -381,7 +405,7 @@ void NetworkLoadChecker::checkCORSRequestWithPreflight(ResourceRequest&& request
>     if (CrossOriginPreflightResultCache::singleton().canSkipPreflight(m_origin->toString(), request.url(), m_storedCredentialsPolicy, request.httpMethod(), m_originalRequestHeaders)) {
>         RELEASE_LOG_IF_ALLOWED("checkCORSRequestWithPreflight - preflight can be skipped thanks to cached result");
>         updateRequestForAccessControl(request, *m_origin, m_storedCredentialsPolicy);
>-        handler(WTFMove(request));
>+        handler(WTFMove(request), std::nullopt);
>         return;
>     }
> 
>@@ -402,7 +426,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(makeUnexpected(WTFMove(error)), std::nullopt);
>             return;
>         }
> 
>@@ -411,7 +435,7 @@ void NetworkLoadChecker::checkCORSRequestWithPreflight(ResourceRequest&& request
> 
>         auto corsPreflightChecker = WTFMove(m_corsPreflightChecker);
>         updateRequestForAccessControl(request, *m_origin, m_storedCredentialsPolicy);
>-        handler(WTFMove(request));
>+        handler(WTFMove(request), std::nullopt);
>     });
>     m_corsPreflightChecker->startPreflight();
> }
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>index 6bde17ccd65b1caf3bda6934a28c9773c0927b06..e6b719f94749116286c7a384d8bffd2c59c0d1bc 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -47,11 +47,13 @@ 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);
>+    enum class LoadType { MainFrame, Other };
>+
>+    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();
> 
>     using RequestOrError = Expected<WebCore::ResourceRequest, WebCore::ResourceError>;
>-    using ValidationHandler = CompletionHandler<void(RequestOrError&&)>;
>+    using ValidationHandler = CompletionHandler<void(RequestOrError&&, std::optional<WebCore::ResourceResponse>&&)>;
>     void check(WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, ValidationHandler&&);
> 
>     struct RedirectionTriplet {
>@@ -94,6 +96,7 @@ 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&&);
>@@ -111,6 +114,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 +147,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..29e46bb5b1b88711ec283347b20ff2706ae0bb3f 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)
>@@ -186,7 +187,7 @@ void NetworkResourceLoader::start()
>     m_wasStarted = true;
> 
>     if (m_networkLoadChecker) {
>-        m_networkLoadChecker->check(ResourceRequest { originalRequest() }, this, [this] (auto&& result) {
>+        m_networkLoadChecker->check(ResourceRequest { originalRequest() }, this, [this] (auto&& result, std::optional<WebCore::ResourceResponse>&& response) {
>             if (!result.has_value()) {
>                 if (!result.error().isCancellation())
>                     this->didFailLoading(result.error());
>@@ -194,6 +195,16 @@ void NetworkResourceLoader::start()
>             }
> 
>             auto currentRequest = result.value();
>+
>+            if (response.has_value()) {
>+                m_isWaitingContinueWillSendRequestForCachedRedirect = true;
>+                willSendRedirectedRequest(ResourceRequest(originalRequest()), WTFMove(currentRequest), WTFMove(response.value()));
>+
>+                RELEASE_LOG_IF_ALLOWED("NetworkResourceLoader: simulated redirect sent because request URL was modified.");
>+
>+                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);
>diff --git a/Source/WebKit/NetworkProcess/PingLoad.cpp b/Source/WebKit/NetworkProcess/PingLoad.cpp
>index 983a42ac8bc25fcaa94f0b0c915d6fae020ef1c3..289703a31f4f01ee3e0aa18e26c1b50f6b4739cb 100644
>--- a/Source/WebKit/NetworkProcess/PingLoad.cpp
>+++ b/Source/WebKit/NetworkProcess/PingLoad.cpp
>@@ -56,7 +56,7 @@ PingLoad::PingLoad(NetworkResourceLoadParameters&& parameters, WTF::CompletionHa
>     // Set a very generous timeout, just in case.
>     m_timeoutTimer.startOneShot(60000_s);
> 
>-    m_networkLoadChecker->check(ResourceRequest { m_parameters.request }, nullptr, [this] (auto&& result) {
>+    m_networkLoadChecker->check(ResourceRequest { m_parameters.request }, nullptr, [this] (auto&& result, std::optional<WebCore::ResourceResponse>&& response) {
>         if (!result.has_value()) {
>             this->didFinish(result.error());
>             return;

Actions: View | Formatted Diff | Diff

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