Cross hash for JS and Native
#3070
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
armanbilge
commented
Nov 30, 2022
| test("sha256")(forAll((s: String) => checkDigest(sha256, "SHA-256", s))) | ||
| test("sha384")(forAll((s: String) => checkDigest(sha384, "SHA-384", s))) | ||
| test("sha512")(forAll((s: String) => checkDigest(sha512, "SHA-512", s))) | ||
| if (isJVM) test("md2")(forAllF((s: String) => checkDigest(md2, "MD2", s))) |
There was a problem hiding this comment.
So, crypto is weird, since ultimately it depends on availability of algorithms at runtime. Seems that default OpenSSL builds (also used by Node.js) don't include MD2. But an alternative build could.
| /** Computes the digest of the source stream, emitting the digest as a chunk | ||
| * after completion of the source stream. | ||
| */ | ||
| private[this] def digest[F[_]](digest: => Ptr[EVP_MD])(implicit F: Sync[F]): Pipe[F, Byte, Byte] = |
There was a problem hiding this comment.
So I caved and added a Sync constraint here, unlike the JVM/JS implementations. Since we are allocating and freeing native memory I didn't see a way around it.
This might end up being annoying in practice, especially for cross-builds. Maybe a capability typeclass Hash[F] would be a better design.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On JS, Node.js only. This is the sole Node.js-only thing in core, but meh. Web Crypto doesn't support streaming hashing yet. Further reading:
On Native, requires OpenSSL.