[WebCryptoAPI] Add tests for argument read order by twiss · Pull Request #57614 · web-platform-tests/wpt

twiss · 2026-02-06T15:35:36Z

Add tests for the reading order of arguments in Web Crypto, updated in w3c/webcrypto#426 to match the majority of implementations.

To be merged after w3c/webcrypto#426.

panva · 2026-02-16T11:42:55Z

WebCryptoAPI/digest/cshake.tentative.https.any.js

+              get name() {
+                // Alter the buffer back while calling digest
+                buffer[0] = ~buffer[0];
+                return alg;
+              },


Shouldn't this and all the others using the same pattern truly alter the buffer back instead of continuously switching it back and forth? Otherwise it's dependant on that the name is only accessed once.

Ah, interesting question. It's true that it wasn't my intention for this test to check that name is accessed only once, however, I do think WebIDL and WebCrypto are defined as such, because the property is accessed once in step 4 of normalizing an algorithm, and then never thereafter. So I think it should be fine to rely on this?

Then again, it might make it harder to debug what's the cause of this test failing if it does, and of course this is not quite a proper test of the property being accessed once as it would also pass if it's accessed three times :D So, we could also split that up, if we think the latter is worth testing.

it might make it harder to debug what's the cause of this test failing if it does

Exactly where i'm coming from. I'm not interested in testing that algorithm properties are only accessed once. Nor in the order itself FWIW but here we are. Optimizations that we have in Node.js rule out this proposed test pattern.

Well we might need a separate test then because it sounds like Node.js might not have a conforming bindings implementation?

All i'm asking is that the getter "switches back" instead of "switching back and forth".

OK, I've updated the tests. We can separately add more tests for the bindings / argument normalization later.

I understand what you're asking for, but since you're supposed to only call these once and in order, we'd be hiding a bug of sorts by removing coverage for that.

Rather than flipping the data on each call to the argument name getter function, restore it to the original value. This makes it easier to distinguish the cause of test failures if the argument name getter is called multiple times.

annevk

By-and-large this seems good, though not sure we needed this many tests. Quality of JS could be improved a bit while here, but I suppose that's largely up to you all.

annevk · 2026-02-16T16:52:07Z

WebCryptoAPI/digest/cshake.tentative.https.any.js

+              get name() {
+                // Alter the buffer back while calling digest
+                buffer[0] = ~buffer[0];
+                return alg;
+              },


I understand what you're asking for, but since you're supposed to only call these once and in order, we'd be hiding a bug of sorts by removing coverage for that.

annevk · 2026-02-16T16:52:26Z

WebCryptoAPI/digest/cshake.tentative.https.any.js

