Skip to content

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!

@Scottmitch Scottmitch deleted the ssl_handler_beginhandshake_once branch January 29, 2016 06:55
@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.

5 participants