From 96592dce442f1ca6b67015350d2d0409a84954ab Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Thu, 11 Sep 2025 05:48:18 -0700 Subject: [PATCH] Invalid return type for key conversion (#39252) `convert_value_to_key` returns a `ConversionResult` now, so keys can be considered "Invalid" rather than throwing an exception. Testing: WPT Unblocks: #38288 --------- Signed-off-by: Ashwin Naren --- components/script/dom/idbfactory.rs | 4 +-- components/script/dom/idbkeyrange.rs | 12 +++---- components/script/dom/idbobjectstore.rs | 4 +-- components/script/indexed_db.rs | 44 +++++++++++++++++-------- 4 files changed, 41 insertions(+), 23 deletions(-) diff --git a/components/script/dom/idbfactory.rs b/components/script/dom/idbfactory.rs index 39d801be1b9..961d5efbb26 100644 --- a/components/script/dom/idbfactory.rs +++ b/components/script/dom/idbfactory.rs @@ -98,8 +98,8 @@ impl IDBFactoryMethods for IDBFactory { // https://www.w3.org/TR/IndexedDB-2/#dom-idbfactory-cmp fn Cmp(&self, cx: SafeJSContext, first: HandleValue, second: HandleValue) -> Fallible { - let first_key = convert_value_to_key(cx, first, None)?; - let second_key = convert_value_to_key(cx, second, None)?; + let first_key = convert_value_to_key(cx, first, None)?.into_result()?; + let second_key = convert_value_to_key(cx, second, None)?.into_result()?; let cmp = first_key.partial_cmp(&second_key); if let Some(cmp) = cmp { match cmp { diff --git a/components/script/dom/idbkeyrange.rs b/components/script/dom/idbkeyrange.rs index 5036b172e07..7e823fa2a3b 100644 --- a/components/script/dom/idbkeyrange.rs +++ b/components/script/dom/idbkeyrange.rs @@ -71,7 +71,7 @@ impl IDBKeyRangeMethods for IDBKeyRange { global: &GlobalScope, value: HandleValue, ) -> Fallible> { - let key = convert_value_to_key(cx, value, None)?; + let key = convert_value_to_key(cx, value, None)?.into_result()?; let inner = IndexedDBKeyRange::only(key); Ok(IDBKeyRange::new(global, inner, CanGc::note())) } @@ -83,7 +83,7 @@ impl IDBKeyRangeMethods for IDBKeyRange { lower: HandleValue, open: bool, ) -> Fallible> { - let key = convert_value_to_key(cx, lower, None)?; + let key = convert_value_to_key(cx, lower, None)?.into_result()?; let inner = IndexedDBKeyRange::lower_bound(key, open); Ok(IDBKeyRange::new(global, inner, CanGc::note())) } @@ -95,7 +95,7 @@ impl IDBKeyRangeMethods for IDBKeyRange { upper: HandleValue, open: bool, ) -> Fallible> { - let key = convert_value_to_key(cx, upper, None)?; + let key = convert_value_to_key(cx, upper, None)?.into_result()?; let inner = IndexedDBKeyRange::upper_bound(key, open); Ok(IDBKeyRange::new(global, inner, CanGc::note())) } @@ -112,12 +112,12 @@ impl IDBKeyRangeMethods for IDBKeyRange { // Step 1. Let lowerKey be the result of running the steps to convert a value to a key with // lower. Rethrow any exceptions. // Step 2. If lowerKey is invalid, throw a "DataError" DOMException. - let lower_key = convert_value_to_key(cx, lower, None)?; + let lower_key = convert_value_to_key(cx, lower, None)?.into_result()?; // Step 3. Let upperKey be the result of running the steps to convert a value to a key with // upper. Rethrow any exceptions. // Step 4. If upperKey is invalid, throw a "DataError" DOMException. - let upper_key = convert_value_to_key(cx, upper, None)?; + let upper_key = convert_value_to_key(cx, upper, None)?.into_result()?; // Step 5. If lowerKey is greater than upperKey, throw a "DataError" DOMException. if lower_key > upper_key { @@ -134,7 +134,7 @@ impl IDBKeyRangeMethods for IDBKeyRange { // https://www.w3.org/TR/IndexedDB-2/#dom-idbkeyrange-_includes fn Includes(&self, cx: SafeJSContext, value: HandleValue) -> Fallible { - let key = convert_value_to_key(cx, value, None)?; + let key = convert_value_to_key(cx, value, None)?.into_result()?; if self.inner.contains(&key) { return Ok(true); } diff --git a/components/script/dom/idbobjectstore.rs b/components/script/dom/idbobjectstore.rs index a395e482b02..f7563ace040 100644 --- a/components/script/dom/idbobjectstore.rs +++ b/components/script/dom/idbobjectstore.rs @@ -211,7 +211,7 @@ impl IDBObjectStore { let serialized_key: Option; if !key.is_undefined() { - serialized_key = Some(convert_value_to_key(cx, key, None)?); + serialized_key = Some(convert_value_to_key(cx, key, None)?.into_result()?); } else { // Step 11: We should use in-line keys instead if let Some(Ok(ExtractionResult::Key(kpk))) = self @@ -287,7 +287,7 @@ impl IDBObjectStoreMethods for IDBObjectStore { // Step 6 // TODO: Convert to key range instead - let serialized_query = convert_value_to_key(cx, query, None); + let serialized_query = convert_value_to_key(cx, query, None)?.into_result(); // Step 7. Let operation be an algorithm to run delete records from an object store with store and range. // Stpe 8. Return the result (an IDBRequest) of running asynchronously execute a request with this and operation. let (sender, receiver) = indexed_db::create_channel(self.global()); diff --git a/components/script/indexed_db.rs b/components/script/indexed_db.rs index 864f7e7dc0e..4401e79e2af 100644 --- a/components/script/indexed_db.rs +++ b/components/script/indexed_db.rs @@ -101,13 +101,27 @@ pub fn is_valid_key_path(key_path: &StrOrStringSequence) -> bool { } } +pub(crate) enum ConversionResult { + Valid(IndexedDBKeyType), + Invalid, +} + +impl ConversionResult { + pub fn into_result(self) -> Result { + match self { + ConversionResult::Valid(key) => Ok(key), + ConversionResult::Invalid => Err(Error::Data), + } + } +} + // https://www.w3.org/TR/IndexedDB-2/#convert-value-to-key #[allow(unsafe_code)] pub fn convert_value_to_key( cx: SafeJSContext, input: HandleValue, seen: Option>, -) -> Result { +) -> Result { // Step 1: If seen was not given, then let seen be a new empty set. let _seen = seen.unwrap_or_default(); @@ -121,15 +135,17 @@ pub fn convert_value_to_key( // FIXME:(arihant2math) Accept array as well if input.is_number() { if input.to_number().is_nan() { - return Err(Error::Data); + return Ok(ConversionResult::Invalid); } - return Ok(IndexedDBKeyType::Number(input.to_number())); + return Ok(ConversionResult::Valid(IndexedDBKeyType::Number( + input.to_number(), + ))); } if input.is_string() { let string_ptr = std::ptr::NonNull::new(input.to_string()).unwrap(); let key = unsafe { jsstr_to_string(*cx, string_ptr) }; - return Ok(IndexedDBKeyType::String(key)); + return Ok(ConversionResult::Valid(IndexedDBKeyType::String(key))); } if input.is_object() { @@ -149,13 +165,15 @@ pub fn convert_value_to_key( if f.is_nan() { return Err(Error::Data); } - return Ok(IndexedDBKeyType::Date(f)); + return Ok(ConversionResult::Valid(IndexedDBKeyType::Date(f))); } if IsArrayBufferObject(*object) || JS_IsArrayBufferViewObject(*object) { // FIXME:(arihant2math) implement it the correct way (is this correct?) let key = structuredclone::write(cx, input, None)?; - return Ok(IndexedDBKeyType::Binary(key.serialized.clone())); + return Ok(ConversionResult::Valid(IndexedDBKeyType::Binary( + key.serialized.clone(), + ))); } if let ESClass::Array = built_in_class { @@ -166,7 +184,7 @@ pub fn convert_value_to_key( } } - Err(Error::Data) + Ok(ConversionResult::Invalid) } // https://www.w3.org/TR/IndexedDB-2/#convert-a-value-to-a-key-range @@ -191,7 +209,7 @@ pub fn convert_value_to_key_range( if (input.get().is_undefined() || input.get().is_null()) && null_disallowed { return Err(Error::Data); } - let key = convert_value_to_key(cx, input, None)?; + let key = convert_value_to_key(cx, input, None)?.into_result()?; Ok(IndexedDBKeyRange::only(key)) } @@ -403,8 +421,6 @@ pub(crate) fn evaluate_key_path_on_value( /// pub(crate) enum ExtractionResult { Key(IndexedDBKeyType), - // NOTE: Invalid is not used for now. Remove the unused annotation when it is used. - #[expect(unused)] Invalid, Failure, } @@ -434,11 +450,13 @@ pub(crate) fn extract_key( // TODO: implement convert_value_to_multientry_key unimplemented!("multiEntry keys are not yet supported"); }, - _ => convert_value_to_key(cx, r.handle(), None)?, + _ => match convert_value_to_key(cx, r.handle(), None)? { + ConversionResult::Valid(key) => key, + // Step 4. If key is invalid, return invalid. + ConversionResult::Invalid => return Ok(ExtractionResult::Invalid), + }, }; - // TODO: Step 4. If key is invalid, return invalid. - // Step 5. Return key. Ok(ExtractionResult::Key(key)) }