Ensure we not try to call select
when the AbstractSniHandler
was already removed from the pipeline.
Motivation: We tried to call `select` after we closed the channel (and so removed all the handlers from the pipeline) when we detected a non SSL record. This would cause an exception like this: ``` Caused by: java.util.NoSuchElementException: io.netty.handler.ssl.SniHandler at io.netty.channel.DefaultChannelPipeline.getContextOrDie(DefaultChannelPipeline.java:1098) at io.netty.channel.DefaultChannelPipeline.replace(DefaultChannelPipeline.java:506) at io.netty.handler.ssl.SniHandler.replaceHandler(SniHandler.java:133) at io.netty.handler.ssl.SniHandler.onLookupComplete(SniHandler.java:113) at io.netty.handler.ssl.AbstractSniHandler.select(AbstractSniHandler.java:225) at io.netty.handler.ssl.AbstractSniHandler.decode(AbstractSniHandler.java:218) at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:489) at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:428) ... 40 more ``` Modifications: - Ensure we rethrow the NotSslRecordException when detecting it (and closing the channel). This will also ensure we not call `select(...)` - Not catch `Throwable` but only `Exception` - Add test case. Result: Correctly handle the case of an non SSL record.
This commit is contained in:
parent
27a4e4a3c1
commit
d729ae716d
@ -239,6 +239,9 @@ public class SniHandler extends ByteToMessageDecoder {
|
||||
break loop;
|
||||
}
|
||||
}
|
||||
} catch (NotSslRecordException e) {
|
||||
// Just rethrow as in this case we also closed the channel and this is consistent with SslHandler.
|
||||
throw e;
|
||||
} catch (Exception e) {
|
||||
// unexpected encoding, ignore sni and use default
|
||||
if (logger.isDebugEnabled()) {
|
||||
|
@ -23,6 +23,7 @@ import io.netty.buffer.Unpooled;
|
||||
import io.netty.channel.Channel;
|
||||
import io.netty.channel.ChannelFuture;
|
||||
import io.netty.channel.ChannelHandlerContext;
|
||||
import io.netty.channel.ChannelInboundHandlerAdapter;
|
||||
import io.netty.channel.ChannelInitializer;
|
||||
import io.netty.channel.ChannelPipeline;
|
||||
import io.netty.channel.EventLoopGroup;
|
||||
@ -30,6 +31,7 @@ import io.netty.channel.embedded.EmbeddedChannel;
|
||||
import io.netty.channel.nio.NioEventLoopGroup;
|
||||
import io.netty.channel.socket.nio.NioServerSocketChannel;
|
||||
import io.netty.channel.socket.nio.NioSocketChannel;
|
||||
import io.netty.handler.codec.DecoderException;
|
||||
import io.netty.util.DomainNameMapping;
|
||||
import io.netty.util.DomainNameMappingBuilder;
|
||||
import io.netty.util.ReferenceCountUtil;
|
||||
@ -44,12 +46,11 @@ import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.is;
|
||||
import static org.hamcrest.CoreMatchers.nullValue;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.*;
|
||||
import static org.junit.Assume.assumeTrue;
|
||||
|
||||
@RunWith(Parameterized.class)
|
||||
@ -126,6 +127,42 @@ public class SniHandlerTest {
|
||||
this.provider = provider;
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNonSslRecord() throws Exception {
|
||||
SslContext nettyContext = makeSslContext(provider, false);
|
||||
try {
|
||||
final AtomicReference<SslHandshakeCompletionEvent> evtRef =
|
||||
new AtomicReference<SslHandshakeCompletionEvent>();
|
||||
SniHandler handler = new SniHandler(new DomainNameMappingBuilder<SslContext>(nettyContext).build());
|
||||
EmbeddedChannel ch = new EmbeddedChannel(handler, new ChannelInboundHandlerAdapter() {
|
||||
@Override
|
||||
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
|
||||
if (evt instanceof SslHandshakeCompletionEvent) {
|
||||
assertTrue(evtRef.compareAndSet(null, (SslHandshakeCompletionEvent) evt));
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
try {
|
||||
byte[] bytes = new byte[1024];
|
||||
bytes[0] = SslUtils.SSL_CONTENT_TYPE_ALERT;
|
||||
|
||||
try {
|
||||
ch.writeInbound(Unpooled.wrappedBuffer(bytes));
|
||||
fail();
|
||||
} catch (DecoderException e) {
|
||||
assertTrue(e.getCause() instanceof NotSslRecordException);
|
||||
}
|
||||
assertFalse(ch.finish());
|
||||
} finally {
|
||||
ch.finishAndReleaseAll();
|
||||
}
|
||||
assertTrue(evtRef.get().cause() instanceof NotSslRecordException);
|
||||
} finally {
|
||||
releaseAll(nettyContext);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testServerNameParsing() throws Exception {
|
||||
SslContext nettyContext = makeSslContext(provider, false);
|
||||
|
Loading…
Reference in New Issue
Block a user