diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java index 7c52cd2b5c..de3d9f4605 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java @@ -22,11 +22,10 @@ import io.netty.util.internal.UnstableApi; import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_HEADER_LIST_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_INITIAL_HUFFMAN_DECODE_CAPACITY; import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_MAX_RESERVED_STREAMS; +import static io.netty.handler.codec.http2.Http2PromisedRequestVerifier.ALWAYS_VERIFY; import static io.netty.util.internal.ObjectUtil.checkNotNull; import static io.netty.util.internal.ObjectUtil.checkPositive; import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; -import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static java.util.concurrent.TimeUnit.SECONDS; /** * Abstract base class which defines commonly used features required to build {@link Http2ConnectionHandler} instances. @@ -106,6 +105,8 @@ public abstract class AbstractHttp2ConnectionHandlerBuilder outstandingLocalSettingsQueue = new ArrayDeque(4); + private final Queue outstandingLocalSettingsQueue = new ArrayDeque(4); + private Queue outstandingRemoteSettingsQueue; public DefaultHttp2ConnectionEncoder(Http2Connection connection, Http2FrameWriter frameWriter) { this.connection = checkNotNull(connection, "connection"); @@ -274,7 +278,32 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { @Override public ChannelFuture writeSettingsAck(ChannelHandlerContext ctx, ChannelPromise promise) { - return frameWriter.writeSettingsAck(ctx, promise); + if (outstandingRemoteSettingsQueue == null) { + return frameWriter.writeSettingsAck(ctx, promise); + } + Http2Settings settings = outstandingRemoteSettingsQueue.poll(); + if (settings == null) { + return promise.setFailure(new Http2Exception(INTERNAL_ERROR, "attempted to write a SETTINGS ACK with no " + + " pending SETTINGS")); + } + SimpleChannelPromiseAggregator aggregator = new SimpleChannelPromiseAggregator(promise, ctx.channel(), + ctx.executor()); + // Acknowledge receipt of the settings. We should do this before we process the settings to ensure our + // remote peer applies these settings before any subsequent frames that we may send which depend upon + // these new settings. See https://github.com/netty/netty/issues/6520. + frameWriter.writeSettingsAck(ctx, aggregator); + + // We create a "new promise" to make sure that status from both the write and the application are taken into + // account independently. + ChannelPromise applySettingsPromise = aggregator.newPromise(); + try { + remoteSettings(settings); + applySettingsPromise.setSuccess(); + } catch (Throwable e) { + applySettingsPromise.setFailure(e); + lifecycleManager.onError(ctx, true, e); + } + return aggregator.doneAllocatingPromises(); } @Override @@ -367,6 +396,14 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { return stream; } + @Override + public void consumeReceivedSettings(Http2Settings settings) { + if (outstandingRemoteSettingsQueue == null) { + outstandingRemoteSettingsQueue = new ArrayDeque(2); + } + outstandingRemoteSettingsQueue.add(settings); + } + /** * Wrap a DATA frame so it can be written subject to flow-control. Note that this implementation assumes it * only writes padding once for the entire payload as opposed to writing it once per-frame. This makes the diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2SettingsAckFrame.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2SettingsAckFrame.java new file mode 100644 index 0000000000..259b4a0068 --- /dev/null +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2SettingsAckFrame.java @@ -0,0 +1,33 @@ +/* + * Copyright 2019 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.codec.http2; + +import io.netty.util.internal.StringUtil; + +/** + * The default {@link Http2SettingsAckFrame} implementation. + */ +final class DefaultHttp2SettingsAckFrame implements Http2SettingsAckFrame { + @Override + public String name() { + return "SETTINGS(ACK)"; + } + + @Override + public String toString() { + return StringUtil.simpleClassName(this); + } +} diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2SettingsFrame.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2SettingsFrame.java index c60f59feec..f0b6b942fe 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2SettingsFrame.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2SettingsFrame.java @@ -42,6 +42,20 @@ public class DefaultHttp2SettingsFrame implements Http2SettingsFrame { return "SETTINGS"; } + @Override + public boolean equals(Object o) { + if (!(o instanceof Http2SettingsFrame)) { + return false; + } + Http2SettingsFrame other = (Http2SettingsFrame) o; + return settings.equals(other.settings()); + } + + @Override + public int hashCode() { + return settings.hashCode(); + } + @Override public String toString() { return StringUtil.simpleClassName(this) + "(settings=" + settings + ')'; diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java index 618a4a6771..da5bc85a4a 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java @@ -90,25 +90,6 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http } } - Http2ConnectionHandler(boolean server, Http2FrameWriter frameWriter, Http2FrameLogger frameLogger, - Http2Settings initialSettings) { - this.initialSettings = checkNotNull(initialSettings, "initialSettings"); - - Http2Connection connection = new DefaultHttp2Connection(server); - - Long maxHeaderListSize = initialSettings.maxHeaderListSize(); - Http2FrameReader frameReader = new DefaultHttp2FrameReader(maxHeaderListSize == null ? - new DefaultHttp2HeadersDecoder(true) : - new DefaultHttp2HeadersDecoder(true, maxHeaderListSize)); - - if (frameLogger != null) { - frameWriter = new Http2OutboundFrameLogger(frameWriter, frameLogger); - frameReader = new Http2InboundFrameLogger(frameReader, frameLogger); - } - encoder = new DefaultHttp2ConnectionEncoder(connection, frameWriter); - decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, frameReader); - } - /** * Get the amount of time (in milliseconds) this endpoint will wait for all streams to be closed before closing * the connection during the graceful shutdown process. Returns -1 if this connection is configured to wait 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 cf756cab25..4547f6188c 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 @@ -297,6 +297,10 @@ public class Http2FrameCodec extends Http2ConnectionHandler { encoder().writePing(ctx, frame.ack(), frame.content(), promise); } else if (msg instanceof Http2SettingsFrame) { encoder().writeSettings(ctx, ((Http2SettingsFrame) msg).settings(), promise); + } else if (msg instanceof Http2SettingsAckFrame) { + // In the event of manual SETTINGS ACK is is assumed the encoder will apply the earliest received but not + // yet ACKed settings. + encoder().writeSettingsAck(ctx, promise); } else if (msg instanceof Http2GoAwayFrame) { writeGoAwayFrame(ctx, (Http2GoAwayFrame) msg, promise); } else if (msg instanceof Http2UnknownFrame) { @@ -572,7 +576,7 @@ public class Http2FrameCodec extends Http2ConnectionHandler { @Override public void onSettingsAckRead(ChannelHandlerContext ctx) { - // TODO: Maybe handle me + onHttp2Frame(ctx, Http2SettingsAckFrame.INSTANCE); } @Override diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java index eb45723897..078630d94e 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java @@ -162,7 +162,8 @@ public class Http2FrameCodecBuilder extends if (encoderEnforceMaxConcurrentStreams()) { encoder = new StreamBufferingEncoder(encoder); } - Http2ConnectionDecoder decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, frameReader); + Http2ConnectionDecoder decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, frameReader, + promisedRequestVerifier(), isAutoAckSettingsFrame()); return build(decoder, encoder, initialSettings()); } 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 d19ce2b8f8..dc45839cd6 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 @@ -1119,7 +1119,7 @@ public class Http2MultiplexCodec extends Http2FrameCodec { } return; } - } else { + } else { String msgStr = msg.toString(); ReferenceCountUtil.release(msg); promise.setFailure(new IllegalArgumentException( diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java index c5732ec687..25f63a6a7e 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java @@ -165,6 +165,11 @@ public class Http2MultiplexCodecBuilder return super.initialHuffmanDecodeCapacity(initialHuffmanDecodeCapacity); } + @Override + public Http2MultiplexCodecBuilder autoAckSettingsFrame(boolean autoAckSettings) { + return super.autoAckSettingsFrame(autoAckSettings); + } + @Override public Http2MultiplexCodec build() { Http2FrameWriter frameWriter = this.frameWriter; @@ -185,7 +190,8 @@ public class Http2MultiplexCodecBuilder if (encoderEnforceMaxConcurrentStreams()) { encoder = new StreamBufferingEncoder(encoder); } - Http2ConnectionDecoder decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, frameReader); + Http2ConnectionDecoder decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, frameReader, + promisedRequestVerifier(), isAutoAckSettingsFrame()); return build(decoder, encoder, initialSettings()); } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2SettingsAckFrame.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2SettingsAckFrame.java new file mode 100644 index 0000000000..fc46ce50f7 --- /dev/null +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2SettingsAckFrame.java @@ -0,0 +1,29 @@ +/* + * Copyright 2019 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.codec.http2; + +/** + * An ack for a previously received {@link Http2SettingsFrame}. + *

+ * The HTTP/2 protocol enforces that ACKs are applied in + * order, so this ACK will apply to the earliest received and not yet ACKed {@link Http2SettingsFrame} frame. + */ +public interface Http2SettingsAckFrame extends Http2Frame { + Http2SettingsAckFrame INSTANCE = new DefaultHttp2SettingsAckFrame(); + + @Override + String name(); +} diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2SettingsReceivedConsumer.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2SettingsReceivedConsumer.java new file mode 100644 index 0000000000..97dfd3933b --- /dev/null +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2SettingsReceivedConsumer.java @@ -0,0 +1,25 @@ +/* + * Copyright 2019 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package io.netty.handler.codec.http2; + +/** + * Provides a Consumer like interface to consume remote settings received but not yet ACKed. + */ +public interface Http2SettingsReceivedConsumer { + /** + * Consume the most recently received but not yet ACKed settings. + */ + void consumeReceivedSettings(Http2Settings settings); +} diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java index 27d13cf50b..92045a8d6e 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java @@ -59,7 +59,6 @@ import static io.netty.handler.codec.http2.Http2TestUtil.anyChannelPromise; import static io.netty.handler.codec.http2.Http2TestUtil.anyHttp2Settings; import static io.netty.handler.codec.http2.Http2TestUtil.assertEqualsAndRelease; import static io.netty.handler.codec.http2.Http2TestUtil.bb; - import static org.hamcrest.Matchers.instanceOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -152,6 +151,8 @@ public class Http2FrameCodecTest { Http2SettingsFrame settingsFrame = inboundHandler.readInbound(); assertNotNull(settingsFrame); + Http2SettingsAckFrame settingsAckFrame = inboundHandler.readInbound(); + assertNotNull(settingsAckFrame); } @Test diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTest.java index 7788e6dd64..b01fe69c0a 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTest.java @@ -46,12 +46,11 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import static io.netty.util.ReferenceCountUtil.release; import static io.netty.handler.codec.http2.Http2TestUtil.anyChannelPromise; import static io.netty.handler.codec.http2.Http2TestUtil.anyHttp2Settings; import static io.netty.handler.codec.http2.Http2TestUtil.assertEqualsAndRelease; import static io.netty.handler.codec.http2.Http2TestUtil.bb; - +import static io.netty.util.ReferenceCountUtil.release; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -107,6 +106,8 @@ public class Http2MultiplexCodecTest { Http2SettingsFrame settingsFrame = parentChannel.readInbound(); assertNotNull(settingsFrame); + Http2SettingsAckFrame settingsAckFrame = parentChannel.readInbound(); + assertNotNull(settingsAckFrame); // Handshake verify(frameWriter).writeSettings(eqMultiplexCodecCtx(), diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTransportTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTransportTest.java new file mode 100644 index 0000000000..1a925a8369 --- /dev/null +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTransportTest.java @@ -0,0 +1,143 @@ +/* + * Copyright 2019 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.codec.http2; + +import io.netty.bootstrap.Bootstrap; +import io.netty.bootstrap.ServerBootstrap; +import io.netty.channel.Channel; +import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.channel.ChannelInitializer; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.nio.NioEventLoopGroup; +import io.netty.channel.socket.nio.NioServerSocketChannel; +import io.netty.channel.socket.nio.NioSocketChannel; +import io.netty.util.NetUtil; +import io.netty.util.ReferenceCountUtil; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.net.InetSocketAddress; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.junit.Assert.assertFalse; + +public class Http2MultiplexCodecTransportTest { + private EventLoopGroup eventLoopGroup; + private Channel clientChannel; + private Channel serverChannel; + private Channel serverConnectedChannel; + + @Before + public void setup() { + eventLoopGroup = new NioEventLoopGroup(); + } + + @After + public void teardown() { + if (clientChannel != null) { + clientChannel.close(); + } + if (serverChannel != null) { + serverChannel.close(); + } + if (serverConnectedChannel != null) { + serverConnectedChannel.close(); + } + eventLoopGroup.shutdownGracefully(0, 0, MILLISECONDS); + } + + @Test(timeout = 10000) + public void asyncSettingsAck() throws InterruptedException { + // The client expects 2 settings frames. One from the connection setup and one from this test. + final CountDownLatch serverAckOneLatch = new CountDownLatch(1); + final CountDownLatch serverAckAllLatch = new CountDownLatch(2); + final CountDownLatch clientSettingsLatch = new CountDownLatch(2); + final CountDownLatch serverConnectedChannelLatch = new CountDownLatch(1); + final AtomicReference serverConnectedChannelRef = new AtomicReference(); + ServerBootstrap sb = new ServerBootstrap(); + sb.group(eventLoopGroup); + sb.channel(NioServerSocketChannel.class); + sb.childHandler(new ChannelInitializer() { + @Override + protected void initChannel(Channel ch) { + ch.pipeline().addLast(Http2MultiplexCodecBuilder.forServer(new HttpInboundHandler()).build()); + ch.pipeline().addLast(new ChannelInboundHandlerAdapter() { + @Override + public void channelActive(ChannelHandlerContext ctx) { + serverConnectedChannelRef.set(ctx.channel()); + serverConnectedChannelLatch.countDown(); + } + + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) { + if (msg instanceof Http2SettingsAckFrame) { + serverAckOneLatch.countDown(); + serverAckAllLatch.countDown(); + } + ReferenceCountUtil.release(msg); + } + }); + } + }); + serverChannel = sb.bind(new InetSocketAddress(NetUtil.LOCALHOST, 0)).awaitUninterruptibly().channel(); + + Bootstrap bs = new Bootstrap(); + bs.group(eventLoopGroup); + bs.channel(NioSocketChannel.class); + bs.handler(new ChannelInitializer() { + @Override + protected void initChannel(Channel ch) { + ch.pipeline().addLast(Http2MultiplexCodecBuilder + .forClient(new HttpInboundHandler()).autoAckSettingsFrame(false).build()); + ch.pipeline().addLast(new ChannelInboundHandlerAdapter() { + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) { + if (msg instanceof Http2SettingsFrame) { + clientSettingsLatch.countDown(); + } + ReferenceCountUtil.release(msg); + } + }); + } + }); + clientChannel = bs.connect(serverChannel.localAddress()).awaitUninterruptibly().channel(); + serverConnectedChannelLatch.await(); + serverConnectedChannel = serverConnectedChannelRef.get(); + + serverConnectedChannel.writeAndFlush(new DefaultHttp2SettingsFrame(new Http2Settings() + .maxConcurrentStreams(10))).sync(); + + clientSettingsLatch.await(); + + // We expect a timeout here because we want to asynchronously generate the SETTINGS ACK below. + assertFalse(serverAckOneLatch.await(300, MILLISECONDS)); + + // We expect 2 settings frames, the initial settings frame during connection establishment and the setting frame + // written in this test. We should ack both of these settings frames. + clientChannel.writeAndFlush(Http2SettingsAckFrame.INSTANCE).sync(); + clientChannel.writeAndFlush(Http2SettingsAckFrame.INSTANCE).sync(); + + serverAckAllLatch.await(); + } + + @ChannelHandler.Sharable + private static final class HttpInboundHandler extends ChannelInboundHandlerAdapter { } +}