Skip to content

Workaround SSLEngine.unwrap(...) bug in Android 5.0#7761

Merged
normanmaurer merged 1 commit into
4.1from
android_unwrap_workaround
Mar 3, 2018
Merged

Workaround SSLEngine.unwrap(...) bug in Android 5.0#7761
normanmaurer merged 1 commit into
4.1from
android_unwrap_workaround

Conversation

@normanmaurer
Copy link
Copy Markdown
Member

Motivation:

Android 5.0 sometimes not correctly update the bytesConsumed of the SSLEngineResult when consuming data from the input ByteBuffer. This will lead to handshake failures.

Modifications:

Add a workaround for Android 5.0

Result:

Be able to use netty on Android 5.0 by fixing #7758 .

Motivation:

Android 5.0 sometimes not correctly update the bytesConsumed of the SSLEngineResult when consuming data from the input ByteBuffer. This will lead to handshake failures.

Modifications:

Add a workaround for Android 5.0

Result:

Be able to use netty on Android 5.0 by fixing #7758 .
@normanmaurer normanmaurer force-pushed the android_unwrap_workaround branch from cbc4dcf to d0b43d1 Compare March 2, 2018 06:37
@normanmaurer normanmaurer self-assigned this Mar 2, 2018
@normanmaurer
Copy link
Copy Markdown
Member Author

@MarkVilkel reported it fixes his issues 👍

Copy link
Copy Markdown
Member

@carl-mastrangelo carl-mastrangelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

// - https://android-review.googlesource.com/c/platform/external/conscrypt/+/122080
// - https://github.com/netty/netty/issues/7758
if (result.bytesConsumed() == 0) {
int consumed = inNioBuffer.position() - position;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert PlatformDependent.isAndroid();

?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do this ... that said I am not sure its necessary. @carl-mastrangelo if you are strong on it I will add it.... WDYT ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not keen on it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok let me merge it then

@normanmaurer normanmaurer merged commit bf8cac4 into 4.1 Mar 3, 2018
@normanmaurer normanmaurer deleted the android_unwrap_workaround branch March 3, 2018 23:01
@normanmaurer normanmaurer added this to the 4.1.23.Final milestone Mar 3, 2018
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.

3 participants