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

Use ERR_get_error to get last SSL error in openssl.d #796

Merged
merged 1 commit into from
Sep 27, 2014

Conversation

cifvts
Copy link
Contributor

@cifvts cifvts commented Aug 29, 2014

Getting last error should return an error code which is easier to understand
when translated into a error string

@s-ludwig
Copy link
Member

I think we may need two versions of enforceSSL and carefully check the documentation of each function for which of the two error codes is actually set. Alternatively, just querying both and displaying only the one(s) that indicate an error would be an option.

@cifvts
Copy link
Contributor Author

cifvts commented Aug 30, 2014

I tried to understand the difference and to me it looks that SSL_get_error() return code is not meant to work with ERR_error_string().

@cifvts
Copy link
Contributor Author

cifvts commented Sep 24, 2014

I dug more into this:

  • SSL_get_error() returns a generic SSL Error but the error stays in the error queue so it may affects later calls;
  • SSL_get_error() returns the error of the SSL object which is generic error;
  • ERR_get_error() returns the correct error code for ERR_error_string() and removes that error from the error queue.

So the right approach IMHO would be to check if an error exists using SSL_get_error() and then get the error (or the errors) using ERR_get_error().

@cifvts
Copy link
Contributor Author

cifvts commented Sep 26, 2014

Trying to refactor the patch I saw there is no need to check both function, we already know an error exists and we only need to give the user a proper error string so he can understand why SSL failed. Using ERR_get_error() / ERR_error_string() we do the job.

Getting last error should return an error code which  is easier to understand
when tranlated into a error string
@s-ludwig
Copy link
Member

Sounds reasonable, thanks for looking up the details. I'll add an additional loop that collects and logs and additional errors in the queue and only throws the last one, just to be sure.

s-ludwig added a commit that referenced this pull request Sep 27, 2014
Use ERR_get_error to get last SSL error in openssl.d
@s-ludwig s-ludwig merged commit ce6eb1f into vibe-d:master Sep 27, 2014
@cifvts cifvts deleted the ssl_better_error branch September 27, 2014 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants