Title: [280410] trunk
Revision
280410
Author
sihui_...@apple.com
Date
2021-07-28 16:35:42 -0700 (Wed, 28 Jul 2021)

Log 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):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (280409 => 280410)


--- trunk/LayoutTests/ChangeLog	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/LayoutTests/ChangeLog	2021-07-28 23:35:42 UTC (rev 280410)
@@ -1,3 +1,17 @@
+2021-07-28  Sihui Liu  <sihui_...@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_koj...@apple.com>
 
         [ iPad ] platform/ipad/media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html is a flaky timeout.

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (280409 => 280410)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-07-28 23:35:42 UTC (rev 280410)
@@ -1,3 +1,13 @@
+2021-07-28  Sihui Liu  <sihui_...@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_koj...@apple.com>
 
         Unreviewed, reverting r280402.

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any-expected.txt (280409 => 280410)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any-expected.txt	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any-expected.txt	2021-07-28 23:35:42 UTC (rev 280410)
@@ -3,5 +3,5 @@
 PASS Enumerate one database.
 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.
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any.worker-expected.txt (280409 => 280410)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any.worker-expected.txt	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/IndexedDB/get-databases.any.worker-expected.txt	2021-07-28 23:35:42 UTC (rev 280410)
@@ -3,5 +3,5 @@
 PASS Enumerate one database.
 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.
 

Added: trunk/LayoutTests/storage/indexeddb/getdatabases-expected.txt (0 => 280410)


--- trunk/LayoutTests/storage/indexeddb/getdatabases-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/getdatabases-expected.txt	2021-07-28 23:35:42 UTC (rev 280410)
@@ -0,0 +1,18 @@
+Ensure all databases can be retrieved
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB.open('getdatabase-first')
+PASS databases.length is 0
+indexedDB.open('getdatabase-second', 2)
+PASS databases.length is 1
+PASS databases[0].version is 1
+PASS databases[0].name is "getdatabase-first"
+PASS databases.length is 2
+PASS databases[1].version is 2
+PASS databases[1].name is "getdatabase-second"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/storage/indexeddb/getdatabases-private-expected.txt (0 => 280410)


--- trunk/LayoutTests/storage/indexeddb/getdatabases-private-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/getdatabases-private-expected.txt	2021-07-28 23:35:42 UTC (rev 280410)
@@ -0,0 +1,18 @@
+Ensure all databases can be retrieved
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB.open('getdatabase-first')
+PASS databases.length is 0
+indexedDB.open('getdatabase-second', 2)
+PASS databases.length is 1
+PASS databases[0].version is 1
+PASS databases[0].name is "getdatabase-first"
+PASS databases.length is 2
+PASS databases[1].version is 2
+PASS databases[1].name is "getdatabase-second"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/storage/indexeddb/getdatabases-private.html (0 => 280410)


--- trunk/LayoutTests/storage/indexeddb/getdatabases-private.html	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/getdatabases-private.html	2021-07-28 23:35:42 UTC (rev 280410)
@@ -0,0 +1,10 @@
+<!-- webkit-test-runner [ useEphemeralSession=true ] -->
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<script src=""
+</body>
+</html>
\ No newline at end of file

Added: trunk/LayoutTests/storage/indexeddb/getdatabases.html (0 => 280410)


--- trunk/LayoutTests/storage/indexeddb/getdatabases.html	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/getdatabases.html	2021-07-28 23:35:42 UTC (rev 280410)
@@ -0,0 +1,9 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<script src=""
+</body>
+</html>
\ No newline at end of file

Added: trunk/LayoutTests/storage/indexeddb/resources/getdatabases.js (0 => 280410)


