Skip to content
New issue

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

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

Already on GitHub? # to your account

Add basic impl for Node.js KeyObject equals #629

Merged
merged 1 commit into from
May 11, 2023

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented May 10, 2023

Since we're using CryptoKey under the covers of KeyObject, we need to teach the various CryptoKey::Impl subclasses how to compare their contents.

@jasnell jasnell requested a review from fhanau May 10, 2023 16:49
@fhanau
Copy link
Collaborator

fhanau commented May 11, 2023

There should be potential here to reduce duplication of the equals() methods for the symmetric key algorithms, but I guess it would require another class and might not be worth it.

@jasnell
Copy link
Member Author

jasnell commented May 11, 2023

Yeah I'll be revisiting that later. Just not sure if the additional class and virtual method are worth it.

Since we're using CryptoKey under the covers of KeyObject, we
need to teach the various CryptoKey::Impl subclasses how to
compare their contents.
@jasnell jasnell force-pushed the jsnell/nodejs-crypto-keyobject-equals branch from ae2e45c to af9fbc6 Compare May 11, 2023 14:16
@jasnell jasnell merged commit dd77cc9 into main May 11, 2023
@fhanau fhanau deleted the jsnell/nodejs-crypto-keyobject-equals branch December 4, 2023 22:18
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants