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: use BoringSSL-compatible flag getter #29604

Closed

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Sep 18, 2019

This PR changes HashInit to return the md flags via EVP_MD_flags() instead of EVP_MD_meth_get_flags().

In OpenSSL 1.1.1c, which Node.js bundles at present, these functions are implemented in precisely the same way (see linked implementations above), with the primary difference being that EVP_MD_flags() is exposed by BoringSSL whereas EVP_MD_meth_get_flags() is not.

This allows Electron to reduce its patch surface while changing no behavior in Node.js.

cc @tniessen

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@codebytere codebytere requested a review from tniessen September 18, 2019 19:44
@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. labels Sep 18, 2019
@nodejs-github-bot
Copy link
Collaborator

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 19, 2019
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

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

Thank you, Shelley!

@Trott
Copy link
Member

Trott commented Sep 22, 2019

Landed in ef2e9e3

@Trott Trott closed this Sep 22, 2019
Trott pushed a commit that referenced this pull request Sep 22, 2019
PR-URL: #29604
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@codebytere codebytere deleted the use-boringssl-compat-flag-getter branch September 23, 2019 01:18
targos pushed a commit that referenced this pull request Sep 23, 2019
PR-URL: #29604
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@BridgeAR BridgeAR mentioned this pull request Sep 24, 2019
BridgeAR pushed a commit that referenced this pull request Sep 25, 2019
PR-URL: #29604
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants