Skip to content

Conversation

tniessen
Copy link
Member

  1. Use bool instead of int.
  2. Don't call Buffer::HasInstance. We shouldn't silently ignore non-buffers, and ByteSource::FromBuffer will CHECK that the input is a Buffer anyway.
  3. Rename key to context, because that's what it is.
  4. Rename useContext to use_context for consistency with the rest of node_crypto.cc.
  5. Use IsUndefined instead of IsNull, because the JavaScript layer will always either pass undefined or a Buffer.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@tniessen tniessen added crypto Issues and PRs related to the crypto subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Feb 23, 2020
Co-Authored-By: Anna Henningsen <[email protected]>
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Mar 1, 2020

Landed in 51cae73

Trott pushed a commit that referenced this pull request Mar 1, 2020
PR-URL: #31922
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@Trott Trott closed this Mar 1, 2020
MylesBorins pushed a commit that referenced this pull request Mar 4, 2020
PR-URL: #31922
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 4, 2020
@targos
Copy link
Member

targos commented Apr 20, 2020

depends on #31814 to land on v12.x

targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
PR-URL: nodejs#31922
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
PR-URL: #31922
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants