Skip to content

crypto: support outputLength option in crypto.hash for XOF functions #58121

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

Merged
merged 1 commit into from
Jul 8, 2025

Conversation

Aditi-1400
Copy link
Contributor

Support outputLength option in crypto.hash() for XOF hash functions to align with the behaviour of crypto.createHash() API

Fixes: #57312

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added 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 May 2, 2025
@panva panva requested review from tniessen and joyeecheung May 2, 2025 14:26
@panva panva added the crypto Issues and PRs related to the crypto subsystem. label May 2, 2025
Copy link
Member

@panva panva left a comment

Choose a reason for hiding this comment

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

@Aditi-1400 please change the PR title and commit message to crypto: support outputLength in crypto.hash for XOF functions

@Aditi-1400 Aditi-1400 changed the title src: support outputLength in XOF hash functions crypto: support outputLength in XOF hash functions May 2, 2025
Copy link

codecov bot commented May 2, 2025

Codecov Report

Attention: Patch coverage is 82.85714% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (f5da8f8) to head (a27fe2b).
Report is 68 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_hash.cc 73.33% 9 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58121      +/-   ##
==========================================
- Coverage   90.08%   90.08%   -0.01%     
==========================================
  Files         640      640              
  Lines      188517   188569      +52     
  Branches    36975    36995      +20     
==========================================
+ Hits       169826   169868      +42     
- Misses      11402    11418      +16     
+ Partials     7289     7283       -6     
Files with missing lines Coverage Δ
lib/internal/crypto/hash.js 98.53% <100.00%> (+0.09%) ⬆️
src/crypto/crypto_hash.cc 70.35% <73.33%> (-0.21%) ⬇️

... and 29 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.

DataPointer xofHashDigest(const Buffer<const unsigned char>& buf,
const EVP_MD* md,
size_t output_length) {
if (md == nullptr) return {};
Copy link
Member

Choose a reason for hiding this comment

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

@codebytere ... can I ask you to take a look at this from a Whether-This-Will-Work-In-BoringSSL perspective?

panva added a commit to panva/node that referenced this pull request Jul 3, 2025
panva added a commit to panva/node that referenced this pull request Jul 3, 2025
panva added a commit to panva/node that referenced this pull request Jul 3, 2025
panva added a commit to panva/node that referenced this pull request Jul 3, 2025
@Aditi-1400 Aditi-1400 force-pushed the crypto-hash branch 3 times, most recently from beb4f68 to 068c2ce Compare July 3, 2025 17:26
@nodejs-github-bot
Copy link
Collaborator

@panva panva requested review from joyeecheung and jasnell July 5, 2025 10:36
@nodejs-github-bot
Copy link
Collaborator

panva added a commit to panva/node that referenced this pull request Jul 5, 2025
Reverts: nodejs#56160
Fixes: nodejs#56159
Fixes: nodejs#58913
Refs: nodejs#58121
PR-URL: nodejs#58942
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
panva added a commit to panva/node that referenced this pull request Jul 5, 2025
Reverts: nodejs#56160
Fixes: nodejs#56159
Fixes: nodejs#58913
Refs: nodejs#58121
PR-URL: nodejs#58942
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@panva panva added the review wanted PRs that need reviews. label Jul 8, 2025
Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

LGTM % the doc nit

@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2025
@joyeecheung joyeecheung added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 8, 2025
@joyeecheung
Copy link
Member

Needs a new CI after the push

@panva panva removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2025
@panva
Copy link
Member

panva commented Jul 8, 2025

@joyeecheung CI has ran with the latest push.

@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2025
@nodejs-github-bot nodejs-github-bot merged commit 1c4fe6d into nodejs:main Jul 8, 2025
68 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 1c4fe6d

RafaelGSS pushed a commit that referenced this pull request Jul 8, 2025
Reverts: #56160
Fixes: #56159
Fixes: #58913
Refs: #58121
PR-URL: #58942
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Jul 8, 2025
Support `outputLength` option in crypto.hash() for XOF hash
functions to align with the behaviour of crypto.createHash()
API

closes: #57312

Co-authored-by: Filip Skokan <[email protected]>
PR-URL: #58121
Fixes: #57312
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
nodejs-github-bot added a commit that referenced this pull request Jul 8, 2025
Notable changes:

crypto:
  * (SEMVER-MINOR) support outputLength option in crypto.hash for XOF functions (Aditi) #58121
doc:
  * (SEMVER-MINOR) add all watch-mode related flags to node.1 (Dario Piotrowicz) #58719
fs:
  * (SEMVER-MINOR) add disposable mkdtempSync (Kevin Gibbons) #58516
permission:
  * (SEMVER-MINOR) propagate permission model flags on spawn (Rafael Gonzaga) #58853
sqlite:
  * (SEMVER-MINOR) add support for readBigInts option in db connection level (Miguel Marcondes Filho) #58697
src,permission:
  * (SEMVER-MINOR) add support to permission.has(addon) (Rafael Gonzaga) #58951
watch:
  * (SEMVER-MINOR) add `--watch-kill-signal` flag (Dario Piotrowicz) #58719

PR-URL: #58993
RafaelGSS pushed a commit that referenced this pull request Jul 9, 2025
Notable changes:

crypto:
  * (SEMVER-MINOR) support outputLength option in crypto.hash for XOF functions (Aditi) #58121
doc:
  * (SEMVER-MINOR) add all watch-mode related flags to node.1 (Dario Piotrowicz) #58719
fs:
  * (SEMVER-MINOR) add disposable mkdtempSync (Kevin Gibbons) #58516
permission:
  * (SEMVER-MINOR) propagate permission model flags on spawn (Rafael Gonzaga) #58853
sqlite:
  * (SEMVER-MINOR) add support for readBigInts option in db connection level (Miguel Marcondes Filho) #58697
src,permission:
  * (SEMVER-MINOR) add support to permission.has(addon) (Rafael Gonzaga) #58951
watch:
  * (SEMVER-MINOR) add `--watch-kill-signal` flag (Dario Piotrowicz) #58719

PR-URL: #58993
aduh95 pushed a commit to panva/node that referenced this pull request Jul 21, 2025
Reverts: nodejs#56160
Fixes: nodejs#56159
Fixes: nodejs#58913
Refs: nodejs#58121
PR-URL: nodejs#58942
Backport-PR-URL: nodejs#58960
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@aduh95 aduh95 added the backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. label Jul 21, 2025
@aduh95
Copy link
Contributor

aduh95 commented Jul 21, 2025

This doesn't land cleanly on v22.x-staging

@B-Scott33

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. review wanted PRs that need reviews. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crypto.hash XOF hash functions outputLength option
8 participants