andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Motivation: (#7848)

Browse Source 
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.
This commit is contained in:
Bryce Anderson 2018-04-21 00:23:15 -06:00 committed by Norman Maurer
parent b75f44db9a
commit f9604eeff5
3 changed files with 12 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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);
}

4
codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
View File

@ -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>

2
codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java
View File

@ -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());