From f9604eeff5019219ea2895556309735bb27077fe Mon Sep 17 00:00:00 2001
From: Bryce Anderson <banderson@twitter.com>
Date: Sat, 21 Apr 2018 00:23:15 -0600
Subject: [PATCH] Motivation: (#7848)

It is possible to create streams in the half-closed state where the
stream state doesn't reflect that the request headers have been sent by
the client or the server hasn't received the request headers. This
state isn't possible in the H2 spec as a half closed stream must have
either received a full request or have received the headers from a
pushed stream. In the current implementation, this can cause the stream
created as part of an h2c upgrade request to be in this invalid state
and result in the omission of RST frames as the client doesn't believe
it has sent the request to begin with.

Modification:

The `DefaultHttp2Connection.activate` method checks the state and
modifies the status of the request headers as appropriate.

Result:

Fixes #7847.
---
 .../netty/handler/codec/http2/DefaultHttp2Connection.java | 8 ++++++++
 .../java/io/netty/handler/codec/http2/Http2Stream.java    | 4 ++--
 .../handler/codec/http2/DefaultHttp2ConnectionTest.java   | 2 ++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java
index 12815c225c..4251317457 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java
@@ -478,11 +478,19 @@ public class DefaultHttp2Connection implements Http2Connection {
             if (!createdBy().canOpenStream()) {
                 throw connectionError(PROTOCOL_ERROR, "Maximum active streams violated for this endpoint.");
             }
+
             activate();
             return this;
         }
 
         void activate() {
+            // If the stream is opened in a half-closed state, the headers must have either
+            // been sent if this is a local stream, or received if it is a remote stream.
+            if (state == HALF_CLOSED_LOCAL) {
+                headersSent(/*isInformational*/ false);
+            } else if (state == HALF_CLOSED_REMOTE) {
+                headersReceived(/*isInformational*/ false);
+            }
             activeStreams.activate(this);
         }
 
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
index 3b654425cf..6db9d78a80 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
@@ -76,9 +76,9 @@ public interface Http2Stream {
      * <ul>
      * <li>{@link State#OPEN} if {@link #state()} is {@link State#IDLE} and {@code halfClosed} is {@code false}.</li>
      * <li>{@link State#HALF_CLOSED_LOCAL} if {@link #state()} is {@link State#IDLE} and {@code halfClosed}
-     * is {@code true} and the stream is local.</li>
+     * is {@code true} and the stream is local. In this state, {@link #isHeadersSent()} is {@code true}</li>
      * <li>{@link State#HALF_CLOSED_REMOTE} if {@link #state()} is {@link State#IDLE} and {@code halfClosed}
-     * is {@code true} and the stream is remote.</li>
+     * is {@code true} and the stream is remote. In this state, {@link #isHeadersReceived()} is {@code true}</li>
      * <li>{@link State#RESERVED_LOCAL} if {@link #state()} is {@link State#HALF_CLOSED_REMOTE}.</li>
      * <li>{@link State#RESERVED_REMOTE} if {@link #state()} is {@link State#HALF_CLOSED_LOCAL}.</li>
      * </ul>
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java
index 966f668b40..ef385beed6 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java
@@ -294,12 +294,14 @@ public class DefaultHttp2ConnectionTest {
         assertEquals(State.HALF_CLOSED_REMOTE, stream.state());
         assertEquals(2, client.numActiveStreams());
         assertEquals(4, client.remote().lastStreamCreated());
+        assertTrue(stream.isHeadersReceived());
 
         stream = client.local().createStream(3, true);
         assertEquals(3, stream.id());
         assertEquals(State.HALF_CLOSED_LOCAL, stream.state());
         assertEquals(3, client.numActiveStreams());
         assertEquals(3, client.local().lastStreamCreated());
+        assertTrue(stream.isHeadersSent());
 
         stream = client.local().createStream(5, false);
         assertEquals(5, stream.id());