Skip to content
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

src,crypto: refactor crypto_dh.cc #42492

Closed

Conversation

RaisinTen
Copy link
Contributor

src,crypto: remove uses of AllocatedBuffer from crypto_dh.cc

Refs: #39941

src: remove unnecessary static qualifier in crypto_dh.cc

ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

src,crypto: handle empty maybe correctly in crypto_dh.cc

Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen raisinten@gmail.com

Refs: nodejs#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
@nodejs-github-bot
Copy link
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++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Mar 27, 2022
@nodejs-github-bot

This comment was marked as outdated.

@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 27, 2022
@nodejs-github-bot
Copy link
Collaborator

@RaisinTen RaisinTen added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 30, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 30, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/42492
✔  Done loading data for nodejs/node/pull/42492
----------------------------------- PR info ------------------------------------
Title      src,crypto: refactor `crypto_dh.cc` (#42492)
Author     Darshan Sen  (@RaisinTen)
Branch     RaisinTen:src,crypto/refactor-crypto_dh -> nodejs:master
Labels     crypto, c++, author ready, needs-ci
Commits    3
 - src,crypto: remove uses of AllocatedBuffer from crypto_dh.cc
 - src: remove unnecessary static qualifier in crypto_dh.cc
 - src,crypto: handle empty maybe correctly in crypto_dh.cc
Committers 1
 - Darshan Sen 
PR-URL: https://github.com/nodejs/node/pull/42492
Refs: https://github.com/nodejs/node/pull/39941
Reviewed-By: Tobias Nießen 
Reviewed-By: Anna Henningsen 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/42492
Refs: https://github.com/nodejs/node/pull/39941
Reviewed-By: Tobias Nießen 
Reviewed-By: Anna Henningsen 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 27 Mar 2022 13:25:58 GMT
   ✔  Approvals: 2
   ✔  - Tobias Nießen (@tniessen) (TSC): https://github.com/nodejs/node/pull/42492#pullrequestreview-922458162
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/42492#pullrequestreview-925243682
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-03-27T21:15:26Z: https://ci.nodejs.org/job/node-test-pull-request/43203/
- Querying data for job/node-test-pull-request/43203/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/master up to date...
From https://github.com/nodejs/node
 * branch                  master     -> FETCH_HEAD
✔  origin/master is now up-to-date
- Downloading patch for 42492
From https://github.com/nodejs/node
 * branch                  refs/pull/42492/merge -> FETCH_HEAD
✔  Fetched commits as 85f679fff101..c1ac41dce2ec
--------------------------------------------------------------------------------
[master 4e0135cabd] src,crypto: remove uses of AllocatedBuffer from crypto_dh.cc
 Author: Darshan Sen 
 Date: Sun Mar 27 18:02:52 2022 +0530
 1 file changed, 52 insertions(+), 24 deletions(-)
[master e572e53817] src: remove unnecessary static qualifier in crypto_dh.cc
 Author: Darshan Sen 
 Date: Sun Mar 27 18:06:08 2022 +0530
 1 file changed, 3 insertions(+), 3 deletions(-)
[master 68d23c2f72] src,crypto: handle empty maybe correctly in crypto_dh.cc
 Author: Darshan Sen 
 Date: Sun Mar 27 18:20:20 2022 +0530
 1 file changed, 4 insertions(+), 3 deletions(-)
   ✔  Patches applied
There are 3 commits in the PR. Attempting autorebase.
Rebasing (2/6)

Executing: git node land --amend --yes
⚠ Found Refs: #39941, skipping..
--------------------------------- New Message ----------------------------------
src,crypto: remove uses of AllocatedBuffer from crypto_dh.cc

Refs: #39941
Signed-off-by: Darshan Sen raisinten@gmail.com

PR-URL: #42492
Reviewed-By: Tobias Nießen tniessen@tnie.de
Reviewed-By: Anna Henningsen anna@addaleax.net

[detached HEAD 2b2431f042] src,crypto: remove uses of AllocatedBuffer from crypto_dh.cc
Author: Darshan Sen raisinten@gmail.com
Date: Sun Mar 27 18:02:52 2022 +0530
1 file changed, 52 insertions(+), 24 deletions(-)
Rebasing (3/6)
Rebasing (4/6)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
src: remove unnecessary static qualifier in crypto_dh.cc

ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen raisinten@gmail.com

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen tniessen@tnie.de
Reviewed-By: Anna Henningsen anna@addaleax.net

[detached HEAD 18b44ac73a] src: remove unnecessary static qualifier in crypto_dh.cc
Author: Darshan Sen raisinten@gmail.com
Date: Sun Mar 27 18:06:08 2022 +0530
1 file changed, 3 insertions(+), 3 deletions(-)
Rebasing (5/6)
Rebasing (6/6)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
src,crypto: handle empty maybe correctly in crypto_dh.cc

Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen raisinten@gmail.com

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen tniessen@tnie.de
Reviewed-By: Anna Henningsen anna@addaleax.net

[detached HEAD 4d0259a00f] src,crypto: handle empty maybe correctly in crypto_dh.cc
Author: Darshan Sen raisinten@gmail.com
Date: Sun Mar 27 18:20:20 2022 +0530
1 file changed, 4 insertions(+), 3 deletions(-)

Successfully rebased and updated refs/heads/master.

ℹ Use --fixupAll option, squash the PR manually or land the PR from the command line.

https://github.com/nodejs/node/actions/runs/2066012062

@RaisinTen RaisinTen added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Mar 31, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 31, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 85f679f...f2a22ef

nodejs-github-bot pushed a commit that referenced this pull request Mar 31, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
nodejs-github-bot pushed a commit that referenced this pull request Mar 31, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
nodejs-github-bot pushed a commit that referenced this pull request Mar 31, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@RaisinTen RaisinTen deleted the src,crypto/refactor-crypto_dh branch March 31, 2022 04:30
juanarbol pushed a commit to juanarbol/node that referenced this pull request Apr 5, 2022
Refs: nodejs#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit to juanarbol/node that referenced this pull request Apr 5, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#42492
Refs: nodejs#39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit to juanarbol/node that referenced this pull request Apr 5, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#42492
Refs: nodejs#39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@juanarbol juanarbol mentioned this pull request Apr 5, 2022
@juanarbol juanarbol mentioned this pull request Apr 5, 2022
juanarbol pushed a commit that referenced this pull request Apr 6, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request Apr 6, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request Apr 6, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
Refs: nodejs#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#42492
Refs: nodejs#39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#42492
Refs: nodejs#39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request May 31, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request May 31, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
juanarbol pushed a commit that referenced this pull request May 31, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 31, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 31, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 31, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Refs: nodejs/node#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42492
Refs: nodejs/node#39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42492
Refs: nodejs/node#39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
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. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants