Skip to content

crypto: remove CipherBase::Init#57787

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.

Sign up for GitHub

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

Jump to bottom
Merged
nodejs-github-bot merged 1 commit into from
Apr 9, 2025
Merged

crypto: remove CipherBase::Init #57787

nodejs-github-bot merged 1 commit into from
Apr 9, 2025

Conversation

@tniessen
Copy link
Member

@tniessentniessen commented Apr 7, 2025

As far as I can tell, the iv parameter can never be undefined (but it can be null!), so this code appears to have been dead since Node.js 22.

This change removes dead code and adds a tiny test case for passing undefined as the IV.

Refs: #50973

@tniessen
crypto: remove CipherBase::Init
5c90482
As far as I can tell, the `iv` parameter can never be `undefined` (but it can be `null`!), so this code appears to have been dead since Node.js 22. This change removes dead code and adds a tiny test case for passing `undefined` as the IV. Refs: nodejs#50973
@tniessentniessen requested review from jasnell and panvaApril 7, 2025 22:00
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 7, 2025

Review requested:

  • @nodejs/crypto

@nodejs-github-botnodejs-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 Apr 7, 2025
@codecov
Copy link

codecovbot commented Apr 7, 2025•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.24%. Comparing base (b85505d) to head (5c90482).
Report is 29 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #57787 +/- ## ========================================== - Coverage 90.24% 90.24% -0.01%  ========================================== Files 630 630 Lines 185198 185249 +51 Branches 36295 36329 +34 ========================================== + Hits 167140 167173 +33 + Misses 10987 10974 -13 - Partials 7071 7102 +31
Files with missing linesCoverage Δ
lib/internal/crypto/cipher.js98.27% <100.00%> (+0.65%)⬆️
src/crypto/crypto_cipher.cc77.05% <ø> (+4.59%)⬆️
src/crypto/crypto_cipher.h60.00% <ø> (ø)

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

@tniessentniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 7, 2025
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 7, 2025
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@tniessentniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 7, 2025
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@tniessentniessen mentioned this pull request Apr 8, 2025
Merged
@tniessentniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 8, 2025
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 8, 2025
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 9, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/66156/

@aduh95aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 9, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 9, 2025
@nodejs-github-botnodejs-github-bot merged commit 9bbbe60 into nodejs:mainApr 9, 2025
80 checks passed
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 9, 2025

Landed in 9bbbe60

JonasBa pushed a commit to JonasBa/node that referenced this pull request Apr 11, 2025
@tniessen@JonasBa
crypto: remove CipherBase::Init
fdc7c82
As far as I can tell, the `iv` parameter can never be `undefined` (but it can be `null`!), so this code appears to have been dead since Node.js 22. This change removes dead code and adds a tiny test case for passing `undefined` as the IV. Refs: nodejs#50973 PR-URL: nodejs#57787 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@tniessen@RafaelGSS
crypto: remove CipherBase::Init
fcffbee
As far as I can tell, the `iv` parameter can never be `undefined` (but it can be `null`!), so this code appears to have been dead since Node.js 22. This change removes dead code and adds a tiny test case for passing `undefined` as the IV. Refs: #50973 PR-URL: #57787 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
@RafaelGSSRafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@tniessen@RafaelGSS
crypto: remove CipherBase::Init
4793bb2
As far as I can tell, the `iv` parameter can never be `undefined` (but it can be `null`!), so this code appears to have been dead since Node.js 22. This change removes dead code and adds a tiny test case for passing `undefined` as the IV. Refs: #50973 PR-URL: #57787 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
@aduh95aduh95 added the backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. label May 6, 2025
@aduh95
Copy link
Contributor

aduh95 commented May 6, 2025

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

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

Reviewers

@panvapanvapanva approved these changes

@jasnelljasnelljasnell approved these changes

@anonriganonriganonrig approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.backport-requested-v22.xPRs awaiting manual backport to the v22.x-staging branch.c++Issues and PRs that require attention from people who are familiar with C++.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@tniessen@nodejs-github-bot@aduh95@panva@jasnell@anonrig