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

crypto: simplify control flow in HKDF #44272

Conversation

tniessen
Copy link
Member

Unify the implementation and perform the same OpenSSL calls regardless of whether the key and/or salt are empty. This simplifies the code and improves coverage.

Refs: #44201

Unify the implementation and perform the same OpenSSL calls regardless
of whether the key and/or salt are empty. This simplifies the code and
improves coverage.

Refs: nodejs#44201
@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 Aug 18, 2022
@tniessen tniessen requested a review from panva Aug 18, 2022
@nodejs-github-bot
Copy link
Contributor

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Aug 18, 2022
@panva panva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 18, 2022
panva
panva approved these changes Aug 18, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 18, 2022
@nodejs-github-bot
Copy link
Contributor

@tniessen
Copy link
Member Author

cc @nodejs/cpp-reviewers

@RaisinTen RaisinTen added backport-blocked-v16.x PRs that should land on the v16.x-staging branch but are blocked by another PR's pending backport. dont-land-on-v16.x PRs that should not land on the v16.x-staging branch and should not be released in v16.x. and removed backport-blocked-v16.x PRs that should land on the v16.x-staging branch but are blocked by another PR's pending backport. labels Aug 23, 2022
@RaisinTen
Copy link
Member

Applied https://github.com/nodejs/node/labels/dont-land-on-v16.x because this uses std::string_view which is a C++17 feature.

@tniessen tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 25, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 25, 2022
@nodejs-github-bot nodejs-github-bot merged commit 4d49d65 into nodejs:main Aug 25, 2022
63 of 64 checks passed
@nodejs-github-bot
Copy link
Contributor

Landed in 4d49d65

sidwebworks pushed a commit to sidwebworks/node that referenced this pull request Aug 26, 2022
Unify the implementation and perform the same OpenSSL calls regardless
of whether the key and/or salt are empty. This simplifies the code and
improves coverage.

Refs: nodejs#44201
PR-URL: nodejs#44272
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Sep 5, 2022
Unify the implementation and perform the same OpenSSL calls regardless
of whether the key and/or salt are empty. This simplifies the code and
improves coverage.

Refs: #44201
PR-URL: #44272
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Fyko pushed a commit to Fyko/node that referenced this pull request Sep 15, 2022
Unify the implementation and perform the same OpenSSL calls regardless
of whether the key and/or salt are empty. This simplifies the code and
improves coverage.

Refs: nodejs#44201
PR-URL: nodejs#44272
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
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++. crypto Issues and PRs related to the crypto subsystem. dont-land-on-v16.x PRs that should not land on the v16.x-staging branch and should not be released in v16.x. 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