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

Correctly set the alert type depending of the CertificateException #4885

Closed
wants to merge 1 commit into from

Conversation

normanmaurer
Copy link
Member

Motivation:

Depending on the actual CertificateException we should set the correct alert type so it will be sent back to the remote peer and so make it easier for them to fix it.

Modification:

Correctly set the alert and not always just use a general alert.

Result:

It's easier for the remote peer to fix the problems.

@normanmaurer normanmaurer added this to the 4.0.35.Final milestone Feb 19, 2016
@normanmaurer normanmaurer self-assigned this Feb 19, 2016
@normanmaurer
Copy link
Member Author

@nmittler @Scottmitch PTAL... This depends on netty/netty-tcnative#117

@nmittler
Copy link
Member

LGTM

1 similar comment
@trustin
Copy link
Member

trustin commented Feb 19, 2016

LGTM

@Scottmitch
Copy link
Member

+1 for OpenSslCertificateException. lgtm after check style fixed.

@normanmaurer normanmaurer force-pushed the ssl_cert_error branch 2 times, most recently from 7948f35 to 175ac2b Compare February 19, 2016 05:15
Motivation:

Depending on the actual CertificateException we should set the correct alert type so it will be sent back to the remote peer and so make it easier for them to fix it.

Modification:

Correctly set the alert and not always just use a general alert.

Result:

It's easier for the remote peer to fix the problems.
@normanmaurer
Copy link
Member Author

Cherry-picked into 4.0 (b91c9be) and 4.1 (6e9d2bf)

@normanmaurer normanmaurer deleted the ssl_cert_error branch February 19, 2016 16:29
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.

4 participants