subtlecrypto: Don't throw exceptions twice when converting to Algorithm object (#34239)

* Don't throw exceptions twice when converting to Algorithm object

Removes match statements like
```rust
let Ok(ConversionResult::Success(algorithm)) = Algorithm::new(cx, value.handle())
else {
    return Err(Error::Syntax);
};
```
These don't cause issues if `Algorithm::new` returns `Ok(ConversionResult::Failure`,
but in the case of `Err(())` the implementation already called `throw_type_error`
and we must not throw an additional Syntax error, otherwise we'll crash.

Luckily, this case is already handled elsewhere by the
`value_from_js_object` macro.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Test that calling subtlecrypto methods with empty algorithm objects throws a TypeError

The WebCryptoAPI spec does not tell us which error to throw exactly, but
according to https://webidl.spec.whatwg.org/ it should be a TypeError.

This previously crashed servo.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

components/script/dom/subtlecrypto.rs

@@ -1126,10 +1126,7 @@ fn normalize_algorithm_for_get_key_length(
     match algorithm {
         AlgorithmIdentifier::Object(obj) => {
             rooted!(in(*cx) let value = ObjectValue(obj.get()));
-            let Ok(ConversionResult::Success(algorithm)) = Algorithm::new(cx, value.handle())
+            let algorithm = value_from_js_object!(Algorithm, cx, value);
-            else {
-                return Err(Error::Syntax);
-            };
 
             let name = algorithm.name.str();
             let normalized_algorithm = if name.eq_ignore_ascii_case(ALG_AES_CBC) ||
@@ -1162,10 +1159,7 @@ fn normalize_algorithm_for_digest(
     let name = match algorithm {
         AlgorithmIdentifier::Object(obj) => {
             rooted!(in(*cx) let value = ObjectValue(obj.get()));
-            let Ok(ConversionResult::Success(algorithm)) = Algorithm::new(cx, value.handle())
+            let algorithm = value_from_js_object!(Algorithm, cx, value);
-            else {
-                return Err(Error::Syntax);
-            };
 
             algorithm.name.str().to_uppercase()
         },
@@ -1191,10 +1185,7 @@ fn normalize_algorithm_for_import_key(
     let name = match algorithm {
         AlgorithmIdentifier::Object(obj) => {
             rooted!(in(*cx) let value = ObjectValue(obj.get()));
-            let Ok(ConversionResult::Success(algorithm)) = Algorithm::new(cx, value.handle())
+            let algorithm = value_from_js_object!(Algorithm, cx, value);
-            else {
-                return Err(Error::Syntax);
-            };
 
             let name = algorithm.name.str().to_uppercase();
             if name == ALG_HMAC {
@@ -1230,9 +1221,7 @@ fn normalize_algorithm_for_derive_bits(
     };
 
     rooted!(in(*cx) let value = ObjectValue(obj.get()));
-    let Ok(ConversionResult::Success(algorithm)) = Algorithm::new(cx, value.handle()) else {
+    let algorithm = value_from_js_object!(Algorithm, cx, value);
-        return Err(Error::Syntax);
-    };
 
     let normalized_algorithm = if algorithm.name.str().eq_ignore_ascii_case(ALG_PBKDF2) {
         let params = value_from_js_object!(Pbkdf2Params, cx, value);
@@ -1260,9 +1249,7 @@ fn normalize_algorithm_for_encrypt_or_decrypt(
     };
 
     rooted!(in(*cx) let value = ObjectValue(obj.get()));
-    let Ok(ConversionResult::Success(algorithm)) = Algorithm::new(cx, value.handle()) else {
+    let algorithm = value_from_js_object!(Algorithm, cx, value);
-        return Err(Error::Syntax);
-    };
 
     let name = algorithm.name.str();
     let normalized_algorithm = if name.eq_ignore_ascii_case(ALG_AES_CBC) {
@@ -1287,10 +1274,7 @@ fn normalize_algorithm_for_sign_or_verify(
     let name = match algorithm {
         AlgorithmIdentifier::Object(obj) => {
             rooted!(in(*cx) let value = ObjectValue(obj.get()));
-            let Ok(ConversionResult::Success(algorithm)) = Algorithm::new(cx, value.handle())
+            let algorithm = value_from_js_object!(Algorithm, cx, value);
-            else {
-                return Err(Error::Syntax);
-            };
 
             algorithm.name.str().to_uppercase()
         },
@@ -1316,9 +1300,7 @@ fn normalize_algorithm_for_generate_key(
     };
 
     rooted!(in(*cx) let value = ObjectValue(obj.get()));
-    let Ok(ConversionResult::Success(algorithm)) = Algorithm::new(cx, value.handle()) else {
+    let algorithm = value_from_js_object!(Algorithm, cx, value);
-        return Err(Error::Syntax);
-    };
 
     let name = algorithm.name.str();
     let normalized_algorithm =

tests/wpt/meta/MANIFEST.json

@@ -358512,7 +358512,7 @@
     },
     "generateKey": {
      "failures.js": [
-      "e0f0279a69bb885eb30cbe086796c281db0245bb",
+      "deaac636a99be570b118d8e8f8096b36a4a0b28f",
       []
      ],
      "successes.js": [
@@ -358526,7 +358526,7 @@
       []
      ],
      "importKey_failures.js": [
-      "bba48401e616a564200570dddb2fe06d762347fe",
+      "077ae076c648b097977cdbc484eaf79db884f7b2",
       []
      ],
      "okp_importKey.js": [
@@ -516888,7 +516888,7 @@
     },
     "digest": {
      "digest.https.any.js": [
-      "379d9311f30247b85e2e5ea8981d1d180fe783a1",
+      "3b0972b1f2bf7d5a285765ab7cf2447acca21467",
       [
        "WebCryptoAPI/digest/digest.https.any.html",
        {

tests/wpt/tests/WebCryptoAPI/digest/digest.https.any.js

@@ -118,6 +118,20 @@
         });
     });
 
+    // Call digest() with empty algorithm object
+    Object.keys(sourceData).forEach(function(size) {
+        promise_test(function(test) {
+            var promise = subtle.digest({}, sourceData[size])
+            .then(function(result) {
+                assert_unreached("digest() with missing algorithm name should have thrown a TypeError");
+            }, function(err) {
+                assert_equals(err.name, "TypeError", "Missing algorithm name should cause TypeError")
+            });
+
+            return promise;
+        }, "empty algorithm object with " + size);
+    });
+
 
     done();
 

tests/wpt/tests/WebCryptoAPI/generateKey/failures.js

@@ -166,6 +166,14 @@ function run_test(algorithmNames) {
         });
     });
 
+    // Empty algorithm should fail with TypeError
+    allValidUsages(["decrypt", "sign", "deriveBits"], true, []) // Small search space, shouldn't matter because should fail before used
+        .forEach(function(usages) {
+            [false, true, "RED", 7].forEach(function(extractable){
+                testError({}, extractable, usages, "TypeError", "Empty algorithm");
+            });
+        });
+
 
     // Algorithms normalize okay, but usages bad (though not empty).
     // It shouldn't matter what other extractable is. Should fail

tests/wpt/tests/WebCryptoAPI/import_export/importKey_failures.js

@@ -192,4 +192,19 @@ function run_test(algorithmNames) {
             });
         });
     });
+
+    // Missing mandatory "name" field on algorithm
+    testVectors.forEach(function(vector) {
+        var name = vector.name;
+        // We just need *some* valid keydata, so pick the first available algorithm.
+        var algorithm = allAlgorithmSpecifiersFor(name)[0];
+        getValidKeyData(algorithm).forEach(function(test) {
+            validUsages(vector, test.format, test.data).forEach(function(usages) {
+                [true, false].forEach(function(extractable) {
+                    testError(test.format, {}, test.data, name, usages, extractable, "TypeError", "Missing algorithm name");
+                });
+            });
+        });
+    });
+
 }