Attachment 369127 Details for Bug 197510 – Patch

[patch] Patch

bug-197510-20190506094712.patch (text/plain), 12.61 KB, created by Per Arne Vollan on 2019-05-06 09:47:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Per Arne Vollan

Created: 2019-05-06 09:47:13 PDT

Size: 12.61 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 244947)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,24 @@
>+2019-05-04  Per Arne Vollan  <pvollan@apple.com>
>+
>+        -[WKWebsiteDataStore removeDataOfTypes:forDataRecords:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
>+        https://bugs.webkit.org/show_bug.cgi?id=197510
>+        <rdar://problem/50372338>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch implements deletion of non persistent credentials for a set of origins. In order for this to work, fetching
>+        credentials from the credential storage needs to return a set of SecurityOriginData objects, instead of a set of origin
>+        strings. This is implemented by iterating over all the elements in the credential map, and creating a SecurityOriginData
>+        object for each credential based on the protection space.
>+
>+        API test: WKWebsiteDataStore.RemoveNonPersistentCredentials
>+
>+        * platform/network/CredentialStorage.cpp:
>+        (WebCore::CredentialStorage::removeCredentialsWithOrigin):
>+        (WebCore::CredentialStorage::originsWithCredentials const):
>+        * platform/network/CredentialStorage.h:
>+        (WebCore::CredentialStorage::originsWithCredentials const): Deleted.
>+
> 2019-05-03  Simon Fraser  <simon.fraser@apple.com>
> 
>         [macOS] Fix programmatic scrolling with async overflow scroll
>Index: Source/WebCore/platform/network/CredentialStorage.cpp
>===================================================================
>--- Source/WebCore/platform/network/CredentialStorage.cpp	(revision 244947)
>+++ Source/WebCore/platform/network/CredentialStorage.cpp	(working copy)
>@@ -86,6 +86,56 @@ void CredentialStorage::remove(const Str
>     m_protectionSpaceToCredentialMap.remove(std::make_pair(partitionName, protectionSpace));
> }
> 
>+void CredentialStorage::removeCredentialsWithOrigin(const SecurityOriginData& origin)
>+{
>+    Vector<std::pair<String, ProtectionSpace>> keysToRemove;
>+    for (auto& keyValuePair : m_protectionSpaceToCredentialMap) {
>+        auto& protectionSpace = keyValuePair.key.second;
>+        if (protectionSpace.host() == origin.host
>+            && ((origin.port && protectionSpace.port() == *origin.port)
>+            || (!origin.port && protectionSpace.port() == 80))
>+            && ((protectionSpace.serverType() == ProtectionSpaceServerHTTP && origin.protocol == "http"_s)
>+            || (protectionSpace.serverType() == ProtectionSpaceServerHTTPS && origin.protocol == "https"_s)))
>+            keysToRemove.append(keyValuePair.key);
>+    }
>+    for (auto& key : keysToRemove) {
>+        auto& partitionName = key.first;
>+        auto& protectionSpace = key.second;
>+        remove(partitionName, protectionSpace);
>+    }
>+}
>+
>+Vector<SecurityOriginData> CredentialStorage::originsWithCredentials() const
>+{
>+    Vector<SecurityOriginData> origins;
>+    for (auto& keyValuePair : m_protectionSpaceToCredentialMap) {
>+        auto& protectionSpace = keyValuePair.key.second;
>+        if (protectionSpace.isProxy())
>+            continue;
>+        String protocol;
>+        switch (protectionSpace.serverType()) {
>+        case ProtectionSpaceServerHTTP:
>+            protocol = "http"_s;
>+            break;
>+        case ProtectionSpaceServerHTTPS:
>+            protocol = "https"_s;
>+            break;
>+        case ProtectionSpaceServerFTP:
>+            protocol = "ftp"_s;
>+            break;
>+        case ProtectionSpaceServerFTPS:
>+            protocol = "ftps"_s;
>+            break;
>+        default:
>+            ASSERT_NOT_REACHED();
>+        }
>+
>+        SecurityOriginData origin { protocol, protectionSpace.host(), static_cast<uint16_t>(protectionSpace.port())};
>+        origins.append(WTFMove(origin));
>+    }
>+    return origins;
>+}
>+
> HashMap<String, ProtectionSpace>::iterator CredentialStorage::findDefaultProtectionSpaceForURL(const URL& url)
> {
>     ASSERT(url.protocolIsInHTTPFamily());
>Index: Source/WebCore/platform/network/CredentialStorage.h
>===================================================================
>--- Source/WebCore/platform/network/CredentialStorage.h	(revision 244947)
>+++ Source/WebCore/platform/network/CredentialStorage.h	(working copy)
>@@ -43,6 +43,7 @@ public:
>     WEBCORE_EXPORT void set(const String&, const Credential&, const ProtectionSpace&, const URL&);
>     WEBCORE_EXPORT Credential get(const String&, const ProtectionSpace&);
>     WEBCORE_EXPORT void remove(const String&, const ProtectionSpace&);
>+    WEBCORE_EXPORT void removeCredentialsWithOrigin(const SecurityOriginData&);
> 
>     // OS persistent storage.
>     WEBCORE_EXPORT static Credential getFromPersistentStorage(const ProtectionSpace&);
>@@ -55,7 +56,7 @@ public:
>     WEBCORE_EXPORT bool set(const String&, const Credential&, const URL&); // Returns true if the URL corresponds to a known protection space, so credentials could be updated.
>     WEBCORE_EXPORT Credential get(const String&, const URL&);
> 
>-    const HashSet<String>& originsWithCredentials() const { return m_originsWithCredentials; }
>+    WEBCORE_EXPORT Vector<SecurityOriginData> originsWithCredentials() const;
> 
> private:
>     HashMap<std::pair<String /* partitionName */, ProtectionSpace>, Credential> m_protectionSpaceToCredentialMap;
>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 244947)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2019-05-04  Per Arne Vollan  <pvollan@apple.com>
>+
>+        -[WKWebsiteDataStore removeDataOfTypes:forDataRecords:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
>+        https://bugs.webkit.org/show_bug.cgi?id=197510
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * NetworkProcess/NetworkProcess.cpp:
>+        (WebKit::NetworkProcess::fetchWebsiteData):
>+        (WebKit::NetworkProcess::deleteWebsiteDataForOrigins):
>+        (WebKit::NetworkProcess::registrableDomainsWithWebsiteData):
>+
> 2019-05-03  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         REGRESSION (r244897): Caret may appear wider than normal after zooming to focus an editable element
>Index: Source/WebKit/NetworkProcess/NetworkProcess.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/NetworkProcess.cpp	(revision 244947)
>+++ Source/WebKit/NetworkProcess/NetworkProcess.cpp	(working copy)
>@@ -1322,8 +1322,11 @@ void NetworkProcess::fetchWebsiteData(PA
>     }
> 
>     if (websiteDataTypes.contains(WebsiteDataType::Credentials)) {
>-        if (storageSession(sessionID))
>-            callbackAggregator->m_websiteData.originsWithCredentials = storageSession(sessionID)->credentialStorage().originsWithCredentials();
>+        if (storageSession(sessionID)) {
>+            auto securityOrigins = storageSession(sessionID)->credentialStorage().originsWithCredentials();
>+            for (auto& securityOrigin : securityOrigins)
>+                callbackAggregator->m_websiteData.entries.append({ securityOrigin, WebsiteDataType::Credentials, 0 });
>+        }
>     }
> 
>     if (websiteDataTypes.contains(WebsiteDataType::DOMCache)) {
>@@ -1504,6 +1507,13 @@ void NetworkProcess::deleteWebsiteDataFo
>     if (websiteDataTypes.contains(WebsiteDataType::DiskCache) && !sessionID.isEphemeral())
>         clearDiskCacheEntries(cache(), originDatas, [clearTasksHandler = WTFMove(clearTasksHandler)] { });
> 
>+    if (websiteDataTypes.contains(WebsiteDataType::Credentials)) {
>+        if (auto* session = storageSession(sessionID)) {
>+            for (auto& originData : originDatas)
>+                session->credentialStorage().removeCredentialsWithOrigin(originData);
>+        }
>+    }
>+
>     // FIXME: Implement storage quota clearing for these origins.
> }
> 
>@@ -1778,8 +1788,11 @@ void NetworkProcess::registrableDomainsW
> #endif
> 
>     if (websiteDataTypes.contains(WebsiteDataType::Credentials)) {
>-        if (auto* networkStorageSession = storageSession(sessionID))
>-            websiteDataStore.originsWithCredentials = networkStorageSession->credentialStorage().originsWithCredentials();
>+        if (auto* networkStorageSession = storageSession(sessionID)) {
>+            auto securityOrigins = networkStorageSession->credentialStorage().originsWithCredentials();
>+            for (auto& securityOrigin : securityOrigins)
>+                callbackAggregator->m_websiteData.entries.append({ securityOrigin, WebsiteDataType::Credentials, 0 });
>+        }
>     }
>     
>     if (websiteDataTypes.contains(WebsiteDataType::DOMCache)) {
>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 244950)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2019-05-04  Per Arne Vollan  <pvollan@apple.com>
>+
>+        -[WKWebsiteDataStore removeDataOfTypes:forDataRecords:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
>+        https://bugs.webkit.org/show_bug.cgi?id=197510
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm:
>+        (TestWebKitAPI::TEST):
>+
> 2019-05-03  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         REGRESSION (r244897): Caret may appear wider than normal after zooming to focus an editable element
>Index: Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>===================================================================
>--- Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm	(revision 244947)
>+++ Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm	(working copy)
>@@ -141,6 +141,7 @@ TEST(WKWebsiteDataStore, FetchNonPersist
> {
>     TCPServer server(respondWithChallengeThenOK);
>     
>+    usePersistentCredentialStorage = false;
>     auto configuration = adoptNS([WKWebViewConfiguration new]);
>     auto websiteDataStore = [WKWebsiteDataStore nonPersistentDataStore];
>     [configuration setWebsiteDataStore:websiteDataStore];
>@@ -200,7 +201,7 @@ TEST(WKWebsiteDataStore, FetchPersistent
> TEST(WKWebsiteDataStore, RemovePersistentCredentials)
> {
>     TCPServer server(respondWithChallengeThenOK);
>-    
>+
>     usePersistentCredentialStorage = true;
>     auto websiteDataStore = [WKWebsiteDataStore defaultDataStore];
>     auto navigationDelegate = adoptNS([[NavigationTestDelegate alloc] init]);
>@@ -248,4 +249,57 @@ TEST(WKWebsiteDataStore, RemovePersisten
>     TestWebKitAPI::Util::run(&done);
> }
> 
>+TEST(WKWebsiteDataStore, RemoveNonPersistentCredentials)
>+{
>+    TCPServer server(respondWithChallengeThenOK);
>+
>+    usePersistentCredentialStorage = false;
>+    auto configuration = adoptNS([WKWebViewConfiguration new]);
>+    auto websiteDataStore = [WKWebsiteDataStore nonPersistentDataStore];
>+    [configuration setWebsiteDataStore:websiteDataStore];
>+    auto navigationDelegate = adoptNS([[NavigationTestDelegate alloc] init]);
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+    [webView setNavigationDelegate:navigationDelegate.get()];
>+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"http://127.0.0.1:%d/", server.port()]]]];
>+    [navigationDelegate waitForDidFinishNavigation];
>+
>+    __block bool done = false;
>+    __block RetainPtr<WKWebsiteDataRecord> expectedRecord;
>+    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+        int credentialCount = dataRecords.count;
>+        ASSERT_GT(credentialCount, 0);
>+        for (WKWebsiteDataRecord *record in dataRecords) {
>+            auto name = [record displayName];
>+            if ([name isEqualToString:@"127.0.0.1"]) {
>+                expectedRecord = record;
>+                break;
>+            }
>+        }
>+        EXPECT_TRUE(expectedRecord);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+
>+    done = false;
>+    [websiteDataStore removeDataOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] forDataRecords:[NSArray arrayWithObject:expectedRecord.get()] completionHandler:^(void) {
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+
>+    done = false;
>+    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+        bool foundLocalHostRecord = false;
>+        for (WKWebsiteDataRecord *record in dataRecords) {
>+            auto name = [record displayName];
>+            if ([name isEqualToString:@"127.0.0.1"]) {
>+                foundLocalHostRecord = true;
>+                break;
>+            }
>+        }
>+        EXPECT_FALSE(foundLocalHostRecord);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+}
>+
> }

Flags:

achristensen: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197510: 368772 | 368779 | 368797 | 369073 | 369127 | 369165