src: cleanup crypto more #57323
Open
src: cleanup crypto more #57323
Conversation
This file contains 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
|
Review requested:
|
nodejs-github-bot
added
lib / src
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57323 +/- ##
==========================================
+ Coverage 90.21% 90.22% +0.01%
==========================================
Files 630 630
Lines 185304 185289 -15
Branches 36266 36265 -1
==========================================
+ Hits 167171 167184 +13
+ Misses 11084 11072 -12
+ Partials 7049 7033 -16
🚀 New features to boost your workflow:
|
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
23c0321 to
5ed620a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
5ed620a to
062c774
Compare
This comment was marked as outdated.
This comment was marked as outdated.
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
062c774 to
a8744d3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
anonrig
approved these changes
Mar 5, 2025
jasnell
added
the
author ready
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
a8744d3 to
defdb2f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
There are relevant CI failures to investigate before this can land. |
This comment was marked as outdated.
This comment was marked as outdated.
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
0f7222f to
e025d3f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
e025d3f to
2c83aa7
Compare
This comment was marked as outdated.
This comment was marked as outdated.
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
2c83aa7 to
066f084
Compare
This comment was marked as outdated.
This comment was marked as outdated.
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
066f084 to
76afdc6
Compare
* Use ncrypto APIs where appropriate * Remove obsolete no-longer used functions * Improve error handling a bit
To simplify handling of boringssl/openssl, move secure heap impl to ncrypto. Overall the reduces the complexity of the code in crypto_util by eliminating additional ifdef branches.
The ByteSource does not currently know how to free a DataPointer allocated on the secure heap, so just verify. DataPointers on the secure heap are not something that users can allocate on their own. Their use is rare. Eventually ByteSource is going to be refactored around ncrypto APIs so these additional checks should be temporary.
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
76afdc6 to
f6d919c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
jasnell
force-pushed
the
jasnell/more-crypto-cleanups
branch
from
24bbedd to
b63dd85
Compare
|
CI: https://ci.nodejs.org/job/node-test-pull-request/65625/ 💛 sorry for the extra CI runs.. the issue is happening only with openssl 1.1.1 and I have no way of testing it locally and the CI output is not enough to fully diagnose what the issues are... so... repeated CI runs are necessary. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
More cleanup in the crypto code... encapsulating stuff into ncrypto, moving ifdef branches into ncrypto, removing some obsolete stuff, simplifying and improving some error handling, etc.