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.

Sign up for GitHub

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

Jump to bottom

crypto: check return code from EVP_DigestUpdate #41800

Closed
wants to merge 2 commits into from
Closed

crypto: check return code from EVP_DigestUpdate #41800

wants to merge 2 commits into from

Conversation

mhdawson
Copy link
Member

Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson [email protected]

@mhdawson
crypto: check return code from EVP_DigestUpdate
b5239bb 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto

@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. needs-ci PRs that need a full CI run. labels Jan 31, 2022
@mhdawson
Copy link
Member Author

In case anybody is interested this is the other place we call EVP_DigestUpdate -

node/src/crypto/crypto_sig.cc

Line 316 in 0ff62b4

if (!EVP_DigestUpdate(mdctx_.get(), data, len))

tniessen
tniessen approved these changes Jan 31, 2022
View reviewed changes
src/crypto/crypto_hash.cc Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Feb 3, 2022
Open
41 tasks
@mhdawson
Copy link
Member Author

mhdawson commented Feb 3, 2022

Landed in 674a955

@mhdawson mhdawson closed this Feb 3, 2022
mhdawson added a commit that referenced this pull request Feb 3, 2022
@mhdawson
crypto: check return code from EVP_DigestUpdate
674a955 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #41800
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
VoltrexKeyva pushed a commit to VoltrexKeyva/node that referenced this pull request Feb 3, 2022
@mhdawson @VoltrexKeyva
crypto: check return code from EVP_DigestUpdate
b381ea4 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#41800
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
This was referenced Feb 4, 2022
Open
Open
Open
Open
ruyadorno pushed a commit that referenced this pull request Feb 8, 2022
@mhdawson @ruyadorno
crypto: check return code from EVP_DigestUpdate
e117c0f 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #41800
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Feb 8, 2022
Merged
danielleadams pushed a commit that referenced this pull request Mar 2, 2022
@mhdawson @danielleadams
crypto: check return code from EVP_DigestUpdate
9a60cd8 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #41800
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
@danielleadams danielleadams mentioned this pull request Mar 3, 2022
Merged
danielleadams pushed a commit that referenced this pull request Mar 3, 2022
@mhdawson @danielleadams
crypto: check return code from EVP_DigestUpdate
5214038 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #41800
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
danielleadams pushed a commit to danielleadams/node that referenced this pull request Mar 4, 2022
@mhdawson @danielleadams
crypto: check return code from EVP_DigestUpdate
8cd534d 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#41800
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
danielleadams pushed a commit to danielleadams/node that referenced this pull request Mar 4, 2022
@mhdawson @danielleadams
crypto: check return code from EVP_DigestUpdate
5497454 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#41800
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
danielleadams pushed a commit that referenced this pull request Mar 8, 2022
@mhdawson @danielleadams
crypto: check return code from EVP_DigestUpdate
418b1cd 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #41800
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
danielleadams pushed a commit that referenced this pull request Mar 14, 2022
@mhdawson @danielleadams
crypto: check return code from EVP_DigestUpdate
46f714f 
Coverity was complaining that we did not check the
return code. We seem to check in the other place it
is called and the method already handles returning
a result.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #41800
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tniessen tniessen tniessen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@panva panva panva approved these changes

Assignees
No one assigned
Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mhdawson @nodejs-github-bot @panva @lpinca @cjihrig @tniessen