Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement failure tests cases for the EC import operation #48243

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

javifernandez
Copy link
Contributor

Apply some refactoring to the code already used for OPK keys.

Copy link

@Frosne Frosne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@javifernandez javifernandez force-pushed the ec-import-failures branch 3 times, most recently from e340c93 to b2dc94c Compare September 19, 2024 16:14
Copy link

@Frosne Frosne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

return missingJWKFieldKeyData["P-521"];
}

function getInvalidJWKKeyData(algorithm) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function is only for the keys where the public key does not correspond to the private key?

I would prefer a more specific name maybe?

Copy link
Contributor

@panva panva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR implements new tests for the import operation of
ECDH and ECDSA algorithms, defining failure test cases.

Additionally, it applies some refactoring to the code
already used for OPK keys.
@javifernandez
Copy link
Contributor Author

missingJWKFieldKeyData is not defined for ECDSA and ECDH tests, the harness fails

* https://wpt.fyi/results/WebCryptoAPI/import_export/ec_importKey_failures_ECDSA.https.any.html?diff&filter=ADC&run_id=4867596877037568&run_id=5151923644923904

* https://wpt.fyi/results/WebCryptoAPI/import_export/ec_importKey_failures_ECDH.https.any.html?diff&filter=ADC&run_id=4867596877037568&run_id=5151923644923904

Fixed !

Copy link
Contributor

@panva panva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@mozkeeler mozkeeler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, the issue I noted was addressed.

Copy link
Member

@twiss twiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @javifernandez, and thanks all for the reviews!

@twiss twiss merged commit 4f0be15 into web-platform-tests:master Sep 23, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants