Skip to content

chore(ext/node): strengthen scrypt regression test for 128-byte keylen#34601

Merged
littledivy merged 1 commit into
mainfrom
orch/divybot-353
May 31, 2026
Merged

chore(ext/node): strengthen scrypt regression test for 128-byte keylen#34601
littledivy merged 1 commit into
mainfrom
orch/divybot-353

Conversation

@divybot
Copy link
Copy Markdown
Contributor

@divybot divybot commented May 31, 2026

Summary

The existing log_n > 64 doesn't panic test, added with the AWS-LC scrypt
migration, was misnamed (it actually tests keylen=128 with default options,
not log_n > 64) and resolved its promise unconditionally — so a silent
failure inside the callback would not surface.

Replace it with two assertive tests against the original reproduction from
#27716:

  • scrypt with 128-byte keylen matches Node runs both the async and sync APIs
    with keylen=128 and asserts the output matches Node.js's hash byte-for-byte.
  • scrypt handles concurrent 128-byte keylen calls issues two scrypt calls in
    parallel (mirroring the original report's double panic) and verifies both
    callbacks fire cleanly with identical output.

The Node-compatible behavior itself was already implemented when scrypt
switched to AWS-LC's EVP_PBE_scrypt in #33773; this just hardens the
regression coverage so a future regression would be caught.

Closes denoland/orchid#353

Test plan

  • dprint fmt clean
  • deno lint --config tests/config/deno.json clean
  • Expected hash verified to match Node.js's output and the stable Deno
    2.8.1 release
  • CI: cargo test unit_node::crypto::crypto_scrypt_test

The existing "log_n > 64 doesn't panic" test was misnamed (it tested keylen=128
with default options, not log_n > 64) and resolved its promise regardless of
the callback's error argument, so a silent failure would not surface.

Replace it with two assertive tests against the reproduction from
#27716: one verifying both the async and sync paths produce the
hash Node.js emits, and one running two scrypt calls concurrently (matching
the original report's double panic).

Co-Authored-By: Divy Srivastava <me@littledivy.com>
@littledivy littledivy changed the title test(ext/node): strengthen scrypt regression test for 128-byte keylen chore(ext/node): strengthen scrypt regression test for 128-byte keylen May 31, 2026
@littledivy littledivy enabled auto-merge (squash) May 31, 2026 06:25
@littledivy littledivy merged commit 8c09a51 into main May 31, 2026
137 checks passed
@littledivy littledivy deleted the orch/divybot-353 branch May 31, 2026 06:43
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.

2 participants