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

Remove the 'required' flag in the JsonWebKey's 'kty' field #16869

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

javifernandez
Copy link
Contributor

@javifernandez javifernandez commented Aug 19, 2023

58f6c8c

Remove the 'required' flag in the JsonWebKey's 'kty' field
https://bugs.webkit.org/show_bug.cgi?id=260430

Reviewed by Youenn Fablet.

Marking the 'kty' field as required causes that we throw a TypeError
exception during the IDL-JS type mapping phase. However, the specs of
the different algorithms state that we should throw a DataError instead.

By removing the 'required' flag of the mentioned field we allow the
implementation of each algorithm to deal with the issue, detecting
either the lack of that mandatory field or an incorrect value for the
specific algorithm.

It's worth mentioning that the current implementation of the algorithms
already performs the required checks, throwing a DataError exception
as the spec states. Hence, this change just avoids the early checks and
let the algorithms execute their specific logic.

* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_Ed25519.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_Ed25519.https.any.worker-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_Ed448.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_Ed448.https.any.worker-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_X25519.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_X25519.https.any.worker-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_X448.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_X448.https.any.worker-expected.txt:
* Source/WebCore/crypto/JsonWebKey.idl:

Canonical link: https://commits.webkit.org/267131@main

f215874

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@javifernandez javifernandez added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Aug 19, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 20, 2023
@zdobersek
Copy link
Contributor

The affected tests needs an updated baseline, beyond that @youennf should weigh in.

@zdobersek zdobersek requested review from youennf and removed request for zdobersek August 21, 2023 10:28
@javifernandez javifernandez removed the merging-blocked Applied to prevent a change from being merged label Aug 21, 2023
@javifernandez javifernandez added the merge-queue Applied to send a pull request to merge-queue label Aug 22, 2023
https://bugs.webkit.org/show_bug.cgi?id=260430

Reviewed by Youenn Fablet.

Marking the 'kty' field as required causes that we throw a TypeError
exception during the IDL-JS type mapping phase. However, the specs of
the different algorithms state that we should throw a DataError instead.

By removing the 'required' flag of the mentioned field we allow the
implementation of each algorithm to deal with the issue, detecting
either the lack of that mandatory field or an incorrect value for the
specific algorithm.

It's worth mentioning that the current implementation of the algorithms
already performs the required checks, throwing a DataError exception
as the spec states. Hence, this change just avoids the early checks and
let the algorithms execute their specific logic.

* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_Ed25519.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_Ed25519.https.any.worker-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_Ed448.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_Ed448.https.any.worker-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_X25519.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_X25519.https.any.worker-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_X448.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/okp_importKey_failures_X448.https.any.worker-expected.txt:
* Source/WebCore/crypto/JsonWebKey.idl:

Canonical link: https://commits.webkit.org/267131@main
@webkit-commit-queue
Copy link
Collaborator

Committed 267131@main (58f6c8c): https://commits.webkit.org/267131@main

Reviewed commits have been landed. Closing PR #16869 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 22, 2023
@webkit-commit-queue webkit-commit-queue merged commit 58f6c8c into WebKit:main Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
6 participants