andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Correctly resume wrap / unwrap when SslTask execution completes (#8899)

Browse Source 
Motivation:

fa6a8cb09c introduced correct dispatching of delegated tasks for SSLEngine but did not correctly handle some cases for resuming wrap / unwrap after the task was executed. This could lead to stales, which showed up during tests when running with Java11 and BoringSSL.

Modifications:

- Correctly resume wrap / unwrap in all cases.
- Fix timeout value which was changed in previous commit by mistake.

Result:

No more stales after task execution.
This commit is contained in:
Norman Maurer 2019-02-28 20:29:40 +01:00 committed by GitHub
parent d3d0b6478b
commit c6d3792df0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 22 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
handler/src/main/java/io/netty/handler/ssl/SslHandler.java
View File

@ -953,7 +953,8 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
out = null;
}
switch (result.getHandshakeStatus()) {
HandshakeStatus status = result.getHandshakeStatus();
switch (status) {
case FINISHED:
setHandshakeSuccess();
return false;
@ -988,7 +989,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
throw new IllegalStateException("Unknown handshake status: " + result.getHandshakeStatus());
}
if (result.bytesProduced() == 0) {
if (result.bytesProduced() == 0 && status != HandshakeStatus.NEED_TASK) {
break;
}
@ -1438,7 +1439,9 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
throw new IllegalStateException("unknown handshake status: " + handshakeStatus);
}
if (status == Status.BUFFER_UNDERFLOW || consumed == 0 && produced == 0) {
if (status == Status.BUFFER_UNDERFLOW ||
// If we processed NEED_TASK we should try again even we did not consume or produce anything.
handshakeStatus != HandshakeStatus.NEED_TASK && consumed == 0 && produced == 0) {
if (handshakeStatus == HandshakeStatus.NEED_UNWRAP) {
// The underlying engine is starving so we need to feed it with more data.
// See https://github.com/netty/netty/pull/5039
@ -1624,6 +1627,11 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
taskError(e);
return;
}
if (inUnwrap) {
// If we were in the unwrap call when the task was processed we should also try to unwrap
// non app data first as there may not anything left in the inbound buffer to process.
unwrapNonAppData(ctx);
}
// Flush now as we may have written some data as part of the wrap call.
forceFlush(ctx);
@ -1642,13 +1650,20 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
// that will be written to the Channel.
case NEED_WRAP:
try {
wrapNonAppData(ctx, inUnwrap);
if (!wrapNonAppData(ctx, false) && inUnwrap) {
// The handshake finished in wrapNonAppData(...), we need to try call
// unwrapNonAppData(...) as we may have some alert that we should read.
//
// This mimics what we would do when we are calling this method while in unwrap(...).
unwrapNonAppData(ctx);
}
// Flush now as we may have written some data as part of the wrap call.
forceFlush(ctx);
} catch (Throwable e) {
taskError(e);
return;
}
// Flush now as we may have written some data as part of the wrap call.
forceFlush(ctx);
// Now try to feed in more data that we have buffered.
tryDecodeAgain();

2
handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java
View File

@ -1151,7 +1151,7 @@ public abstract class SSLEngineTest {
MessageReceiver receiver) throws Exception {
List<ByteBuf> dataCapture = null;
try {
assertTrue(sendChannel.writeAndFlush(message).await(50, TimeUnit.SECONDS));
assertTrue(sendChannel.writeAndFlush(message).await(5, TimeUnit.SECONDS));
receiverLatch.await(5, TimeUnit.SECONDS);
message.resetReaderIndex();
ArgumentCaptor<ByteBuf> captor = ArgumentCaptor.forClass(ByteBuf.class);