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

SslHandler should call beginHanshake once for the initial handshake #4764

Closed

Conversation

Scottmitch
Copy link
Member

Motivation:
Not all SSLEngine implementations permit beginHandshake being called while a handshake is in progress during the initial handshake. We should ensure we only go through the initial handshake code once to prevent unexpected exceptions from being thrown.

Modifications:

  • Only call beginHandshake if there is not currently a handshake in progress

Result:
SslHandler's handshake method is compatible with OpenSSLEngineImpl in Android 5.0+ and 6.0+.
Fixes #4718

Motivation:
Not all SSLEngine implementations permit beginHandshake being called while a handshake is in progress during the initial handshake. We should ensure we only go through the initial handshake code once to prevent unexpected exceptions from being thrown.

Modifications:
- Only call beginHandshake if there is not currently a handshake in progress

Result:
SslHandler's handshake method is compatible with OpenSSLEngineImpl in Android 5.0+ and 6.0+.
Fixes netty#4718
@Scottmitch Scottmitch self-assigned this Jan 27, 2016
@Scottmitch Scottmitch added this to the 4.0.34.Final milestone Jan 27, 2016
@Scottmitch
Copy link
Member Author

@normanmaurer - PTAL

@ganskef - Please verify.

@normanmaurer
Copy link
Member

@Scottmitch LGTM

@johnou
Copy link
Contributor

johnou commented Jan 28, 2016

@ganskef any luck testing if the issue is solved with this fix?

@normanmaurer
Copy link
Member

Cherry-picked into 4.1 (e1d34ef) and 4.0 (cf2e829).

@Scottmitch thanks!

@ganskef
Copy link

ganskef commented Feb 4, 2016

@Scottmitch thank you very much! I had a commented it at #4718. It works well now with Android 5.0, 5.1, and 6.0 on emulator and on a Nexus 5 with the latest Android from Google. I use it on Linux desktop with OpenJDK 7 for a week without notable issues.
@normanmaurer it's well done to close this PR. I'm happy to await a release with it.

@Scottmitch
Copy link
Member Author

@ganskef - No problem .. thanks for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants