From 750d23583c187b4e8dafdfea3f4fbeaf1d9be7cd Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Thu, 1 Jul 2021 17:40:52 +0530 Subject: [PATCH] Add ALPN Buffering to support HTTP/2 Prior Knowledge (#11407) Motivation: Currently, Netty cannot handle HTTP/2 Preface messages if the client used the Prior knowledge technique. In Prior knowledge, the client sends an HTTP/2 preface message immediately after finishing TLS Handshake. But in Netty, when TLS Handshake is finished, ALPNHandler is triggered to configure the pipeline. And between these 2 operations, if an HTTP/2 preface message arrives, it gets dropped. Modification: Buffer messages until we are done with the ALPN handling. Result: Fixes #11403. Co-authored-by: Norman Maurer --- ...ApplicationProtocolNegotiationHandler.java | 37 ++++++++++++++ ...icationProtocolNegotiationHandlerTest.java | 49 +++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/handler/src/main/java/io/netty/handler/ssl/ApplicationProtocolNegotiationHandler.java b/handler/src/main/java/io/netty/handler/ssl/ApplicationProtocolNegotiationHandler.java index 34399cc69b..f46c85da9f 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ApplicationProtocolNegotiationHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/ApplicationProtocolNegotiationHandler.java @@ -21,7 +21,9 @@ import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.ChannelInitializer; import io.netty.channel.ChannelPipeline; import io.netty.handler.codec.DecoderException; +import io.netty.util.ReferenceCountUtil; import io.netty.util.internal.ObjectUtil; +import io.netty.util.internal.RecyclableArrayList; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -68,6 +70,8 @@ public abstract class ApplicationProtocolNegotiationHandler extends ChannelInbou InternalLoggerFactory.getInstance(ApplicationProtocolNegotiationHandler.class); private final String fallbackProtocol; + private final RecyclableArrayList bufferedMessages = RecyclableArrayList.newInstance(); + private ChannelHandlerContext ctx; /** * Creates a new instance with the specified fallback protocol name. @@ -79,6 +83,38 @@ public abstract class ApplicationProtocolNegotiationHandler extends ChannelInbou this.fallbackProtocol = ObjectUtil.checkNotNull(fallbackProtocol, "fallbackProtocol"); } + @Override + public void handlerAdded(ChannelHandlerContext ctx) throws Exception { + this.ctx = ctx; + super.handlerAdded(ctx); + } + + @Override + public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { + fireBufferedMessages(); + bufferedMessages.recycle(); + super.handlerRemoved(ctx); + } + + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { + // Let's buffer all data until this handler will be removed from the pipeline. + bufferedMessages.add(msg); + } + + /** + * Process all backlog into pipeline from List. + */ + private void fireBufferedMessages() { + if (!bufferedMessages.isEmpty()) { + for (int i = 0; i < bufferedMessages.size(); i++) { + ctx.fireChannelRead(bufferedMessages.get(i)); + } + ctx.fireChannelReadComplete(); + bufferedMessages.clear(); + } + } + @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { if (evt instanceof SslHandshakeCompletionEvent) { @@ -118,6 +154,7 @@ public abstract class ApplicationProtocolNegotiationHandler extends ChannelInbou pipeline.remove(this); } } + /** * Invoked on successful initial SSL/TLS handshake. Implement this method to configure your pipeline * for the negotiated application-level protocol. diff --git a/handler/src/test/java/io/netty/handler/ssl/ApplicationProtocolNegotiationHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/ApplicationProtocolNegotiationHandlerTest.java index 678d0d22db..f80f56cbd9 100644 --- a/handler/src/test/java/io/netty/handler/ssl/ApplicationProtocolNegotiationHandlerTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/ApplicationProtocolNegotiationHandlerTest.java @@ -18,17 +18,20 @@ package io.netty.handler.ssl; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.embedded.EmbeddedChannel; import io.netty.handler.codec.DecoderException; import org.junit.Test; import java.security.NoSuchAlgorithmException; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLHandshakeException; import static io.netty.handler.ssl.CloseNotifyTest.assertCloseNotify; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -99,4 +102,50 @@ public class ApplicationProtocolNegotiationHandlerTest { assertFalse(channel.finishAndReleaseAll()); } } + + @Test + public void testBufferMessagesUntilHandshakeComplete() throws Exception { + final AtomicReference channelReadData = new AtomicReference(); + final AtomicBoolean channelReadCompleteCalled = new AtomicBoolean(false); + ChannelHandler alpnHandler = new ApplicationProtocolNegotiationHandler(ApplicationProtocolNames.HTTP_1_1) { + @Override + protected void configurePipeline(ChannelHandlerContext ctx, String protocol) { + assertEquals(ApplicationProtocolNames.HTTP_1_1, protocol); + ctx.pipeline().addLast(new ChannelInboundHandlerAdapter() { + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { + channelReadData.set((byte[]) msg); + } + + @Override + public void channelReadComplete(ChannelHandlerContext ctx) throws Exception { + channelReadCompleteCalled.set(true); + } + }); + } + }; + + SSLEngine engine = SSLContext.getDefault().createSSLEngine(); + // This test is mocked/simulated and doesn't go through full TLS handshake. Currently only JDK SSLEngineImpl + // client mode will generate a close_notify. + engine.setUseClientMode(true); + + final byte[] someBytes = new byte[1024]; + + EmbeddedChannel channel = new EmbeddedChannel(new SslHandler(engine), new ChannelInboundHandlerAdapter() { + @Override + public void userEventTriggered(ChannelHandlerContext ctx, Object evt) { + if (evt == SslHandshakeCompletionEvent.SUCCESS) { + ctx.fireChannelRead(someBytes); + } + ctx.fireUserEventTriggered(evt); + } + }, alpnHandler); + channel.pipeline().fireUserEventTriggered(SslHandshakeCompletionEvent.SUCCESS); + assertNull(channel.pipeline().context(alpnHandler)); + assertArrayEquals(someBytes, channelReadData.get()); + assertTrue(channelReadCompleteCalled.get()); + assertNull(channel.readInbound()); + channel.finishAndReleaseAll(); + } }