script: Check if IndexedDB key path is ECMAScript identifier name (#39255)

From IndexedDB spec, when we check whether a key path is valid, we have
to check whether it is an ECMAScript identifier name. We have not yet
implemented this logic, and always return true.

This patch uses the function `js::rust::wrappers::JS_IsIdentifier` to
achieve this checking.

Testing: Pass WPT tests that were expected to fail.
Fixes: #25324

---------

Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>

components/script/dom/idbdatabase.rs

@@ -211,7 +211,7 @@ impl IDBDatabaseMethods<crate::DomTypeHolder> for IDBDatabase {
 
         // Step 5
         if let Some(path) = key_path {
-            if !is_valid_key_path(path) {
+            if !is_valid_key_path(path)? {
                 return Err(Error::Syntax(None));
             }
         }

components/script/indexed_db.rs

@@ -7,6 +7,7 @@ use std::iter::repeat_n;
 use std::ptr;
 
 use ipc_channel::ipc::IpcSender;
+use itertools::Itertools;
 use js::conversions::jsstr_to_string;
 use js::gc::MutableHandle;
 use js::jsapi::{
@@ -14,7 +15,7 @@ use js::jsapi::{
     JS_IsArrayBufferViewObject, JS_NewObject, NewDateObject,
 };
 use js::jsval::{DoubleValue, UndefinedValue};
-use js::rust::wrappers::{IsArrayObject, JS_GetProperty, JS_HasOwnProperty};
+use js::rust::wrappers::{IsArrayObject, JS_GetProperty, JS_HasOwnProperty, JS_IsIdentifier};
 use js::rust::{HandleValue, MutableHandleValue};
 use net_traits::indexeddb_thread::{BackendResult, IndexedDBKeyRange, IndexedDBKeyType};
 use profile_traits::ipc;
@@ -78,24 +79,58 @@ pub fn key_type_to_jsval(
     }
 }
 
-// https://www.w3.org/TR/IndexedDB-2/#valid-key-path
-pub fn is_valid_key_path(key_path: &StrOrStringSequence) -> bool {
-    fn is_identifier(_s: &str) -> bool {
-        // FIXME: (arihant2math)
-        true
-    }
+/// <https://www.w3.org/TR/IndexedDB-2/#valid-key-path>
+pub(crate) fn is_valid_key_path(key_path: &StrOrStringSequence) -> Result<bool, Error> {
+    // <https://tc39.es/ecma262/#prod-IdentifierName>
+    #[allow(unsafe_code)]
+    let is_identifier_name = |name: &str| -> Result<bool, Error> {
+        let cx = GlobalScope::get_cx();
+        rooted!(in(*cx) let mut value = UndefinedValue());
+        name.safe_to_jsval(cx, value.handle_mut());
+        rooted!(in(*cx) let string = value.to_string());
 
-    let is_valid = |path: &DOMString| {
-        path.is_empty() || is_identifier(path) || path.split(".").all(is_identifier)
+        unsafe {
+            let mut is_identifier = false;
+            if !JS_IsIdentifier(*cx, string.handle(), &mut is_identifier) {
+                return Err(Error::JSFailed);
+            }
+            Ok(is_identifier)
+        }
+    };
+
+    // A valid key path is one of:
+    let is_valid = |path: &DOMString| -> Result<bool, Error> {
+        // An empty string.
+        let is_empty_string = path.is_empty();
+
+        // An identifier, which is a string matching the IdentifierName production from the
+        // ECMAScript Language Specification [ECMA-262].
+        let is_identifier = is_identifier_name(path)?;
+
+        // A string consisting of two or more identifiers separated by periods (U+002E FULL STOP).
+        let is_identifier_list = path
+            .split(".")
+            .map(is_identifier_name)
+            .try_collect::<bool, Vec<bool>, Error>()?
+            .iter()
+            .all(|&value| value);
+
+        Ok(is_empty_string || is_identifier || is_identifier_list)
     };
 
     match key_path {
         StrOrStringSequence::StringSequence(paths) => {
+            // A non-empty list containing only strings conforming to the above requirements.
             if paths.is_empty() {
-                return false;
+                Ok(false)
+            } else {
+                Ok(paths
+                    .iter()
+                    .map(is_valid)
+                    .try_collect::<bool, Vec<bool>, Error>()?
+                    .iter()
+                    .all(|&value| value))
             }
-
-            paths.iter().all(is_valid)
         },
         StrOrStringSequence::String(path) => is_valid(path),
     }

tests/wpt/meta/IndexedDB/idbdatabase-createObjectStore-exception-order.any.js.ini

@@ -2,9 +2,6 @@
   [IDBDatabase.createObjectStore exception order: InvalidStateError vs. TransactionInactiveError]
     expected: FAIL
 
-  [IDBDatabase.createObjectStore exception order: SyntaxError vs. ConstraintError]
-    expected: FAIL
-
 
 [idbdatabase-createObjectStore-exception-order.any.sharedworker.html]
   expected: ERROR
@@ -15,6 +12,3 @@
 [idbdatabase-createObjectStore-exception-order.any.html]
   [IDBDatabase.createObjectStore exception order: InvalidStateError vs. TransactionInactiveError]
     expected: FAIL
-
-  [IDBDatabase.createObjectStore exception order: SyntaxError vs. ConstraintError]
-    expected: FAIL

tests/wpt/meta/IndexedDB/idbdatabase_createObjectStore.any.js.ini

@@ -8,12 +8,6 @@
   [Attempt to create an object store outside of a version change transaction ]
     expected: FAIL
 
-  [Attempt to create an object store with an invalid key path ]
-    expected: FAIL
-
-  [autoInc and keyPath object]
-    expected: FAIL
-
 
 [idbdatabase_createObjectStore.any.worker.html]
   [Both with empty name]
@@ -22,12 +16,6 @@
   [Attempt to create an object store outside of a version change transaction ]
     expected: FAIL
 
-  [Attempt to create an object store with an invalid key path ]
-    expected: FAIL
-
-  [autoInc and keyPath object]
-    expected: FAIL
-
 
 [idbdatabase_createObjectStore.any.serviceworker.html]
   expected: ERROR

tests/wpt/meta/IndexedDB/transaction-deactivation-timing.any.js.ini

@@ -16,12 +16,6 @@
   [Deactivation of new transactions happens at end of invocation]
     expected: FAIL
 
-  [New transactions are deactivated before next task]
-    expected: FAIL
-
-  [New transactions from microtask are deactivated before next task]
-    expected: FAIL
-
 
 [transaction-deactivation-timing.any.sharedworker.html]
   expected: ERROR