Changeset 290239 in webkit

Timestamp:

Feb 21, 2022 1:23:39 AM (5 months ago)

Author:

sihui_liu@apple.com

Message:

Fetching website data may get wrong record after migrating data to general storage directory
​https://bugs.webkit.org/show_bug.cgi?id=236905

Reviewed by Chris Dumez.

Source/WebKit:

To decide if an origin has data, we need to not only check if its directory exists but also if the directory is
empty.

Modified existing test to add test coverage.

• NetworkProcess/storage/OriginStorageManager.cpp:

(WebKit::OriginStorageManager::StorageBucket::resolvedLocalStoragePath):
(WebKit::OriginStorageManager::StorageBucket::resolvedIDBStoragePath): directory should be created before moving
files.
(WebKit::OriginStorageManager::StorageBucket::fetchDataTypesInListFromDisk): given that IndexedDB and
LocalStorage can have custom paths, we may just check data by type.

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:

(TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp (modified) (3 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2022-02-21  Sihui Liu  <sihui_liu@apple.com>
+
+        Fetching website data may get wrong record after migrating data to general storage directory
+        https://bugs.webkit.org/show_bug.cgi?id=236905
+
+        Reviewed by Chris Dumez.
+
+        To decide if an origin has data, we need to not only check if its directory exists but also if the directory is 
+        empty.
+
+        Modified existing test to add test coverage.
+
+        * NetworkProcess/storage/OriginStorageManager.cpp:
+        (WebKit::OriginStorageManager::StorageBucket::resolvedLocalStoragePath):
+        (WebKit::OriginStorageManager::StorageBucket::resolvedIDBStoragePath): directory should be created before moving
+        files.
+        (WebKit::OriginStorageManager::StorageBucket::fetchDataTypesInListFromDisk): given that IndexedDB and 
+        LocalStorage can have custom paths, we may just check data by type.
+
 2022-02-19  Jon Lee  <jonlee@apple.com>
 

trunk/Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp

@@ -259 +259 @@
 
             auto localStoragePath = LocalStorageManager::localStorageFilePath(localStorageDirectory);
-            if (!m_customLocalStoragePath.isEmpty() && !FileSystem::fileExists(localStoragePath))
+            if (!m_customLocalStoragePath.isEmpty() && !FileSystem::fileExists(localStoragePath) && FileSystem::fileExists(m_customLocalStoragePath))
                 WebCore::SQLiteFileSystem::moveDatabaseFile(m_customLocalStoragePath, localStoragePath);
 
@@ -281 +281 @@
         } else {
             auto idbStoragePath = typeStoragePath(StorageType::IndexedDB);
-            if (!m_customIDBStoragePath.isEmpty() && !FileSystem::fileExists(idbStoragePath)) {
+            if (!m_customIDBStoragePath.isEmpty() && !FileSystem::fileExists(idbStoragePath) && FileSystem::fileExists(m_customIDBStoragePath)) {
+                FileSystem::makeAllDirectories(idbStoragePath);
                 FileSystem::moveFile(m_customIDBStoragePath, idbStoragePath);
-                FileSystem::makeAllDirectories(idbStoragePath);
             }
 
@@ -318 +318 @@
     {
         OptionSet<WebsiteDataType> result;
-        for (auto& storageType : FileSystem::listDirectory(m_rootPath)) {
-            if (auto type = toWebsiteDataType(storageType); type && types.contains(*type))
-                result.add(*type);
-        }
-
-        if (types.contains(WebsiteDataType::LocalStorage) && !result.contains(WebsiteDataType::LocalStorage)) {
+        if (types.contains(WebsiteDataType::FileSystem)) {
+            auto fileSystemStoragePath = typeStoragePath(StorageType::FileSystem);
+            if (auto files = FileSystem::listDirectory(fileSystemStoragePath); !files.isEmpty())
+                result.add(WebsiteDataType::FileSystem);
+        }
+
+        if (types.contains(WebsiteDataType::LocalStorage)) {
             if (FileSystem::fileExists(resolvedLocalStoragePath()))
                 result.add(WebsiteDataType::LocalStorage);
         }
 
-        if (types.contains(WebsiteDataType::IndexedDBDatabases) && !result.contains(WebsiteDataType::IndexedDBDatabases)) {
+        if (types.contains(WebsiteDataType::IndexedDBDatabases)) {
             if (auto databases = FileSystem::listDirectory(resolvedIDBStoragePath()); !databases.isEmpty())
                 result.add(WebsiteDataType::IndexedDBDatabases);

trunk/Tools/ChangeLog

@@ +1 @@
+2022-02-21  Sihui Liu  <sihui_liu@apple.com>
+
+        Fetching website data may get wrong record after migrating data to general storage directory
+        https://bugs.webkit.org/show_bug.cgi?id=236905
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
+        (TEST):
+
 2022-02-20  Sihui Liu  <sihui_liu@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm

@@ -1083 +1083 @@
     EXPECT_FALSE([fileManager fileExistsAtPath:appleGeneralLocalStorageFile.path]);
     EXPECT_FALSE([fileManager fileExistsAtPath:appleGeneralIndexedDBDatabaseFile.path]);
-}
+
+    // Ensure data records do not exist after deletion.
+    done = false;
+    [websiteDataStore fetchDataRecordsOfTypes:dataTypes completionHandler:^(NSArray<WKWebsiteDataRecord *> *records) {
+        EXPECT_EQ(records.count, 0u);
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+}