fix(crypto): #1076 createHmac/createHash silent empty on non-literal alg - #1080
Merged
Conversation
…for-of alg `crypto.createHmac(alg, key).update(data).digest(enc)` silently returned "" whenever `alg` wasn't an inline `Expr::String` literal — const-bound names, for-of bindings, ternaries, function-returned algs all matched the chain-collapse fast path's `_ =>` catch-all (which returned an empty string). HMAC webhook verification, TOTP, and any code that picks the algorithm dynamically silently produced wrong answers instead of failing. Fix: - Replace the catch-all empty-string return with a runtime handle path that allocates an Hmac/Hash handle (`js_crypto_create_hmac` / `js_crypto_create_hash`) and invokes `.update(data).digest(enc)` via `js_native_call_method`. This covers all non-literal alg cases AND the algorithms the fast path doesn't have direct FFI helpers for (sha1, sha512, md5 for HMAC; sha1, sha512 for hash). - Add `HmacHandle` + `js_crypto_create_hmac` + `dispatch_hmac` in perry-stdlib mirroring the existing `HashHandle` + `js_crypto_create_hash` + `dispatch_hash` shape. Supports sha1/sha256/sha512/md5 + hex/base64/ base64url/binary digest encodings. - Register `HmacHandle` in `js_handle_method_dispatch` for `update` / `digest`. - Add a standalone `Expr::Call` arm for `crypto.createHmac(alg, key)` mirroring the existing standalone `createHash` arm — covers `const h = createHmac(...); h.update().digest()` where the chain is split across statements. Verified byte-for-byte against `node --experimental-strip-types` for inline-literal, for-of, const-ref, ternary, and bound-then-used patterns in `test-files/test_crypto_hmac_dynamic_alg.ts`. Existing `test_gap_node_crypto_buffer.ts` still passes (literal-alg fast path unchanged).
4 tasks
This file contains hidden or 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
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.
Closes #1076.
Summary
crypto.createHmac(alg, key).update(data).digest(enc)silently returned""wheneveralgwasn't an inlineExpr::Stringliteral — const-bound names, for-of bindings, ternaries, etc. HMAC webhook verification, TOTP, and any code that picked the algorithm dynamically silently produced wrong answers instead of failing..update(data).digest(enc)viajs_native_call_method. Covers all non-literal alg cases plus the algorithms the fast path doesn't have direct FFI helpers for (sha1, sha512, md5 for HMAC; sha1, sha512 for hash).HmacHandle+js_crypto_create_hmac+dispatch_hmacinperry-stdlibmirroring the existingHashHandleshape. Supports sha1/sha256/sha512/md5 + hex/base64/base64url/binary digest encodings.crypto.createHmac(alg, key)mirroring the existing standalonecreateHasharm — coversconst h = createHmac(...); h.update().digest()split across statements.Approach
Option C (per the task brief): runtime dispatch fallback for non-literal alg, plus a standalone handle-based codegen path for the bound-then-used pattern. The literal-
"sha256"fast path (createHash sha256/md5, createHmac sha256) stays unchanged for the common case.Test plan
cargo test --release -p perry-codegen --test manifest_consistency— 4/4 passcargo build --release -p perry-runtime -p perry-stdlib -p perrytest-files/test_crypto_hmac_dynamic_alg.tscovers inline-literal, for-of, const-ref, ternary, bound-then-used, and createHash const-ref. Byte-for-byte match withnode --experimental-strip-types.test-files/test_gap_node_crypto_buffer.tsstill passes (literal-alg fast path unchanged).Sibling APIs
createHmac(algorithms: sha1, sha256, sha512, md5) — fixed via handle path.createHash(algorithms: sha1, sha256, sha512, md5) — non-literal alg now flows through the same handle path that the standalone arm already used; previously the chain-collapse caught it and returned"".createCipheriv,createDecipheriv,createSign,createVerify— out of scope for this PR. The const-binding aspect described in the issue would affect them similarly, but they're tracked elsewhere (crypto.createCipheriv not wired in codegen (AES-GCM blocker for envelope encryption) #1075 is the active issue forcreateCipherivAES-GCM wiring; signed APIs would need fresh handle types and FFI). Worth a follow-up sweep but not bundled here to keep this PR focused on the reported failure.No version bump or changelog change (per worktree rules — maintainer applies at merge time).