--- trunk/LayoutTests/storage/indexeddb/resources/getdatabases.js	                        (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/resources/getdatabases.js	2021-07-28 23:35:42 UTC (rev 280410)
@@ -0,0 +1,45 @@
+if (this.importScripts) {
+    importScripts('../../../resources/js-test.js');
+    importScripts('shared.js');
+}
+
+description("Ensure all databases can be retrieved");
+
+var databases;
+
+function test() {
+    indexedDB.deleteDatabase('getdatabase-first');
+    indexedDB.deleteDatabase('getdatabase-second');
+
+    request = evalAndLog("indexedDB.open('getdatabase-first')");
+    request._onupgradeneeded_ = () => {
+        indexedDB.databases().then((result)=> {
+            databases = result;
+            shouldBeEqualToNumber("databases.length", 0);
+            continueTest();
+        });
+    }
+}
+
+function continueTest() {
+    request = evalAndLog("indexedDB.open('getdatabase-second', 2)");
+    request._onupgradeneeded_ = () => {
+        indexedDB.databases().then((result) => {
+            databases = result;
+            shouldBeEqualToNumber("databases.length", 1);
+            shouldBeEqualToNumber("databases[0].version", 1)
+            shouldBeEqualToString("databases[0].name", "getdatabase-first");
+        });
+    }
+    request._onsuccess_ = () => {
+        indexedDB.databases().then((result) => {
+            databases = result;
+            shouldBeEqualToNumber("databases.length", 2);
+            shouldBeEqualToNumber("databases[1].version", 2);
+            shouldBeEqualToString("databases[1].name", "getdatabase-second");
+            finishJSTest();
+        });
+    }
+}
+
+test();

Modified: trunk/Source/WebCore/ChangeLog (280409 => 280410)


--- trunk/Source/WebCore/ChangeLog	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/Source/WebCore/ChangeLog	2021-07-28 23:35:42 UTC (rev 280410)
@@ -1,3 +1,34 @@
+2021-07-28  Sihui Liu  <sihui_...@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  <mmaxfi...@apple.com>
 
         [GPU Process] Start tracking resource uses for ImageBuffers

Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h (280409 => 280410)


--- trunk/Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h	2021-07-28 23:35:42 UTC (rev 280410)
@@ -91,6 +91,7 @@
 
     virtual bool supportsSimultaneousTransactions() = 0;
     virtual bool isEphemeral() = 0;
+    virtual String fullDatabasePath() const = 0;
 
     virtual void close() = 0;
 

Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp (280409 => 280410)


--- trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp	2021-07-28 23:35:42 UTC (rev 280410)
@@ -508,33 +508,47 @@
         m_uniqueIDBDatabaseMap.remove(uniqueIDBDatabase->identifier());
 }
 
+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)
 {
     ASSERT(!isMainThread());
     ASSERT(m_lock.isHeld());
 
-    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));
+    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");
+    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);
     if (!connection)
         return;
 
-    connection->didGetAllDatabaseNamesAndVersions(requestIdentifier, WTFMove(databases));
+    connection->didGetAllDatabaseNamesAndVersions(requestIdentifier, WTFMove(result));
 }
 
 static void collectOriginsForVersion(const String& versionPath, HashSet<WebCore::SecurityOriginData>& securityOrigins)

Modified: trunk/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h (280409 => 280410)


--- trunk/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h	2021-07-28 23:35:42 UTC (rev 280410)
@@ -80,6 +80,7 @@
 
     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); }
 

Modified: trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h (280409 => 280410)


--- trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h	2021-07-28 23:35:42 UTC (rev 280410)
@@ -86,6 +86,7 @@
 
     bool supportsSimultaneousTransactions() final { return false; }
     bool isEphemeral() final { return false; }
+    String fullDatabasePath() const final;
 
     bool hasTransaction(const IDBResourceIdentifier&) const final;
 
@@ -103,7 +104,6 @@
 
 private:
     String filenameForDatabaseName() const;
-    String fullDatabasePath() const;
     String fullDatabaseDirectoryWithUpgrade();
 
     IDBError ensureValidRecordsTable();

Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp (280409 => 280410)


--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp	2021-07-28 23:35:42 UTC (rev 280410)
@@ -312,7 +312,7 @@
     ASSERT(m_currentOpenDBRequest->isDeleteRequest());
 
     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,
     // we won't have a m_mostRecentDeletedDatabaseInfo. In that case, we'll manufacture one using the
@@ -1240,5 +1240,30 @@
     return nullptr;
 }
 
+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

Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h (280409 => 280410)


--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h	2021-07-28 23:33:04 UTC (rev 280409)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h	2021-07-28 23:35:42 UTC (rev 280410)
@@ -28,6 +28,7 @@
 #include "IDBBackingStore.h"
 #include "IDBDatabaseIdentifier.h"
 #include "IDBDatabaseInfo.h"
+#include "IDBDatabaseNameAndVersion.h"
 #include "IDBGetResult.h"
 #include "ServerOpenDBRequest.h"
 #include "UniqueIDBDatabaseTransaction.h"
@@ -111,6 +112,9 @@
     void abortActiveTransactions();
     bool tryClose();
 
+    String filePath() const;
+    std::optional<IDBDatabaseNameAndVersion> nameAndVersion() const;
+
 private:
     void handleDatabaseOperations();
     void handleCurrentOperation();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to