Attachment 368240 Details for Bug 196991 – Patch

[patch] Patch

bug-196991-20190425092639.patch (text/plain), 16.04 KB, created by Per Arne Vollan on 2019-04-25 09:26:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Per Arne Vollan

Created: 2019-04-25 09:26:39 PDT

Size: 16.04 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 244645)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,20 @@
>+2019-04-23  Per Arne Vollan  <pvollan@apple.com>
>+
>+        -[WKWebsiteDataStore fetchDataRecordsOfTypes:completionHandler:] never returns _WKWebsiteDataTypeCredentials
>+        https://bugs.webkit.org/show_bug.cgi?id=196991
>+        <rdar://problem/45507423>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add method to get all origins with persistent credentials from credential storage.
>+
>+        API tests: WKWebsiteDataStore.FetchNonPersistentCredentials
>+                   WKWebsiteDataStore.FetchPersistentCredentials
>+
>+        * platform/network/CredentialStorage.h:
>+        * platform/network/mac/CredentialStorageMac.mm:
>+        (WebCore::CredentialStorage::originsWithPersistentCredentials):
>+
> 2019-04-25  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r244627.
>Index: Source/WebCore/platform/network/CredentialStorage.h
>===================================================================
>--- Source/WebCore/platform/network/CredentialStorage.h	(revision 244645)
>+++ Source/WebCore/platform/network/CredentialStorage.h	(working copy)
>@@ -27,6 +27,7 @@
> 
> #include "Credential.h"
> #include "ProtectionSpaceHash.h"
>+#include "SecurityOriginData.h"
> #include <wtf/HashMap.h>
> #include <wtf/HashSet.h>
> #include <wtf/text/StringHash.h>
>@@ -45,6 +46,7 @@ public:
> 
>     // OS persistent storage.
>     WEBCORE_EXPORT static Credential getFromPersistentStorage(const ProtectionSpace&);
>+    WEBCORE_EXPORT static Vector<SecurityOriginData> originsWithPersistentCredentials();
> 
>     WEBCORE_EXPORT void clearCredentials();
> 
>Index: Source/WebCore/platform/network/mac/CredentialStorageMac.mm
>===================================================================
>--- Source/WebCore/platform/network/mac/CredentialStorageMac.mm	(revision 244645)
>+++ Source/WebCore/platform/network/mac/CredentialStorageMac.mm	(working copy)
>@@ -38,4 +38,13 @@ Credential CredentialStorage::getFromPer
>     return credential ? Credential(credential) : Credential();
> }
> 
>+Vector<WebCore::SecurityOriginData> CredentialStorage::originsWithPersistentCredentials()
>+{
>+    Vector<WebCore::SecurityOriginData> origins;
>+    auto allCredentials = [[NSURLCredentialStorage sharedCredentialStorage] allCredentials];
>+    for (NSURLProtectionSpace* key in allCredentials.keyEnumerator)
>+        origins.append(WebCore::SecurityOriginData { String(key.protocol), String(key.host), key.port });
>+    return origins;
>+}
>+
> } // namespace WebCore
>Index: Source/WebKit/NetworkProcess/NetworkProcess.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/NetworkProcess.cpp	(revision 244645)
>+++ Source/WebKit/NetworkProcess/NetworkProcess.cpp	(working copy)
>@@ -2487,6 +2487,11 @@ void NetworkProcess::removeCredential(We
>     completionHandler();
> }
> 
>+void NetworkProcess::originsWithPersistentCredentials(CompletionHandler<void(Vector<WebCore::SecurityOriginData>)>&& completionHandler)
>+{
>+    completionHandler(Vector<WebCore::SecurityOriginData>());
>+}
>+    
> void NetworkProcess::initializeProcess(const AuxiliaryProcessInitializationParameters&)
> {
> }
>Index: Source/WebKit/NetworkProcess/NetworkProcess.h
>===================================================================
>--- Source/WebKit/NetworkProcess/NetworkProcess.h	(revision 244645)
>+++ Source/WebKit/NetworkProcess/NetworkProcess.h	(working copy)
>@@ -430,6 +430,8 @@ private:
> 
>     void removeCredential(WebCore::Credential&&, WebCore::ProtectionSpace&&, CompletionHandler<void()>&&);
> 
>+    void originsWithPersistentCredentials(CompletionHandler<void(Vector<WebCore::SecurityOriginData>)>&&);
>+    
>     void registerURLSchemeAsSecure(const String&) const;
>     void registerURLSchemeAsBypassingContentSecurityPolicy(const String&) const;
>     void registerURLSchemeAsLocal(const String&) const;
>Index: Source/WebKit/NetworkProcess/NetworkProcess.messages.in
>===================================================================
>--- Source/WebKit/NetworkProcess/NetworkProcess.messages.in	(revision 244645)
>+++ Source/WebKit/NetworkProcess/NetworkProcess.messages.in	(working copy)
>@@ -173,4 +173,5 @@ messages -> NetworkProcess LegacyReceive
>     SetAdClickAttributionConversionURLForTesting(PAL::SessionID sessionID, URL url) -> () Async
>     MarkAdClickAttributionsAsExpiredForTesting(PAL::SessionID sessionID) -> () Async
>     RemoveCredential(WebCore::Credential credential, WebCore::ProtectionSpace protectionSpace) -> () Async
>+    OriginsWithPersistentCredentials() -> (Vector<WebCore::SecurityOriginData> persistentCredentials) Async
> }
>Index: Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>===================================================================
>--- Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm	(revision 244645)
>+++ Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm	(working copy)
>@@ -217,6 +217,11 @@ void NetworkProcess::removeCredential(We
>     completionHandler();
> }
> 
>+void NetworkProcess::originsWithPersistentCredentials(CompletionHandler<void(Vector<WebCore::SecurityOriginData>)>&& completionHandler)
>+{
>+    completionHandler(WebCore::CredentialStorage::originsWithPersistentCredentials());
>+}
>+
> #if PLATFORM(MAC)
> void NetworkProcess::setSharedHTTPCookieStorage(const Vector<uint8_t>& identifier)
> {
>Index: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	(revision 244645)
>+++ Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	(working copy)
>@@ -44,6 +44,7 @@
> #include "WebsiteDataStoreClient.h"
> #include "WebsiteDataStoreParameters.h"
> #include <WebCore/ApplicationCacheStorage.h>
>+#include <WebCore/CredentialStorage.h>
> #include <WebCore/DatabaseTracker.h>
> #include <WebCore/HTMLMediaElement.h>
> #include <WebCore/OriginLock.h>
>@@ -238,8 +239,8 @@ static ProcessAccessType computeNetworkP
>             processAccessType = std::max(processAccessType, ProcessAccessType::Launch);
>     }
> 
>-    if (dataTypes.contains(WebsiteDataType::Credentials) && !isNonPersistentStore)
>-        processAccessType = std::max(processAccessType, ProcessAccessType::Launch);
>+    if (dataTypes.contains(WebsiteDataType::Credentials) && isNonPersistentStore)
>+        processAccessType = std::max(processAccessType, ProcessAccessType::OnlyIfLaunched);
> 
>     if (dataTypes.contains(WebsiteDataType::DiskCache) && !isNonPersistentStore)
>         processAccessType = std::max(processAccessType, ProcessAccessType::Launch);
>@@ -356,8 +357,13 @@ void WebsiteDataStore::fetchDataAndApply
> #endif
> 
>             for (auto& origin : websiteData.originsWithCredentials) {
>-                auto& record = m_websiteDataRecords.add(origin, WebsiteDataRecord { }).iterator->value;
>-                
>+                auto displayName = WebsiteDataRecord::displayNameForOrigin(WebCore::SecurityOriginData::fromURL(URL(URL(), origin)));
>+                ASSERT(!displayName.isEmpty());
>+
>+                auto& record = m_websiteDataRecords.add(displayName, WebsiteDataRecord { }).iterator->value;
>+                if (!record.displayName)
>+                    record.displayName = WTFMove(displayName);
>+
>                 record.addOriginWithCredential(origin);
>             }
> 
>@@ -570,6 +576,24 @@ void WebsiteDataStore::fetchDataAndApply
>         });
>     }
> 
>+#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 {
>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 244645)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2019-04-23  Per Arne Vollan  <pvollan@apple.com>
>+
>+        -[WKWebsiteDataStore fetchDataRecordsOfTypes:completionHandler:] never returns _WKWebsiteDataTypeCredentials
>+        https://bugs.webkit.org/show_bug.cgi?id=196991
>+        <rdar://problem/45507423>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm:
>+        (-[NavigationTestDelegate init]):
>+        (-[NavigationTestDelegate waitForDidFinishNavigation]):
>+        (-[NavigationTestDelegate webView:didFinishNavigation:]):
>+        (-[NavigationTestDelegate webView:didReceiveAuthenticationChallenge:completionHandler:]):
>+        (TestWebKitAPI::TEST):
>+        (TEST): Deleted.
>+
> 2019-04-25  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r244627.
>Index: Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>===================================================================
>--- Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm	(revision 244645)
>+++ Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2018 Apple Inc. All rights reserved.
>+ * Copyright (C) 2019 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -26,14 +26,61 @@
> #import "config.h"
> 
> #import "PlatformUtilities.h"
>+#import "TCPServer.h"
> #import "Test.h"
> #import "TestWKWebView.h"
>+#import <WebKit/WKProcessPoolPrivate.h>
>+#import <WebKit/WKWebsiteDataRecordPrivate.h>
> #import <WebKit/WKWebsiteDataStorePrivate.h>
> #import <WebKit/WebKit.h>
> #import <wtf/text/WTFString.h>
> 
> static bool readyToContinue;
> 
>+static RetainPtr<NSURLCredential> persistentCredential;
>+static bool usePersistentCredentialStorage = false;
>+
>+@interface NavigationTestDelegate : NSObject <WKNavigationDelegate>
>+@end
>+
>+@implementation NavigationTestDelegate {
>+    bool _hasFinishedNavigation;
>+}
>+
>+- (instancetype)init
>+{
>+    if (!(self = [super init]))
>+        return nil;
>+    
>+    _hasFinishedNavigation = false;
>+    
>+    return self;
>+}
>+
>+- (void)waitForDidFinishNavigation
>+{
>+    TestWebKitAPI::Util::run(&_hasFinishedNavigation);
>+}
>+
>+- (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
>+{
>+    _hasFinishedNavigation = true;
>+}
>+
>+- (void)webView:(WKWebView *)webView didReceiveAuthenticationChallenge:(NSURLAuthenticationChallenge *)challenge completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition, NSURLCredential *))completionHandler
>+{
>+    static bool firstChallenge = true;
>+    if (firstChallenge) {
>+        firstChallenge = false;
>+        persistentCredential = adoptNS([[NSURLCredential alloc] initWithUser:@"username" password:@"password" persistence:(usePersistentCredentialStorage ? NSURLCredentialPersistencePermanent: NSURLCredentialPersistenceForSession)]);
>+        return completionHandler(NSURLSessionAuthChallengeUseCredential, persistentCredential.get());
>+    }
>+    return completionHandler(NSURLSessionAuthChallengeUseCredential, nil);
>+}
>+@end
>+
>+namespace TestWebKitAPI {
>+
> TEST(WKWebsiteDataStore, RemoveAndFetchData)
> {
>     readyToContinue = false;
>@@ -62,3 +109,92 @@ TEST(WKWebsiteDataStore, RemoveEphemeral
>     }];
>     TestWebKitAPI::Util::run(&done);
> }
>+
>+static void respondWithChallengeThenOK(int socket)
>+{
>+    char readBuffer[1000];
>+    auto bytesRead = ::read(socket, readBuffer, sizeof(readBuffer));
>+    EXPECT_GT(bytesRead, 0);
>+    EXPECT_TRUE(static_cast<size_t>(bytesRead) < sizeof(readBuffer));
>+    
>+    const char* challengeHeader =
>+    "HTTP/1.1 401 Unauthorized\r\n"
>+    "Date: Sat, 23 Mar 2019 06:29:01 GMT\r\n"
>+    "Content-Length: 0\r\n"
>+    "WWW-Authenticate: Basic realm=\"testrealm\"\r\n\r\n";
>+    auto bytesWritten = ::write(socket, challengeHeader, strlen(challengeHeader));
>+    EXPECT_EQ(static_cast<size_t>(bytesWritten), strlen(challengeHeader));
>+    
>+    bytesRead = ::read(socket, readBuffer, sizeof(readBuffer));
>+    EXPECT_GT(bytesRead, 0);
>+    EXPECT_TRUE(static_cast<size_t>(bytesRead) < sizeof(readBuffer));
>+    
>+    const char* responseHeader =
>+    "HTTP/1.1 200 OK\r\n"
>+    "Content-Length: 13\r\n\r\n"
>+    "Hello, World!";
>+    bytesWritten = ::write(socket, responseHeader, strlen(responseHeader));
>+    EXPECT_EQ(static_cast<size_t>(bytesWritten), strlen(responseHeader));
>+}
>+    
>+TEST(WKWebsiteDataStore, FetchNonPersistentCredentials)
>+{
>+    TCPServer server(respondWithChallengeThenOK);
>+    
>+    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;
>+    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+        int credentialCount = dataRecords.count;
>+        ASSERT_EQ(credentialCount, 1);
>+        for (WKWebsiteDataRecord *record in dataRecords)
>+            ASSERT_TRUE([[record displayName] isEqualToString:@"127.0.0.1"]);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+}
>+
>+TEST(WKWebsiteDataStore, FetchPersistentCredentials)
>+{
>+    TCPServer server(respondWithChallengeThenOK);
>+    
>+    usePersistentCredentialStorage = true;
>+    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];
>+
>+    __block bool done = false;
>+    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+        int credentialCount = dataRecords.count;
>+        ASSERT_GT(credentialCount, 0);
>+        bool foundExpectedRecord = false;
>+        for (WKWebsiteDataRecord *record in dataRecords) {
>+            auto name = [record displayName];
>+            if ([name isEqualToString:@"127.0.0.1"]) {
>+                foundExpectedRecord = true;
>+                break;
>+            }
>+        }
>+        EXPECT_TRUE(foundExpectedRecord);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    
>+    __block bool removedCredential = false;
>+    [[[webView configuration] processPool] _removeCredential:persistentCredential.get() forProtectionSpace:[[[NSURLProtectionSpace alloc] initWithHost:@"127.0.0.1" port:server.port() protocol:NSURLProtectionSpaceHTTP realm:@"testrealm" authenticationMethod:NSURLAuthenticationMethodHTTPBasic] autorelease] completionHandler:^{
>+        removedCredential = true;
>+    }];
>+    TestWebKitAPI::Util::run(&removedCredential);
>+}
>+
>+}

Flags:

achristensen: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 196991: 367591 | 367981 | 368085 | 368176 | 368188 | 368240 | 368251