GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix crypto-rsa code by AmitPrajapati-1 · Pull Request #57572 · nodejs/node

Conversation

AmitPrajapati-1 commented Mar 21, 2025

Copy link

Copy Markdown

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

AmitPrajapati-1 added 4 commits

March 20, 2025 15:26


          Fix RSA-OAEP encryption for empty strings

4eb99a4


          Fix lint errors and add hasCrypto check in test-crypto-private-decryp…

756185d

…t.js


          fix empty string bug


          fix code of crypto-rsa

59cb44b

nodejs-github-bot commented Mar 21, 2025

Copy link

Copy Markdown

Collaborator

Review requested:

@nodejs/crypto

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

crypto

Issues and PRs related to the crypto subsystem.

needs-ci PRs that need a full CI run. labels

Mar 21, 2025

This was referenced Mar 21, 2025

Fix RSA-OAEP encryption for empty strings #57558

Closed

rsa-oaep decrypt the ciphertext (an empty string) got an unexpected string #57553

Closed

panva requested changes

Mar 21, 2025

View reviewed changes

panva left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This does not fix the underlying issue introduced somewhere in the ncrypto refactors. Checking that all bytes are 0 and then returning a zero-byte buffer is not a fix to the issue and it introduces more edge cases than it solves. i.e. an n-byte empty buffer will now always return 0-byte buffer.

#57553 (comment)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

aduh95 reviewed

Mar 21, 2025

View reviewed changes

Comment thread

package-lock.json

aduh95 Mar 21, 2025

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Could you please remove this?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

aduh95 reviewed

Mar 21, 2025

View reviewed changes

Comment thread

test/parallel/package-lock.json

aduh95 Mar 21, 2025

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Could you please remove this?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

AmitPrajapati-1 commented Mar 21, 2025

Copy link

Copy Markdown

Author

rather than converting empty string. we can Throwing appropriate errors with meaningful messages instead of trying to handle empty strings by returning zero-byte buffers.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AmitPrajapati-1 requested a review from panva

March 21, 2025 08:58

panva requested changes

Mar 21, 2025

View reviewed changes

panva left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

My original review stands.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

panva added a commit to panva/node that referenced this pull request

Mar 21, 2025


          crypto: fix output of privateDecrypt with zero-length data

057f4c5

closes nodejs#57553
closes nodejs#57572
closes nodejs#57558

panva mentioned this pull request

Mar 21, 2025

crypto: fix output of privateDecrypt with zero-length data #57575

Merged

panva commented Mar 21, 2025

Copy link

Copy Markdown

Member

Fix in #57575

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

panva closed this

Mar 31, 2025

nodejs-github-bot pushed a commit that referenced this pull request

Apr 1, 2025


          crypto: fix output of privateDecrypt with zero-length data

12b81df

closes #57553
closes #57572
closes #57558

PR-URL: #57575
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>

aduh95 pushed a commit that referenced this pull request

Apr 1, 2025


          crypto: fix output of privateDecrypt with zero-length data

7611fc1

closes #57553
closes #57572
closes #57558

PR-URL: #57575
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>

RafaelGSS pushed a commit that referenced this pull request

May 1, 2025


          crypto: fix output of privateDecrypt with zero-length data

33e4380

closes #57553
closes #57572
closes #57558

PR-URL: #57575
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>

RafaelGSS pushed a commit that referenced this pull request

May 2, 2025


          crypto: fix output of privateDecrypt with zero-length data

913a283

closes #57553
closes #57572
closes #57558

PR-URL: #57575
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>

Reviewers

aduh95

aduh95 left review comments

panva

panva requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++

Issues and PRs that require attention from people who are familiar with C++.

crypto

Issues and PRs related to the crypto subsystem.

needs-ci

PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

AmitPrajapati-1 commented Mar 21, 2025

Uh oh!

nodejs-github-bot commented Mar 21, 2025

Uh oh!

panva left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aduh95 Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

aduh95 Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

AmitPrajapati-1 commented Mar 21, 2025

Uh oh!

panva left a comment

Choose a reason for hiding this comment

Uh oh!

panva commented Mar 21, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

panva left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.