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: clear error on return in for TLS methods #719

Closed
wants to merge 1 commit into from
Closed

crypto: clear error on return in for TLS methods #719

wants to merge 1 commit into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Feb 4, 2015

Fix #712

@indutny indutny mentioned this pull request Feb 4, 2015
Closed
@indutny
Copy link
Member Author

indutny commented Feb 4, 2015

cc @iojs/crypto confirmed to fix

@indutny
Copy link
Member Author

indutny commented Feb 4, 2015

NOTE: I'm afraid there is not much point in writing test for it. This error may occur only when we are reading ERR_get_error() output (like in SSL_get_error()) and it is really hard to reproduce without relying on the way things work atm.

@bnoordhuis
Copy link
Member

There is a typo in the commit log ('in for' -> 'in') and the commit log as a whole could go into more detail. Apart from that, LGTM.

indutny added a commit that referenced this pull request Feb 5, 2015
@indutny
crypto: clear error on return in TLS methods
6f7a978 
Methods like `X509_STORE_add_cert` may push errors onto OpenSSL's error
stack. Ensure that they won't pop up in a different places like
`tls_wrap.cc`.

Fix: #712
PR-URL: #719
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@indutny
Copy link
Member Author

indutny commented Feb 5, 2015

Landed in 6f7a978, thank you!

@indutny indutny closed this Feb 5, 2015
@indutny indutny deleted the fix/gh-712 branch February 5, 2015 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@indutny @bnoordhuis