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

use immutable buffers in SubtleCrypto methods #3797

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

eric-seppanen
Copy link
Contributor

This change makes all the methods in SubtleCrypto use &[u8] parameters instead of &mut [u8].

Here are the affected parameters:

  • decrypt(data): the WebCrypto API states:

    Let data be the result of getting a copy of the bytes held by the data parameter passed to the decrypt method.

  • digest(data): the WebCrypto API states:

    Let data be the result of getting a copy of the bytes held by the data parameter passed to the digest method.

  • encrypt(data): the WebCrypto API states:

    Let data be the result of getting a copy of the bytes held by the data parameter passed to the encrypt method.

  • sign(data): the WebCrypto API states:

    Let data be the result of getting a copy of the bytes held by the data parameter passed to the sign method.

  • unwrapKey(wrapped_key): the WebCrypto API states:

    Let wrappedKey be the result of getting a copy of the bytes held by the wrappedKey parameter passed to the unwrapKey method.

  • verify(signature, data): the WebCrypto API states:

    Let signature be the result of getting a copy of the bytes held by the signature parameter passed to the verify method.
    Let data be the result of getting a copy of the bytes held by the data parameter passed to the verify method.

I take the API documentation language "getting a copy of the bytes" to mean that the original buffer is left unmodified.

I can't find any evidence that these buffers would be modified. I'm not sure how strong an argument that is.

Fixes #3795.

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

LGTM!
Can you add an entry in the changelog?

@eric-seppanen
Copy link
Contributor Author

Added a changelog entry.

Rebased due to merge conflicts in the changelog; I made no other changes from the previous version.

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Thank you!

@daxpedda daxpedda merged commit 1942791 into rustwasm:main Jan 23, 2024
25 checks passed
@eric-seppanen
Copy link
Contributor Author

eric-seppanen commented May 31, 2024

I realize after the fact that this change made the 0.3.68 release not semver-compatible, because the signatures of some SubtleCrypto methods changed. Example: 0.3.67, 0.3.68.

Apologies for the breakage! Perhaps cargo-semver-checks could have caught this?

@daxpedda
Copy link
Collaborator

daxpedda commented Jun 1, 2024

AFAIK changing a parameter from &T to &mut T should not be a breaking change.

Is that what you meant or did I miss something else?

@eric-seppanen
Copy link
Contributor Author

@daxpedda Oops, you're right. (Though it's &mut T -> &T). I confused myself by copying some code from a workspace using 0.3.68 into another project using 0.3.67 and it didn't compile. But that's not semver breakage, it's just that downgrade doesn't work. Sorry for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SubtleCrypto verify methods accepts mutable reference that should be immutable?
2 participants