Skip to content

Commit

Permalink
netty: TCP close during TLS handshake should be UNAVAILABLE
Browse files Browse the repository at this point in the history
Normally the first exception/event experienced is the cause and is
followed by a stampede of ClosedChannelExceptions. In this case,
SslHandler is manufacturing a ClosedChannelException of its own and
propagating it _before_ the trigger event. This might be considered a
bug, but changing SslHandler's behavior would be very risky and almost
certainly break someone's code.

Fixes #7376
  • Loading branch information
ejona86 committed Oct 1, 2020
1 parent 0cd56c2 commit ec0d01d
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
14 changes: 13 additions & 1 deletion netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java
Expand Up @@ -58,6 +58,7 @@
import io.netty.util.AttributeMap;
import java.net.SocketAddress;
import java.net.URI;
import java.nio.channels.ClosedChannelException;
import java.util.Arrays;
import java.util.concurrent.Executor;
import java.util.logging.Level;
Expand Down Expand Up @@ -372,7 +373,18 @@ protected void userEventTriggered0(ChannelHandlerContext ctx, Object evt) throws
ctx.fireExceptionCaught(ex);
}
} else {
ctx.fireExceptionCaught(handshakeEvent.cause());
Throwable t = handshakeEvent.cause();
if (t instanceof ClosedChannelException) {
// On channelInactive(), SslHandler creates its own ClosedChannelException and
// propagates it before the actual channelInactive(). So we assume here that any
// such exception is from channelInactive() and emulate the normal behavior of
// WriteBufferingAndExceptionHandler
t = Status.UNAVAILABLE
.withDescription("Connection closed while performing TLS negotiation")
.withCause(t)
.asRuntimeException();
}
ctx.fireExceptionCaught(t);
}
} else {
super.userEventTriggered0(ctx, evt);
Expand Down
16 changes: 16 additions & 0 deletions netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java
Expand Up @@ -33,6 +33,8 @@
import io.grpc.Grpc;
import io.grpc.InternalChannelz.Security;
import io.grpc.SecurityLevel;
import io.grpc.Status;
import io.grpc.StatusRuntimeException;
import io.grpc.internal.GrpcAttributes;
import io.grpc.internal.testing.TestUtils;
import io.grpc.netty.ProtocolNegotiators.ClientTlsHandler;
Expand Down Expand Up @@ -534,6 +536,20 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
assertNull(grpcHandlerCtx);
}

@Test
public void clientTlsHandler_closeDuringNegotiation() throws Exception {
ClientTlsHandler handler = new ClientTlsHandler(grpcHandler, sslContext, "authority", null);
pipeline.addLast(new WriteBufferingAndExceptionHandler(handler));
ChannelFuture pendingWrite = channel.writeAndFlush(NettyClientHandler.NOOP_MESSAGE);

// SslHandler fires userEventTriggered() before channelInactive()
pipeline.fireChannelInactive();

assertThat(pendingWrite.cause()).isInstanceOf(StatusRuntimeException.class);
assertThat(Status.fromThrowable(pendingWrite.cause()).getCode())
.isEqualTo(Status.Code.UNAVAILABLE);
}

@Test
public void engineLog() {
ChannelHandler handler = new ServerTlsHandler(grpcHandler, sslContext, null);
Expand Down

0 comments on commit ec0d01d

Please sign in to comment.