Skip to content

Commit

Permalink
Configures HTTP2 pipeline with more proper way
Browse files Browse the repository at this point in the history
Motivation:

When we use pipeline.replace and we still had ongoing inbound, then
there will be some problem that inbound message would go to wrong
handlers. So we add handler first, and remove self after add, so that
the next handler will be the correct one.

Modifications:

Uses remove after addAfter instead of replace.

Result:

Fixed netty#6881
  • Loading branch information
chhsiao90 authored and normanmaurer committed Aug 2, 2017
1 parent 6ab9c17 commit 8320a45
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,11 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
// following network traffic
ctx.pipeline()
.remove(httpServerCodec)
.remove(httpServerUpgradeHandler)
.replace(this, null, http2ServerHandler);
.remove(httpServerUpgradeHandler);

ctx.pipeline().addAfter(ctx.name(), null, http2ServerHandler);
ctx.pipeline().remove(this);

ctx.fireUserEventTriggered(PriorKnowledgeUpgradeEvent.INSTANCE);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.Channel;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.channel.ChannelInitializer;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.handler.codec.http.DefaultHttpHeaders;
import io.netty.handler.codec.http.HttpMethod;
Expand All @@ -35,7 +37,6 @@
import io.netty.util.CharsetUtil;
import io.netty.util.ReferenceCountUtil;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import java.util.ArrayList;
Expand All @@ -56,8 +57,7 @@ public class CleartextHttp2ServerUpgradeHandlerTest {

private List<Object> userEvents;

@Before
public void setUp() {
private void setUpServerChannel() {
frameListener = mock(Http2FrameListener.class);

http2ConnectionHandler = new Http2ConnectionHandlerBuilder().frameListener(frameListener).build();
Expand Down Expand Up @@ -91,6 +91,8 @@ public void tearDown() throws Exception {

@Test
public void priorKnowledge() throws Exception {
setUpServerChannel();

channel.writeInbound(Http2CodecUtil.connectionPrefaceBuf());

ByteBuf settingsFrame = settingsFrameBuf();
Expand All @@ -109,6 +111,8 @@ public void priorKnowledge() throws Exception {

@Test
public void upgrade() throws Exception {
setUpServerChannel();

String upgradeString = "GET / HTTP/1.1\r\n" +
"Host: example.com\r\n" +
"Connection: Upgrade, HTTP2-Settings\r\n" +
Expand Down Expand Up @@ -138,6 +142,8 @@ public void upgrade() throws Exception {

@Test
public void priorKnowledgeInFragments() throws Exception {
setUpServerChannel();

ByteBuf connectionPreface = Http2CodecUtil.connectionPrefaceBuf();
assertFalse(channel.writeInbound(connectionPreface.readBytes(5), connectionPreface));

Expand All @@ -156,6 +162,8 @@ public void priorKnowledgeInFragments() throws Exception {

@Test
public void downgrade() throws Exception {
setUpServerChannel();

String requestString = "GET / HTTP/1.1\r\n" +
"Host: example.com\r\n\r\n";
ByteBuf inbound = Unpooled.buffer().writeBytes(requestString.getBytes(CharsetUtil.US_ASCII));
Expand All @@ -175,6 +183,45 @@ public void downgrade() throws Exception {
assertNull(channel.readInbound());
}

@Test
public void usedHttp2Codec() throws Exception {
final Http2Codec http2Codec = new Http2CodecBuilder(true, new ChannelInitializer<Channel>() {
@Override
protected void initChannel(Channel ch) throws Exception {
}
}).build();
UpgradeCodecFactory upgradeCodecFactory = new UpgradeCodecFactory() {
@Override
public UpgradeCodec newUpgradeCodec(CharSequence protocol) {
return new Http2ServerUpgradeCodec(http2Codec);
}
};
http2ConnectionHandler = http2Codec.frameCodec().connectionHandler();

userEvents = new ArrayList<Object>();

HttpServerCodec httpServerCodec = new HttpServerCodec();
HttpServerUpgradeHandler upgradeHandler = new HttpServerUpgradeHandler(httpServerCodec, upgradeCodecFactory);

CleartextHttp2ServerUpgradeHandler handler = new CleartextHttp2ServerUpgradeHandler(
httpServerCodec, upgradeHandler, http2Codec);
channel = new EmbeddedChannel(handler, new ChannelInboundHandlerAdapter() {
@Override
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
userEvents.add(evt);
}
});

assertFalse(channel.writeInbound(Http2CodecUtil.connectionPrefaceBuf()));

ByteBuf settingsFrame = settingsFrameBuf();

assertFalse(channel.writeInbound(settingsFrame));

assertEquals(1, userEvents.size());
assertTrue(userEvents.get(0) instanceof PriorKnowledgeUpgradeEvent);
}

private static ByteBuf settingsFrameBuf() {
ByteBuf settingsFrame = Unpooled.buffer();
settingsFrame.writeMedium(12); // Payload length
Expand Down

0 comments on commit 8320a45

Please sign in to comment.