[#3724] HTTP/2 Headers END_STREAM results in RST_STREAM

Motivation:
If headers are sent on a stream that does not yet exist and the END_STREAM flag is set we will send a RST_STREAM frame. We should send the HEADERS frame and no RST_STREAM.

Modifications:
DefaultHttp2RemoteFlowController should allow frames to be sent if stream is created in the 'half closed (local)' state.

Result:
We can send HEADERS frame with the END_STREAM flag sent without sending a RST_STREAM frame.

codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java

@@ -434,9 +434,6 @@ public class DefaultHttp2Connection implements Http2Connection {
         public Http2Stream open(boolean halfClosed) throws Http2Exception {
             state = activeState(id, state, isLocal(), halfClosed);
             activate();
-            if (halfClosed) {
-                notifyHalfClosed(this);
-            }
             return this;
         }
 
@@ -900,9 +897,6 @@ public class DefaultHttp2Connection implements Http2Connection {
         public DefaultStream createStream(int streamId, boolean halfClosed) throws Http2Exception {
             DefaultStream stream = createStream(streamId, activeState(streamId, IDLE, isLocal(), halfClosed));
             stream.activate();
-            if (halfClosed) {
-                notifyHalfClosed(stream);
-            }
             return stream;
         }
 

codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java

@@ -529,7 +529,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
 
         /**
          * Helper method for determining whether or not to ignore inbound frames. A stream is considered to be created
-         * after a go away is sent if the following conditions hold:
+         * after a {@code GOAWAY} is sent if the following conditions hold:
          * <p/>
          * <ul>
          *     <li>A {@code GOAWAY} must have been sent by the local endpoint</li>

codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java

@@ -605,8 +605,7 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
                     frame.writeComplete();
                 }
             } catch (Throwable t) {
-                // Mark the state as cancelled, we'll clear the pending queue
-                // via cancel() below.
+                // Mark the state as cancelled, we'll clear the pending queue via cancel() below.
                 cancelled = true;
                 cause = t;
             } finally {

codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Connection.java

@@ -43,8 +43,10 @@ public interface Http2Connection {
         void onStreamActive(Http2Stream stream);
 
         /**
-         * Notifies the listener that the given stream is now {@code HALF CLOSED}. The stream can be
-         * inspected to determine which side is {@code CLOSED}.
+         * Notifies the listener that the given stream has transitioned from {@code OPEN} to {@code HALF CLOSED}.
+         * This method will <strong>not</strong> be called until a state transition occurs from when
+         * {@link #onStreamActive(Http2Stream)} was called.
+         * The stream can be inspected to determine which side is {@code HALF CLOSED}.
          * <p>
          * If a {@link RuntimeException} is thrown it will be logged and <strong>not propagated</strong>.
          * Throwing from this method is not supported and is considered a programming error.

codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java

@@ -42,6 +42,7 @@ import io.netty.channel.socket.nio.NioServerSocketChannel;
 import io.netty.channel.socket.nio.NioSocketChannel;
 import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpHeaderValues;
+import io.netty.handler.codec.http2.Http2Stream.State;
 import io.netty.handler.codec.http2.Http2TestUtil.FrameAdapter;
 import io.netty.handler.codec.http2.Http2TestUtil.FrameCountDown;
 import io.netty.handler.codec.http2.Http2TestUtil.Http2Runnable;
@@ -270,6 +271,12 @@ public class DataCompressionHttp2Test {
         clientConnection = new DefaultHttp2Connection(false);
 
         serverConnection.addListener(new Http2ConnectionAdapter() {
+            @Override
+            public void onStreamActive(Http2Stream stream) {
+                if (stream.state() == State.HALF_CLOSED_LOCAL || stream.state() == State.HALF_CLOSED_REMOTE) {
+                    serverLatch.countDown();
+                }
+            }
             @Override
             public void onStreamHalfClosed(Http2Stream stream) {
                 serverLatch.countDown();

codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionRoundtripTest.java

@@ -30,6 +30,7 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyBoolean;
 import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyLong;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doThrow;
@@ -110,9 +111,38 @@ public class Http2ConnectionRoundtripTest {
         clientGroup.sync();
     }
 
+    @Test
+    public void headersWithEndStreamShouldNotSendError() throws Exception {
+        bootstrapEnv(1, 1, 2, 1);
+
+        // Create a single stream by sending a HEADERS frame to the server.
+        final short weight = 16;
+        final Http2Headers headers = dummyHeaders();
+        runInChannel(clientChannel, new Http2Runnable() {
+            @Override
+            public void run() {
+                http2Client.encoder().writeHeaders(ctx(), 3, headers, 0, weight, false, 0, true,
+                        newPromise());
+                ctx().flush();
+            }
+        });
+
+        assertTrue(requestLatch.await(5, SECONDS));
+        verify(serverListener).onHeadersRead(any(ChannelHandlerContext.class), eq(3), eq(headers),
+                eq(0), eq(weight), eq(false), eq(0), eq(true));
+        // Wait for some time to see if a go_away or reset frame will be received.
+        Thread.sleep(1000);
+
+        // Verify that no errors have been received.
+        verify(serverListener, never()).onGoAwayRead(any(ChannelHandlerContext.class), anyInt(),
+                anyLong(), any(ByteBuf.class));
+        verify(serverListener, never()).onRstStreamRead(any(ChannelHandlerContext.class), anyInt(),
+                anyLong());
+    }
+
     @Test
     public void http2ExceptionInPipelineShouldCloseConnection() throws Exception {
-        bootstrapEnv(1, 1, 1, 1);
+        bootstrapEnv(1, 1, 2, 1);
 
         // Create a latch to track when the close occurs.
         final CountDownLatch closeLatch = new CountDownLatch(1);
@@ -190,7 +220,7 @@ public class Http2ConnectionRoundtripTest {
 
     @Test
     public void nonHttp2ExceptionInPipelineShouldNotCloseConnection() throws Exception {
-        bootstrapEnv(1, 1, 1, 1);
+        bootstrapEnv(1, 1, 2, 1);
 
         // Create a latch to track when the close occurs.
         final CountDownLatch closeLatch = new CountDownLatch(1);