Attachment 372113 Details for Bug 198854 – Patch

[patch] Patch

bug-198854-20190614010351.patch (text/plain), 17.97 KB, created by Sihui Liu on 2019-06-14 01:03:52 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-06-14 01:03:52 PDT

Size: 17.97 KB

patch

obsolete

>Subversion Revision: 246416
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c577e5d7eed6d24a42c159436da205a995a8397a..bb766c6a3caac093a1c09f20d3bedf828072232e 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-06-14  Sihui Liu  <sihui_liu@apple.com>
>+
>+        -[WKWebsiteDataStore removeDataOfTypes:modifiedSince:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
>+        https://bugs.webkit.org/show_bug.cgi?id=198854
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: WKWebsiteDataStore.RemoveAllPersistentCredentials
>+
>+        * platform/network/CredentialStorage.h:
>+        * platform/network/mac/CredentialStorageMac.mm:
>+        (WebCore::CredentialStorage::getOriginsWithPersistentCredentials):
>+        (WebCore::CredentialStorage::removePersistentCredentialsWithOrigins):
>+        (WebCore::CredentialStorage::clearPersistentCredentials):
>+        (WebCore::CredentialStorage::originsWithPersistentCredentials): Deleted.
>+
> 2019-06-13  Jer Noble  <jer.noble@apple.com>
> 
>         AVKit play button bypasses MediaElementSession playbackPermitted() checks.
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index b1068f7ee4217268165a048419577308adf51c8c..f16ba0b7375f476c129cdf900532775cd58a209f 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,31 @@
>+2019-06-14  Sihui Liu  <sihui_liu@apple.com>
>+
>+        -[WKWebsiteDataStore removeDataOfTypes:modifiedSince:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
>+        https://bugs.webkit.org/show_bug.cgi?id=198854
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Clear persistent credentials in deleteWebsiteData of network process.
>+
>+        Also, merge originsWithPersistentCredentials and removeCredentialsWithOrigins into fetchWebsiteData and
>+        deleteWebsiteData, and move credentials handling to WebCore.
>+
>+        * NetworkProcess/NetworkProcess.cpp:
>+        (WebKit::NetworkProcess::fetchWebsiteData):
>+        (WebKit::NetworkProcess::deleteWebsiteData):
>+        (WebKit::NetworkProcess::deleteWebsiteDataForOrigins):
>+        (WebKit::NetworkProcess::originsWithPersistentCredentials): Deleted.
>+        (WebKit::NetworkProcess::removeCredentialsWithOrigins): Deleted.
>+        * NetworkProcess/NetworkProcess.h:
>+        * NetworkProcess/NetworkProcess.messages.in:
>+        * NetworkProcess/cocoa/NetworkProcessCocoa.mm:
>+        (WebKit::NetworkProcess::originsWithPersistentCredentials): Deleted.
>+        (WebKit::NetworkProcess::removeCredentialsWithOrigins): Deleted.
>+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
>+        (WebKit::WebsiteDataStore::fetchDataAndApply):
>+        (WebKit::computeWebProcessAccessTypeForDataRemoval):
>+        (WebKit::WebsiteDataStore::removeData):
>+
> 2019-06-13  Tim Horton  <timothy_horton@apple.com>
> 
>         No content shows up for WKWebView while locked even with _canShowWhileLocked is YES
>diff --git a/Source/WebCore/platform/network/CredentialStorage.h b/Source/WebCore/platform/network/CredentialStorage.h
>index f8c75ae929ab2d5425a24a6267cd42738d75c0d7..810956dc5f281000b0fe9cf7c767d4c2373f4fe0 100644
>--- a/Source/WebCore/platform/network/CredentialStorage.h
>+++ b/Source/WebCore/platform/network/CredentialStorage.h
>@@ -47,7 +47,9 @@ public:
> 
>     // OS persistent storage.
>     WEBCORE_EXPORT static Credential getFromPersistentStorage(const ProtectionSpace&);
>-    WEBCORE_EXPORT static Vector<SecurityOriginData> originsWithPersistentCredentials();
>+    WEBCORE_EXPORT static Vector<WebCore::SecurityOriginData> getOriginsWithPersistentCredentials();
>+    WEBCORE_EXPORT static void removePersistentCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>& origins);
>+    WEBCORE_EXPORT static void clearPersistentCredentials();
> 
>     WEBCORE_EXPORT void clearCredentials();
> 
>diff --git a/Source/WebCore/platform/network/mac/CredentialStorageMac.mm b/Source/WebCore/platform/network/mac/CredentialStorageMac.mm
>index 65b51511f9a9b2b8b12f63edae36fef35e76d39c..0fc0f38eabdcf2beccc2edcec6f303c5b063f0c9 100644
>--- a/Source/WebCore/platform/network/mac/CredentialStorageMac.mm
>+++ b/Source/WebCore/platform/network/mac/CredentialStorageMac.mm
>@@ -38,7 +38,7 @@ Credential CredentialStorage::getFromPersistentStorage(const ProtectionSpace& pr
>     return credential ? Credential(credential) : Credential();
> }
> 
>-Vector<WebCore::SecurityOriginData> CredentialStorage::originsWithPersistentCredentials()
>+Vector<WebCore::SecurityOriginData> CredentialStorage::getOriginsWithPersistentCredentials()
> {
>     Vector<WebCore::SecurityOriginData> origins;
>     auto allCredentials = [[NSURLCredentialStorage sharedCredentialStorage] allCredentials];
>@@ -47,4 +47,28 @@ Vector<WebCore::SecurityOriginData> CredentialStorage::originsWithPersistentCred
>     return origins;
> }
> 
>+void CredentialStorage::removePersistentCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>& origins)
>+{
>+    for (auto& origin : origins) {
>+        auto allCredentials = [[NSURLCredentialStorage sharedCredentialStorage] allCredentials];
>+        for (NSURLProtectionSpace* space in allCredentials) {
>+            if (origin.protocol == String(space.protocol)
>+                && origin.host == String(space.host)
>+                && origin.port
>+                && *origin.port == space.port) {
>+                auto credentials = allCredentials[space];
>+                for (NSString* user in credentials) {
>+                    auto credential = credentials[user];
>+                    [[NSURLCredentialStorage sharedCredentialStorage] removeCredential:credential forProtectionSpace:space options:@{ NSURLCredentialStorageRemoveSynchronizableCredentials : @YES }];
>+                }
>+            }
>+        }
>+    }
>+}
>+
>+void CredentialStorage::clearPersistentCredentials()
>+{
>+    removePersistentCredentialsWithOrigins(getOriginsWithPersistentCredentials());
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>index dd9a1e3a8c5d76d2dd435303b7b8365ccae092e2..3e7274b3b628f49603f9bf369c3d21f1218a1d18 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>@@ -1282,6 +1282,11 @@ void NetworkProcess::fetchWebsiteData(PAL::SessionID sessionID, OptionSet<Websit
>             for (auto& securityOrigin : securityOrigins)
>                 callbackAggregator->m_websiteData.entries.append({ securityOrigin, WebsiteDataType::Credentials, 0 });
>         }
>+        if (!sessionID.isEphemeral()) {
>+            auto securityOrigins = WebCore::CredentialStorage::getOriginsWithPersistentCredentials();
>+            for (auto& securityOrigin : securityOrigins)
>+                callbackAggregator->m_websiteData.entries.append({ securityOrigin, WebsiteDataType::Credentials, 0 });
>+        }
>     }
> 
>     if (websiteDataTypes.contains(WebsiteDataType::DOMCache)) {
>@@ -1359,6 +1364,9 @@ void NetworkProcess::deleteWebsiteData(PAL::SessionID sessionID, OptionSet<Websi
>     if (websiteDataTypes.contains(WebsiteDataType::Credentials)) {
>         if (auto* session = storageSession(sessionID))
>             session->credentialStorage().clearCredentials();
>+        
>+        if (!sessionID.isEphemeral())
>+            WebCore::CredentialStorage::clearPersistentCredentials();
>     }
> 
>     auto clearTasksHandler = WTF::CallbackAggregator::create([this, callbackID] {
>@@ -1496,6 +1504,8 @@ void NetworkProcess::deleteWebsiteDataForOrigins(PAL::SessionID sessionID, Optio
>             for (auto& originData : originDatas)
>                 session->credentialStorage().removeCredentialsWithOrigin(originData);
>         }
>+        if (!sessionID.isEphemeral())
>+            WebCore::CredentialStorage::removePersistentCredentialsWithOrigins(originDatas);
>     }
> 
>     // FIXME: Implement storage quota clearing for these origins.
>@@ -2541,16 +2551,6 @@ StorageQuotaManager& NetworkProcess::storageQuotaManager(PAL::SessionID sessionI
> }
> 
> #if !PLATFORM(COCOA)
>-void NetworkProcess::originsWithPersistentCredentials(CompletionHandler<void(Vector<WebCore::SecurityOriginData>)>&& completionHandler)
>-{
>-    completionHandler(Vector<WebCore::SecurityOriginData>());
>-}
>-
>-void NetworkProcess::removeCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>&, CompletionHandler<void()>&& completionHandler)
>-{
>-    completionHandler();
>-}
>-
> void NetworkProcess::initializeProcess(const AuxiliaryProcessInitializationParameters&)
> {
> }
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.h b/Source/WebKit/NetworkProcess/NetworkProcess.h
>index 0e1dabd00ee4ca94e22b2d25e0c32fbbc4054fdd..b5d6f0c940880e67fc2dc95d5062b2517af4d601 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.h
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.h
>@@ -436,9 +436,6 @@ private:
> #endif
> 
>     void platformSyncAllCookies(CompletionHandler<void()>&&);
>-
>-    void originsWithPersistentCredentials(CompletionHandler<void(Vector<WebCore::SecurityOriginData>)>&&);
>-    void removeCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>& origins, CompletionHandler<void()>&&);
>     
>     void registerURLSchemeAsSecure(const String&) const;
>     void registerURLSchemeAsBypassingContentSecurityPolicy(const String&) const;
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.messages.in b/Source/WebKit/NetworkProcess/NetworkProcess.messages.in
>index 6d0624083f75f332726ab3fb21dc1d2cff90c21c..91fe69d57a2a36de246c42ab7d778f2bda3d978f 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.messages.in
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.messages.in
>@@ -168,7 +168,5 @@ messages -> NetworkProcess LegacyReceiver {
>     SetAdClickAttributionOverrideTimerForTesting(PAL::SessionID sessionID, bool value) -> () Async
>     SetAdClickAttributionConversionURLForTesting(PAL::SessionID sessionID, URL url) -> () Async
>     MarkAdClickAttributionsAsExpiredForTesting(PAL::SessionID sessionID) -> () Async
>-    OriginsWithPersistentCredentials() -> (Vector<WebCore::SecurityOriginData> origins) Async
>-    RemoveCredentialsWithOrigins(Vector<WebCore::SecurityOriginData> origins) -> () Async
>     GetLocalStorageOriginDetails(PAL::SessionID sessionID) -> (Vector<WebKit::LocalStorageDatabaseTracker::OriginDetails> details) Async
> }
>diff --git a/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm b/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>index bdefe9e4fa59006d5eb597bb7c05d361dd7918a6..5b52cabc0d997ec589a37a5089c9145b28aead3a 100644
>--- a/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>+++ b/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>@@ -212,31 +212,6 @@ void NetworkProcess::clearDiskCache(WallTime modifiedSince, CompletionHandler<vo
>     }).get());
> }
> 
>-void NetworkProcess::originsWithPersistentCredentials(CompletionHandler<void(Vector<WebCore::SecurityOriginData>)>&& completionHandler)
>-{
>-    completionHandler(WebCore::CredentialStorage::originsWithPersistentCredentials());
>-}
>-
>-void NetworkProcess::removeCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>& origins, CompletionHandler<void()>&& completionHandler)
>-{
>-    for (auto& origin : origins) {
>-        auto allCredentials = [[NSURLCredentialStorage sharedCredentialStorage] allCredentials];
>-        for (NSURLProtectionSpace* space in allCredentials) {
>-            if (origin.protocol == String(space.protocol)
>-                && origin.host == String(space.host)
>-                && origin.port
>-                && *origin.port == space.port) {
>-                auto credentials = allCredentials[space];
>-                for (NSString* user in credentials) {
>-                    auto credential = credentials[user];
>-                    [[NSURLCredentialStorage sharedCredentialStorage] removeCredential:credential forProtectionSpace:space];
>-                }
>-            }
>-        }
>-    }
>-    completionHandler();
>-}
>-
> #if PLATFORM(MAC)
> void NetworkProcess::setSharedHTTPCookieStorage(const Vector<uint8_t>& identifier)
> {
>diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>index c28a640f7773d40e2089094db68ca13ba76e3f52..c4d9758ac42a195bf9b8a210e50e7be58a5aad8e 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>@@ -512,24 +512,6 @@ void WebsiteDataStore::fetchDataAndApply(OptionSet<WebsiteDataType> dataTypes, O
>         });
>     }
> 
>-#if PLATFORM(COCOA)
>-    if (dataTypes.contains(WebsiteDataType::Credentials) && isPersistent()) {
>-        for (auto& processPool : processPools()) {
>-            if (!processPool->networkProcess())
>-                continue;
>-            
>-            callbackAggregator->addPendingCallback();
>-            WTF::CompletionHandler<void(Vector<WebCore::SecurityOriginData>&&)> completionHandler = [callbackAggregator](Vector<WebCore::SecurityOriginData>&& origins) mutable {
>-                WebsiteData websiteData;
>-                for (auto& origin : origins)
>-                    websiteData.entries.append(WebsiteData::Entry { origin, WebsiteDataType::Credentials, 0 });
>-                callbackAggregator->removePendingCallback(WTFMove(websiteData));
>-            };
>-            processPool->networkProcess()->sendWithAsyncReply(Messages::NetworkProcess::OriginsWithPersistentCredentials(), WTFMove(completionHandler));
>-        }
>-    }
>-#endif
>-
> #if ENABLE(NETSCAPE_PLUGIN_API)
>     if (dataTypes.contains(WebsiteDataType::PlugInData) && isPersistent()) {
>         class State {
>@@ -636,9 +618,6 @@ static ProcessAccessType computeWebProcessAccessTypeForDataRemoval(OptionSet<Web
>     if (dataTypes.contains(WebsiteDataType::MemoryCache))
>         processAccessType = std::max(processAccessType, ProcessAccessType::OnlyIfLaunched);
> 
>-    if (dataTypes.contains(WebsiteDataType::Credentials))
>-        processAccessType = std::max(processAccessType, ProcessAccessType::OnlyIfLaunched);
>-
>     return processAccessType;
> }
> 
>@@ -1085,19 +1064,6 @@ void WebsiteDataStore::removeData(OptionSet<WebsiteDataType> dataTypes, const Ve
>         });
>     }
> 
>-    if (dataTypes.contains(WebsiteDataType::Credentials) && isPersistent()) {
>-        for (auto& processPool : processPools()) {
>-            if (!processPool->networkProcess())
>-                continue;
>-            
>-            callbackAggregator->addPendingCallback();
>-            WTF::CompletionHandler<void()> completionHandler = [callbackAggregator]() mutable {
>-                callbackAggregator->removePendingCallback();
>-            };
>-            processPool->networkProcess()->sendWithAsyncReply(Messages::NetworkProcess::RemoveCredentialsWithOrigins(origins), WTFMove(completionHandler));
>-        }
>-    }
>-
> #if ENABLE(NETSCAPE_PLUGIN_API)
>     if (dataTypes.contains(WebsiteDataType::PlugInData) && isPersistent()) {
>         Vector<String> hostNames;
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 19e56cbdaf0ba53327a6ac8692e0bbc67fba8b55..cf633a7d0a673579d7792e78699397230c84292c 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-06-14  Sihui Liu  <sihui_liu@apple.com>
>+
>+        -[WKWebsiteDataStore removeDataOfTypes:modifiedSince:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
>+        https://bugs.webkit.org/show_bug.cgi?id=198854
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm:
>+        (TestWebKitAPI::TEST):
>+
> 2019-06-13  Tim Horton  <timothy_horton@apple.com>
> 
>         Make it possible for validate-committer-lists to dump a mailmap file
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>index f9787880e274f3461ffc6fd6a45452499d23dcd3..1e0b1f4d590773554814607089ab97c0648c0d68 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>@@ -252,6 +252,51 @@ TEST(WKWebsiteDataStore, RemovePersistentCredentials)
>     TestWebKitAPI::Util::run(&done);
> }
> 
>+TEST(WKWebsiteDataStore, RemoveAllPersistentCredentials)
>+{
>+    usePersistentCredentialStorage = true;
>+
>+    TCPServer server(respondWithChallengeThenOK);
>+    auto websiteDataStore = [WKWebsiteDataStore defaultDataStore];
>+    auto navigationDelegate = adoptNS([[NavigationTestDelegate alloc] init]);
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
>+    [webView setNavigationDelegate:navigationDelegate.get()];
>+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"http://127.0.0.1:%d/", server.port()]]]];
>+    [navigationDelegate waitForDidFinishNavigation];
>+
>+    readyToContinue = false;
>+    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+        bool foundRecord = false;
>+        for (WKWebsiteDataRecord *record in dataRecords) {
>+            auto name = [record displayName];
>+            if ([name isEqualToString:@"127.0.0.1"])
>+                foundRecord = true;
>+        }
>+        EXPECT_TRUE(foundRecord);
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+
>+    readyToContinue = false;
>+    [websiteDataStore removeDataOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] modifiedSince:[NSDate distantPast] completionHandler:^() {
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+    
>+    readyToContinue = false;
>+    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+        bool foundRecord = false;
>+        for (WKWebsiteDataRecord *record in dataRecords) {
>+            auto name = [record displayName];
>+            if ([name isEqualToString:@"127.0.0.1"])
>+                foundRecord = true;
>+        }
>+        EXPECT_FALSE(foundRecord);
>+        readyToContinue = true;
>+    }];
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+}
>+
> TEST(WKWebsiteDataStore, RemoveNonPersistentCredentials)
> {
>     TCPServer server(respondWithChallengeThenOK);

Actions: View | Formatted Diff | Diff

Attachments on bug 198854: 372113 | 372126 | 372142 | 372306