-            if (buffer.length > 0) {
+      if (sourceData[size].length > 0) {
+        promise_test(function (test) {
+          var buffer = new Uint8Array(sourceData[size]);


annevk · 2026-02-16T16:53:04Z

WebCryptoAPI/digest/cshake.tentative.https.any.js

-            // Alter the buffer after calling digest
-            if (buffer.length > 0) {
+      if (sourceData[size].length > 0) {
+        promise_test(function (test) {


If we are going to reformat anyway, might as well adopt async/await?

I didn't really reformat anything here, I just added an if around it. I can refactor this and other tests in a followup.

annevk · 2026-02-16T16:55:53Z

WebCryptoAPI/digest/cshake.tentative.https.any.js

+            .digest({ name: alg, length: length }, buffer)
+            .then(function (result) {
+              // Alter the buffer after calling digest
              buffer[0] = ~buffer[0];


We should also alter it immediately after calling digest. In fact, that would be the superior test as this would be redundant with that.

Yeah this test is wrong, I fixed that in #57616.

twiss · 2026-02-16T20:44:37Z

Fair enough :D I mostly just copy+pasted all the instances of the "modify the buffer after the call" tests and modified them mechanically. I also thought having all the algorithms at least might be useful since some implementations implement some of them synchronously and others not, but it seems the implementations are consistent between algorithms at least, indeed.

wpt-pr-bot · 2026-02-26T13:59:35Z

There are no reviewers for this pull request besides its author. Please reach out on the chat room to get help with this. Thank you!

The specification of WebCrypto and Modern Algorithms in WebCrypto has updated to reorder reading algorithm and data arguments in "encrypt", "decrypt", "sign", "verify", "digest", "importKey", "unwrapKey", "decapsulateKey" and "decapsulateBits" methods. WebCrypto: w3c/webcrypto@5b57233 Modern Algorithms in WebCrypto: WICG/webcrypto-modern-algos@ae72ee6 This patch updates our implementation accordingly. Testing: Pass new WPT tests added in web-platform-tests/wpt#57614 and servo#42925 Signed-off-by: Kingsley Yung <[email protected]>

…2927) The specification of WebCrypto and Modern Algorithms in WebCrypto has updated to reorder reading algorithm and data arguments in "encrypt", "decrypt", "sign", "verify", "digest", "importKey", "unwrapKey", "decapsulateKey" and "decapsulateBits" methods. This patch updates our implementation accordingly. The relevant commits in specification repositories: - WebCrypto: w3c/webcrypto@5b57233 - Modern Algorithms in WebCrypto: WICG/webcrypto-modern-algos@ae72ee6 Testing: Pass new WPT tests added in web-platform-tests/wpt#57614 and #42925 Signed-off-by: Kingsley Yung <[email protected]>

…rvo#42927) The specification of WebCrypto and Modern Algorithms in WebCrypto has updated to reorder reading algorithm and data arguments in "encrypt", "decrypt", "sign", "verify", "digest", "importKey", "unwrapKey", "decapsulateKey" and "decapsulateBits" methods. This patch updates our implementation accordingly. The relevant commits in specification repositories: - WebCrypto: w3c/webcrypto@5b57233 - Modern Algorithms in WebCrypto: WICG/webcrypto-modern-algos@ae72ee6 Testing: Pass new WPT tests added in web-platform-tests/wpt#57614 and servo#42925 Signed-off-by: Kingsley Yung <[email protected]>

twiss requested a review from annevk February 6, 2026 15:35

twiss mentioned this pull request Feb 6, 2026

Reorder reading algorithm and data arguments w3c/webcrypto#426

Merged

[WebCryptoAPI] Add tests for argument read order

56594eb

twiss force-pushed the webcrypto-argument-read-order branch from f880c2d to 56594eb Compare February 6, 2026 15:53

This was referenced Feb 6, 2026

[WebCryptoAPI] Fix tests of SHA3 and cSHAKE with mutated buffers #57616

Merged

[WebCryptoAPI] Add tests for transferred ArrayBuffers #57656

Merged

panva reviewed Feb 16, 2026

View reviewed changes

annevk approved these changes Feb 16, 2026

View reviewed changes

twiss marked this pull request as ready for review February 26, 2026 13:59

wpt-pr-bot added WebCryptoAPI wg-webcrypto labels Feb 26, 2026

wpt-pr-bot assigned twiss Feb 26, 2026

wpt-pr-bot added the status:needs-reviewers label Feb 26, 2026

twiss merged commit 42e4732 into web-platform-tests:master Feb 26, 2026
25 checks passed

twiss deleted the webcrypto-argument-read-order branch February 26, 2026 14:01

kkoyung mentioned this pull request Mar 1, 2026

script: Reorder reading algorithm and data arguments in WebCrypto servo/servo#42927

Merged

Conversation

twiss commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

panva Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

panva Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twiss commented Feb 16, 2026

Uh oh!

wpt-pr-bot commented Feb 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

twiss commented Feb 6, 2026 •

edited

Loading

panva Feb 16, 2026 •

edited

Loading

panva Feb 16, 2026 •

edited

Loading