From de4627852f5b1c42fd0b8cb2d66634be569a86a3 Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Wed, 5 Aug 2020 23:34:10 -0700 Subject: [PATCH] New H2 stream promise may never complete (#10443) Motivation: `Http2StreamChannelBootstrap#open0` invokes `Http2MultiplexHandler#newOutboundStream()` which may throw an `IllegalStateException`. In this case, it will never complete the passed promise. Modifications: - `try-catch` all invocations of `newOutboundStream()` and fail promise in case of any exception; Result: New H2 stream promise always completes. --- .../http2/Http2StreamChannelBootstrap.java | 13 +++-- .../Http2StreamChannelBootstrapTest.java | 50 +++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 codec-http2/src/test/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrapTest.java diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java index f3007ff7a2..218de8ee30 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java @@ -168,10 +168,15 @@ public final class Http2StreamChannelBootstrap { return; } final Http2StreamChannel streamChannel; - if (ctx.handler() instanceof Http2MultiplexCodec) { - streamChannel = ((Http2MultiplexCodec) ctx.handler()).newOutboundStream(); - } else { - streamChannel = ((Http2MultiplexHandler) ctx.handler()).newOutboundStream(); + try { + if (ctx.handler() instanceof Http2MultiplexCodec) { + streamChannel = ((Http2MultiplexCodec) ctx.handler()).newOutboundStream(); + } else { + streamChannel = ((Http2MultiplexHandler) ctx.handler()).newOutboundStream(); + } + } catch (Exception e) { + promise.setFailure(e); + return; } try { init(streamChannel); diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrapTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrapTest.java new file mode 100644 index 0000000000..e941fc5586 --- /dev/null +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrapTest.java @@ -0,0 +1,50 @@ +/* + * Copyright 2020 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.channel.Channel; +import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelHandlerContext; +import io.netty.util.concurrent.DefaultPromise; +import io.netty.util.concurrent.EventExecutor; +import io.netty.util.concurrent.Promise; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class Http2StreamChannelBootstrapTest { + + @Test + public void open0FailsPromiseOnHttp2MultiplexHandlerError() { + Http2StreamChannelBootstrap bootstrap = new Http2StreamChannelBootstrap(mock(Channel.class)); + + Http2MultiplexHandler handler = new Http2MultiplexHandler(mock(ChannelHandler.class)); + EventExecutor executor = mock(EventExecutor.class); + when(executor.inEventLoop()).thenReturn(true); + ChannelHandlerContext ctx = mock(ChannelHandlerContext.class); + when(ctx.executor()).thenReturn(executor); + when(ctx.handler()).thenReturn(handler); + + Promise promise = new DefaultPromise(mock(EventExecutor.class)); + bootstrap.open0(ctx, promise); + assertThat(promise.isDone(), is(true)); + assertThat(promise.cause(), is(instanceOf(IllegalStateException.class))); + } +}