diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java index 71bac7f6e7..ac9ea6d79c 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java @@ -232,13 +232,6 @@ public class Http2FrameCodec extends Http2ConnectionHandler { // sub-class can override this for extra steps that needs to be done when the handler is added. } - @Override - public void onHttpClientUpgrade() throws Http2Exception { - super.onHttpClientUpgrade(); - // Now make a new Http2FrameStream, set it's underlying Http2Stream, and initialize it. - newStream().setStreamAndProperty(streamKey, connection().stream(HTTP_UPGRADE_STREAM_ID)); - } - /** * Handles the cleartext HTTP upgrade event. If an upgrade occurred, sends a simple response via * HTTP/2 on stream 1 (the stream specifically reserved for cleartext HTTP upgrade). diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java index db31b57001..d9b28473f1 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java @@ -149,39 +149,35 @@ public class Http2MultiplexCodec extends Http2FrameCodec { ctx.fireChannelRead(frame); } - private void onHttp2UpgradeStreamInitialized(ChannelHandlerContext ctx, DefaultHttp2FrameStream stream) { - assert stream.state() == Http2Stream.State.HALF_CLOSED_LOCAL; - AbstractHttp2StreamChannel ch = new Http2MultiplexCodecStreamChannel(stream, null); - ch.closeOutbound(); - - // Add our upgrade handler to the channel and then register the channel. - // The register call fires the channelActive, etc. - ch.pipeline().addLast(upgradeStreamHandler); - ChannelFuture future = ch.register(); - if (future.isDone()) { - Http2MultiplexHandler.registerDone(future); - } else { - future.addListener(Http2MultiplexHandler.CHILD_CHANNEL_REGISTRATION_LISTENER); - } - } - @Override final void onHttp2StreamStateChanged(ChannelHandlerContext ctx, DefaultHttp2FrameStream stream) { - switch (stream.state()) { case HALF_CLOSED_LOCAL: - if (stream.id() == HTTP_UPGRADE_STREAM_ID) { - onHttp2UpgradeStreamInitialized(ctx, stream); + if (stream.id() != HTTP_UPGRADE_STREAM_ID) { + // Ignore everything which was not caused by an upgrade + break; } - break; + // fall-through case HALF_CLOSED_REMOTE: + // fall-through case OPEN: if (stream.attachment != null) { // ignore if child channel was already created. break; } - // fall-trough - ChannelFuture future = new Http2MultiplexCodecStreamChannel(stream, inboundStreamHandler).register(); + final Http2MultiplexCodecStreamChannel streamChannel; + // We need to handle upgrades special when on the client side. + if (stream.id() == HTTP_UPGRADE_STREAM_ID && !connection().isServer()) { + // Add our upgrade handler to the channel and then register the channel. + // The register call fires the channelActive, etc. + assert upgradeStreamHandler != null; + streamChannel = new Http2MultiplexCodecStreamChannel(stream, upgradeStreamHandler); + streamChannel.closeOutbound(); + } else { + streamChannel = new Http2MultiplexCodecStreamChannel(stream, inboundStreamHandler); + } + + ChannelFuture future = streamChannel.register(); if (future.isDone()) { Http2MultiplexHandler.registerDone(future); } else { diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexHandler.java index b19556060b..5943d0a682 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexHandler.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexHandler.java @@ -209,26 +209,24 @@ public final class Http2MultiplexHandler extends Http2ChannelDuplexHandler { // Ignore everything which was not caused by an upgrade break; } - // We must have an upgrade handler or else we can't handle the stream - if (upgradeStreamHandler == null) { - throw connectionError(INTERNAL_ERROR, - "Client is misconfigured for upgrade requests"); - } - // fall-trough + // fall-through case HALF_CLOSED_REMOTE: - // fall-trough + // fall-through case OPEN: if (stream.attachment != null) { // ignore if child channel was already created. break; } final AbstractHttp2StreamChannel ch; - if (stream.state() == Http2Stream.State.HALF_CLOSED_LOCAL) { - ch = new Http2MultiplexHandlerStreamChannel(stream, null); + // We need to handle upgrades special when on the client side. + if (stream.id() == Http2CodecUtil.HTTP_UPGRADE_STREAM_ID && !isServer(ctx)) { + // We must have an upgrade handler or else we can't handle the stream + if (upgradeStreamHandler == null) { + throw connectionError(INTERNAL_ERROR, + "Client is misconfigured for upgrade requests"); + } + ch = new Http2MultiplexHandlerStreamChannel(stream, upgradeStreamHandler); ch.closeOutbound(); - // Add our upgrade handler to the channel and then register the channel. - // The register call fires the channelActive, etc. - ch.pipeline().addLast(upgradeStreamHandler); } else { ch = new Http2MultiplexHandlerStreamChannel(stream, inboundStreamHandler); } @@ -277,9 +275,13 @@ public final class Http2MultiplexHandler extends Http2ChannelDuplexHandler { ctx.fireExceptionCaught(cause); } + private static boolean isServer(ChannelHandlerContext ctx) { + return ctx.channel().parent() instanceof ServerChannel; + } + private void onHttp2GoAwayFrame(ChannelHandlerContext ctx, final Http2GoAwayFrame goAwayFrame) { try { - final boolean server = ctx.channel().parent() instanceof ServerChannel; + final boolean server = isServer(ctx); forEachActiveStream(stream -> { final int streamId = stream.id(); if (streamId > goAwayFrame.lastStreamId() && Http2CodecUtil.isStreamIdValid(streamId, server)) { diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexClientUpgradeTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexClientUpgradeTest.java index 930eeba3f1..f08f217ad0 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexClientUpgradeTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexClientUpgradeTest.java @@ -22,21 +22,23 @@ import io.netty.channel.embedded.EmbeddedChannel; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; public abstract class Http2MultiplexClientUpgradeTest { @ChannelHandler.Sharable - final class NoopHandler implements ChannelHandler { + static final class NoopHandler implements ChannelHandler { @Override public void channelActive(ChannelHandlerContext ctx) { ctx.channel().close(); } } - private final class UpgradeHandler implements ChannelHandler { + private static final class UpgradeHandler implements ChannelHandler { Http2Stream.State stateOnActive; int streamId; + boolean channelInactiveCalled; @Override public void channelActive(ChannelHandlerContext ctx) throws Exception { @@ -45,6 +47,12 @@ public abstract class Http2MultiplexClientUpgradeTest streamId = ch.stream().id(); ctx.fireChannelActive(); } + + @Override + public void channelInactive(ChannelHandlerContext ctx) throws Exception { + channelInactiveCalled = true; + ctx.fireChannelInactive(); + } } protected abstract C newCodec(ChannelHandler upgradeHandler); @@ -61,8 +69,10 @@ public abstract class Http2MultiplexClientUpgradeTest assertFalse(upgradeHandler.stateOnActive.localSideOpen()); assertTrue(upgradeHandler.stateOnActive.remoteSideOpen()); - assertEquals(1, upgradeHandler.streamId); + assertNotNull(codec.connection().stream(Http2CodecUtil.HTTP_UPGRADE_STREAM_ID).getProperty(codec.streamKey)); + assertEquals(Http2CodecUtil.HTTP_UPGRADE_STREAM_ID, upgradeHandler.streamId); assertTrue(ch.finishAndReleaseAll()); + assertTrue(upgradeHandler.channelInactiveCalled); } @Test(expected = Http2Exception.class) diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodecTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodecTest.java index f6788a52e8..22ed1d42de 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodecTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodecTest.java @@ -17,6 +17,8 @@ package io.netty.handler.codec.http2; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.DefaultChannelId; +import io.netty.channel.ServerChannel; import io.netty.channel.embedded.EmbeddedChannel; import io.netty.handler.codec.http.DefaultFullHttpRequest; import io.netty.handler.codec.http.DefaultHttpHeaders; @@ -31,6 +33,7 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import org.junit.Test; +import org.mockito.Mockito; public class Http2ServerUpgradeCodecTest { @@ -62,7 +65,9 @@ public class Http2ServerUpgradeCodecTest { request.headers().set(HttpHeaderNames.UPGRADE, "h2c"); request.headers().set("HTTP2-Settings", "AAMAAABkAAQAAP__"); - EmbeddedChannel channel = new EmbeddedChannel(new ChannelHandler() { }); + ServerChannel parent = Mockito.mock(ServerChannel.class); + EmbeddedChannel channel = new EmbeddedChannel(parent, DefaultChannelId.newInstance(), true, false, + new ChannelHandler() { }); ChannelHandlerContext ctx = channel.pipeline().firstContext(); Http2ServerUpgradeCodec codec; if (multiplexer == null) {