Correct error handling for SSL_ERROR_SSL #42
Merged
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.
Description
This PR fixes two things in the Linux (openssl) implementation:
ERR_clear_error
prior to functions that may requireSSL_get_error
to be calledSSL_ERROR_SSL
case.Motivation and Context
This resolves an intermittent failure exposed by a test added to Kitura-net (Kitura/Kitura-net#219), where the test can cause an SSL protocol error (
SSL_ERROR_SSL
).When this occurs, an error is placed onto that thread's error queue, however the
throwLastError
function did not callERR_get_error
in this case, instead reporting "Could not determine error reason." and leaving the error on the queue.The next SSL call with a non-zero return on that thread would then trigger a call to
SSL_get_error
, which would incorrectly pick up the error on the queue, incorrectly reporting anotherSSL_ERROR_SSL
.Part of the contract of
SSL_get_error
is that the error queue must have been cleared before a call toSSL_read
,SSL_accept
, etc. is made. To ensure we obey this contract, I have addedERR_clear_error
before each such call.How Has This Been Tested?
I have run the Kitura-net tests and they pass.
Since we do not have a test that deliberately or reliably causes an SSL protocol error, to test the output of the error reporting, I modified a test locally to remove the SSL delegate from the client-side socket and then send some erroneous data. This results in the following output:
Checklist: