FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: use automatic memory mgmt in SecretKeyGen by tniessen · Pull Request #44479 · nodejs/node · GitHub

/ node Public

src: use automatic memory mgmt in SecretKeyGen - #44479

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:src-avoid-manual-memory-management-secretkeygen
Sep 5, 2022
ConversationCommits (1)ChecksFiles changed
Merged

src: use automatic memory mgmt in SecretKeyGen#44479
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:src-avoid-manual-memory-management-secretkeygen

Conversation

tniessen commented Sep 1, 2022

Copy link
Copy Markdown
Member

Avoid manual memory management (i.e., calling MallocOpenSSL). This leaves less room for memory leaks and other bugs.

The import bit here is using ByteSource::Builder (see #43202).

src: use automatic memory mgmt in SecretKeyGen
8d19d39 
Avoid manual memory management (i.e., calling MallocOpenSSL). This
leaves less room for memory leaks and other bugs.
tniessen added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 1, 2022

nodejs-github-bot commented Sep 1, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Sep 1, 2022
tniessen added review wanted PRs that need reviews. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 4, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2022

This comment was marked as outdated.

Sign in to view

nodejs-github-bot commented Sep 4, 2022

Copy link
Copy Markdown
Collaborator

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

github-actions Bot mentioned this pull request Sep 5, 2022
Open
17 tasks

tniessen commented Sep 5, 2022

Copy link
Copy Markdown
Member Author

cc @nodejs/cpp-reviewers

tniessen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Sep 5, 2022
tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 5, 2022
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 5, 2022
nodejs-github-bot merged commit 1f54fc2 into nodejs:main Sep 5, 2022

nodejs-github-bot commented Sep 5, 2022

Copy link
Copy Markdown
Collaborator

Landed in 1f54fc2

This was referenced Sep 6, 2022
Open
Open
Open
Open
Open
Open

RafaelGSS commented Sep 26, 2022

Copy link
Copy Markdown
Member

It didn't land cleanly on v18.x. This PR conflicts with a security change in the same method (

node/src/crypto/crypto_keygen.cc

Line 83 in 0c2a572

params->out = MallocOpenSSL<char>(params->length);
).

RafaelGSS mentioned this pull request Oct 14, 2022
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bnoordhuis bnoordhuis approved these changes
addaleax addaleax approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. 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. review wanted PRs that need reviews.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL