Changeset 280410 in webkit

Timestamp:

Jul 28, 2021 4:35:42 PM (12 months ago)

Author:

sihui_liu@apple.com

Message:

IDBFactory.databases should not return databases with invalid version
​https://bugs.webkit.org/show_bug.cgi?id=228554

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

• web-platform-tests/IndexedDB/get-databases.any-expected.txt:
• web-platform-tests/IndexedDB/get-databases.any.worker-expected.txt:

Source/WebCore:

To get database information, we used to open all database files and read from them. To avoid opening new
connection to databases that are already opened, and to ensure we get the latest information, for database that
is already opened, we should get its information from UniqueIDBDatabase directly. If database is not opened, we
open its file and read. By doing this, we also fix the issue that no databases are returned in private browsing
(since there is no file created).

Also, version 0 means the database is just created and has not finished version change transaction (version 1
is the first valid version according to spec), so we should not return databases with version 0.

Tests: storage/indexeddb/getdatabases-private.html

storage/indexeddb/getdatabases.html

• Modules/indexeddb/server/IDBBackingStore.h:
• Modules/indexeddb/server/IDBServer.cpp:

(WebCore::IDBServer::getDatabaseNameAndVersionFromOriginDirectory):
(WebCore::IDBServer::IDBServer::getAllDatabaseNamesAndVersions):

• Modules/indexeddb/server/MemoryIDBBackingStore.h:
• Modules/indexeddb/server/SQLiteIDBBackingStore.h:
• Modules/indexeddb/server/UniqueIDBDatabase.cpp:

(WebCore::IDBServer::UniqueIDBDatabase::didDeleteBackingStore):
(WebCore::IDBServer::UniqueIDBDatabase::filePath const):
(WebCore::IDBServer::UniqueIDBDatabase::nameAndVersion const):

• Modules/indexeddb/server/UniqueIDBDatabase.h:

LayoutTests:

• storage/indexeddb/getdatabases-expected.txt: Added.
• storage/indexeddb/getdatabases-private-expected.txt: Added.
• storage/indexeddb/getdatabases-private.html: Added.
• storage/indexeddb/getdatabases.html: Added.
• storage/indexeddb/resources/getdatabases.js: Added.

(test):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any.worker-expected.txt (modified) (1 diff)
• LayoutTests/storage/indexeddb/getdatabases-expected.txt (added)
• LayoutTests/storage/indexeddb/getdatabases-private-expected.txt (added)
• LayoutTests/storage/indexeddb/getdatabases-private.html (added)
• LayoutTests/storage/indexeddb/getdatabases.html (added)
• LayoutTests/storage/indexeddb/resources/getdatabases.js (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h (modified) (1 diff)
• Source/WebCore/Modules/indexeddb/server/IDBServer.cpp (modified) (3 diffs)
• Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h (modified) (1 diff)
• Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h (modified) (2 diffs)
• Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp (modified) (2 diffs)
• Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-07-28  Sihui Liu  <sihui_liu@apple.com>
+
+        IDBFactory.databases should not return databases with invalid version
+        https://bugs.webkit.org/show_bug.cgi?id=228554
+
+        Reviewed by Chris Dumez.
+
+        * storage/indexeddb/getdatabases-expected.txt: Added.
+        * storage/indexeddb/getdatabases-private-expected.txt: Added.
+        * storage/indexeddb/getdatabases-private.html: Added.
+        * storage/indexeddb/getdatabases.html: Added.
+        * storage/indexeddb/resources/getdatabases.js: Added.
+        (test):
+
 2021-07-28  Ayumi Kojima  <ayumi_kojima@apple.com>
 

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2021-07-28  Sihui Liu  <sihui_liu@apple.com>
+
+        IDBFactory.databases should not return databases with invalid version
+        https://bugs.webkit.org/show_bug.cgi?id=228554
+
+        Reviewed by Chris Dumez.
+
+        * web-platform-tests/IndexedDB/get-databases.any-expected.txt:
+        * web-platform-tests/IndexedDB/get-databases.any.worker-expected.txt:
+
 2021-07-28  Ayumi Kojima  <ayumi_kojima@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any-expected.txt

@@ -4 +4 @@
 PASS Enumerate multiple databases.
 PASS Make sure an empty list is returned for the case of no databases.
-FAIL Ensure that databases() doesn't pick up changes that haven't commited. assert_equals: The result of databases() should be only those databases which have been created at the time of calling, regardless of versionchange transactions currently running. expected 1 but got 2
+PASS Ensure that databases() doesn't pick up changes that haven't commited.
 

trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any.worker-expected.txt

@@ -4 +4 @@
 PASS Enumerate multiple databases.
 PASS Make sure an empty list is returned for the case of no databases.
-FAIL Ensure that databases() doesn't pick up changes that haven't commited. assert_equals: The result of databases() should be only those databases which have been created at the time of calling, regardless of versionchange transactions currently running. expected 1 but got 2
+PASS Ensure that databases() doesn't pick up changes that haven't commited.
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-07-28  Sihui Liu  <sihui_liu@apple.com>
+
+        IDBFactory.databases should not return databases with invalid version
+        https://bugs.webkit.org/show_bug.cgi?id=228554
+
+        Reviewed by Chris Dumez.
+
+        To get database information, we used to open all database files and read from them. To avoid opening new 
+        connection to databases that are already opened, and to ensure we get the latest information, for database that 
+        is already opened, we should get its information from UniqueIDBDatabase directly. If database is not opened, we 
+        open its file and read. By doing this, we also fix the issue that no databases are returned in private browsing
+        (since there is no file created).
+
+        Also, version 0 means the database is just created and has not finished version change transaction (version 1 
+        is the first valid version according to spec), so we should not return databases with version 0.
+
+        Tests: storage/indexeddb/getdatabases-private.html
+               storage/indexeddb/getdatabases.html
+
+        * Modules/indexeddb/server/IDBBackingStore.h:
+        * Modules/indexeddb/server/IDBServer.cpp:
+        (WebCore::IDBServer::getDatabaseNameAndVersionFromOriginDirectory):
+        (WebCore::IDBServer::IDBServer::getAllDatabaseNamesAndVersions):
+        * Modules/indexeddb/server/MemoryIDBBackingStore.h:
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.h:
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::didDeleteBackingStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::filePath const):
+        (WebCore::IDBServer::UniqueIDBDatabase::nameAndVersion const):
+        * Modules/indexeddb/server/UniqueIDBDatabase.h:
+
 2021-07-28  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h

@@ -92 +92 @@
     virtual bool supportsSimultaneousTransactions() = 0;
     virtual bool isEphemeral() = 0;
+    virtual String fullDatabasePath() const = 0;
 
     virtual void close() = 0;

trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp

@@ -509 +509 @@
 }
 
+static void getDatabaseNameAndVersionFromOriginDirectory(const String& directory, HashSet<String>& excludedDatabasePaths, Vector<IDBDatabaseNameAndVersion>& result)
+{
+    Vector<String> databaseDirectoryNames = FileSystem::listDirectory(directory);
+    for (auto& databaseDirectoryName : databaseDirectoryNames) {
+        auto fullDatabasePath = SQLiteIDBBackingStore::fullDatabasePathForDirectory(FileSystem::pathByAppendingComponent(directory, databaseDirectoryName));
+        if (excludedDatabasePaths.contains(fullDatabasePath))
+            continue;
+
+        if (auto nameAndVersion = SQLiteIDBBackingStore::databaseNameAndVersionFromFile(fullDatabasePath))
+            result.append(WTFMove(*nameAndVersion));
+    }
+}
+
 void IDBServer::getAllDatabaseNamesAndVersions(IDBConnectionIdentifier serverConnectionIdentifier, const IDBResourceIdentifier& requestIdentifier, const ClientOrigin& origin)
 {
@@ -514 +527 @@
     ASSERT(m_lock.isHeld());
 
+    Vector<IDBDatabaseNameAndVersion> result;
+    HashSet<String> visitedDatabasePaths;
+
+    for (auto& database : m_uniqueIDBDatabaseMap.values()) {
+        auto path = database->filePath();
+        if (!path.isEmpty())
+            visitedDatabasePaths.add(path);
+
+        if (auto nameAndVersion = database->nameAndVersion())
+            result.append(WTFMove(*nameAndVersion));
+    }
+
     String oldDirectory = IDBDatabaseIdentifier::databaseDirectoryRelativeToRoot(origin.topOrigin, origin.clientOrigin, m_databaseDirectoryPath, "v0");
-    Vector<String> fileNames = FileSystem::listDirectory(oldDirectory);
-    Vector<IDBDatabaseNameAndVersion> databases;
-    for (auto& fileName : fileNames) {
-        auto databaseTuple = SQLiteIDBBackingStore::databaseNameAndVersionFromFile(SQLiteIDBBackingStore::fullDatabasePathForDirectory(FileSystem::pathByAppendingComponent(oldDirectory, fileName)));
-        if (databaseTuple)
-            databases.append(WTFMove(*databaseTuple));
-    }
+    getDatabaseNameAndVersionFromOriginDirectory(oldDirectory, visitedDatabasePaths, result);
 
     String directory = IDBDatabaseIdentifier::databaseDirectoryRelativeToRoot(origin.topOrigin, origin.clientOrigin, m_databaseDirectoryPath, "v1");
-    fileNames = FileSystem::listDirectory(directory);
-    for (auto& fileName : fileNames) {
-        auto databaseTuple = SQLiteIDBBackingStore::databaseNameAndVersionFromFile(SQLiteIDBBackingStore::fullDatabasePathForDirectory(FileSystem::pathByAppendingComponent(directory, fileName)));
-        if (databaseTuple)
-            databases.append(WTFMove(*databaseTuple));
-    }
+    getDatabaseNameAndVersionFromOriginDirectory(directory, visitedDatabasePaths, result);
 
     auto connection = m_connectionMap.get(serverConnectionIdentifier);
@@ -535 +549 @@
         return;
 
-    connection->didGetAllDatabaseNamesAndVersions(requestIdentifier, WTFMove(databases));
+    connection->didGetAllDatabaseNamesAndVersions(requestIdentifier, WTFMove(result));
 }
 

trunk/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h

@@ -81 +81 @@
     bool supportsSimultaneousTransactions() final { return true; }
     bool isEphemeral() final { return true; }
+    String fullDatabasePath() const final { return nullString(); }
 
     bool hasTransaction(const IDBResourceIdentifier& identifier) const final { return m_transactions.contains(identifier); }

trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h

@@ -87 +87 @@
     bool supportsSimultaneousTransactions() final { return false; }
     bool isEphemeral() final { return false; }
+    String fullDatabasePath() const final;
 
     bool hasTransaction(const IDBResourceIdentifier&) const final;
@@ -104 +105 @@
 private:
     String filenameForDatabaseName() const;
-    String fullDatabasePath() const;
     String fullDatabaseDirectoryWithUpgrade();
 

trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp

@@ -313 +313 @@
 
     if (m_databaseInfo)
-        m_mostRecentDeletedDatabaseInfo = WTFMove(m_databaseInfo);
+        m_mostRecentDeletedDatabaseInfo = std::exchange(m_databaseInfo, nullptr);
 
     // If this UniqueIDBDatabase was brought into existence for the purpose of deleting the file on disk,
@@ -1241 +1241 @@
 }
 
+String UniqueIDBDatabase::filePath() const
+{
+    return m_backingStore ? m_backingStore->fullDatabasePath() : nullString();
+}
+
+std::optional<IDBDatabaseNameAndVersion> UniqueIDBDatabase::nameAndVersion() const
+{
+    if (!m_backingStore)
+        return std::nullopt;
+
+    if (m_versionChangeTransaction) {
+        if (auto databaseInfo = m_versionChangeTransaction->originalDatabaseInfo()) {
+            // The database is newly created.
+            if (!databaseInfo->version())
+                return std::nullopt;
+
+            return IDBDatabaseNameAndVersion { databaseInfo->name(), databaseInfo->version() };
+        }
+
+        return std::nullopt;
+    }
+
+    return IDBDatabaseNameAndVersion { m_databaseInfo->name(), m_databaseInfo->version() };
+}
+
 } // namespace IDBServer
 } // namespace WebCore

trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h

@@ -29 +29 @@
 #include "IDBDatabaseIdentifier.h"
 #include "IDBDatabaseInfo.h"
+#include "IDBDatabaseNameAndVersion.h"
 #include "IDBGetResult.h"
 #include "ServerOpenDBRequest.h"
@@ -112 +113 @@
     bool tryClose();
 
+    String filePath() const;
+    std::optional<IDBDatabaseNameAndVersion> nameAndVersion() const;
+
 private:
     void handleDatabaseOperations();