Skip to content

crypto: improve native error handling and key synchronization - #64851

Open
panva wants to merge 4 commits into
nodejs:mainfrom
panva:fixes-src
Open

crypto: improve native error handling and key synchronization#64851
panva wants to merge 4 commits into
nodejs:mainfrom
panva:fixes-src

Conversation

@panva

@panva panva commented Jul 30, 2026

Copy link
Copy Markdown
Member

3 improvements in src/crypto

  • report DH failures instead of aborting or returning an empty secret
  • create KeyObjectData mutex before sharing key data so every copy uses one lock
  • return an operation error when XOF output allocation fails

panva added 3 commits July 30, 2026 22:04
Report DH failures instead of aborting or returning an empty secret.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Create the mutex before sharing key data so every copy uses one lock.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Return an operation error when XOF output allocation fails.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
@panva
panva requested review from anonrig and jasnell July 30, 2026 20:13
@panva panva added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Jul 30, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 30, 2026
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
@panva panva added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 30, 2026
@nodejs-github-bot

This comment was marked as outdated.

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 31.57895% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.16%. Comparing base (e7904d4) to head (a103304).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_dh.cc 0.00% 4 Missing and 3 partials ⚠️
src/crypto/crypto_turboshake.cc 25.00% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64851      +/-   ##
==========================================
+ Coverage   90.14%   90.16%   +0.01%     
==========================================
  Files         746      746              
  Lines      242764   242773       +9     
  Branches    45739    45767      +28     
==========================================
+ Hits       218848   218889      +41     
+ Misses      15426    15365      -61     
- Partials     8490     8519      +29     
Files with missing lines Coverage Δ
src/crypto/crypto_keys.cc 70.77% <100.00%> (+0.06%) ⬆️
src/crypto/crypto_turboshake.cc 79.94% <25.00%> (-1.33%) ⬇️
src/crypto/crypto_dh.cc 65.97% <0.00%> (-1.20%) ⬇️

... and 37 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants