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

Sending RST_STREAM when PUSH_PROMISE for a canceled stream arrives

Browse Source
This commit is contained in:
shorea 2018-06-18 22:11:16 -07:00 committed by Norman Maurer
parent cd9761b2fb
commit fb6c8c658b
4 changed files with 40 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -771,7 +771,9 @@ public class DefaultHttp2Connection implements Http2Connection {
@Override
public DefaultStream reservePushStream(int streamId, Http2Stream parent) throws Http2Exception {
if (parent == null) {
throw connectionError(PROTOCOL_ERROR, "Parent stream missing");
incrementExpectedStreamId(streamId);
throw streamError(streamId, Http2Error.REFUSED_STREAM, "PUSH_PROMISE sent on unknown stream, "
+ "likely discarded due to a RST_STREAM.");
}
if (isLocal() ? !parent.state().localSideOpen() : !parent.state().remoteSideOpen()) {
throw connectionError(PROTOCOL_ERROR, "Stream %d is not open for sending push promise", parent.id());

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

@ -501,26 +501,24 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
throw connectionError(PROTOCOL_ERROR, "A client cannot push.");
}
Http2Stream parentStream = connection.stream(streamId);
if (shouldIgnoreHeadersOrDataFrame(ctx, streamId, parentStream, "PUSH_PROMISE")) {
if (streamCreatedAfterGoAwaySent(promisedStreamId)) {
return;
}
if (parentStream == null) {
throw connectionError(PROTOCOL_ERROR, "Stream %d does not exist", streamId);
}
Http2Stream parentStream = connection.stream(streamId);
switch (parentStream.state()) {
case OPEN:
case HALF_CLOSED_LOCAL:
// Allowed to receive push promise in these states.
break;
default:
// Connection error.
throw connectionError(PROTOCOL_ERROR,
"Stream %d in unexpected state for receiving push promise: %s",
parentStream.id(), parentStream.state());
if (parentStream != null) {
switch (parentStream.state()) {
case OPEN:
case HALF_CLOSED_LOCAL:
// Allowed to receive push promise in these states.
break;
default:
// Connection error.
throw connectionError(PROTOCOL_ERROR,
"Stream %d in unexpected state for receiving push promise: %s",
parentStream.id(), parentStream.state());
}
}
if (!requestVerifier.isAuthoritative(ctx, headers)) {

27
codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
View File

@ -26,6 +26,7 @@ import junit.framework.AssertionFailedError;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.invocation.InvocationOnMock;
@ -586,7 +587,7 @@ public class DefaultHttp2ConnectionDecoderTest {
@Test
public void pushPromiseReadAfterGoAwaySentShouldBeIgnored() throws Exception {
mockGoAwaySent();
mockGoAwaySent(PUSH_STREAM_ID);
decode().onPushPromiseRead(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0);
verify(remote, never()).reservePushStream(anyInt(), any(Http2Stream.class));
verify(listener, never()).onPushPromiseRead(eq(ctx), anyInt(), anyInt(), any(Http2Headers.class), anyInt());
@ -600,21 +601,11 @@ public class DefaultHttp2ConnectionDecoderTest {
verify(listener).onPushPromiseRead(eq(ctx), anyInt(), anyInt(), any(Http2Headers.class), anyInt());
}
@Test
public void pushPromiseReadForUnknownStreamThatMayHaveExistedShouldBeIgnored() throws Exception {
when(connection.stream(STREAM_ID)).thenReturn(null);
decode().onPushPromiseRead(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0);
// Verify that the event was absorbed and not propagated to the observer.
verify(listener, never()).onPushPromiseRead(eq(ctx), anyInt(), anyInt(),
any(Http2Headers.class), anyInt());
verify(remote, never()).createStream(anyInt(), anyBoolean());
verify(stream, never()).open(anyBoolean());
}
@Test(expected = Http2Exception.class)
public void pushPromiseReadForUnknownStreamThatNeverExistedShouldThrow() throws Exception {
@Test(expected = Http2Exception.StreamException.class)
public void pushPromiseReadForUnknownStreamShouldThrowStreamException() throws Exception {
when(connection.streamMayHaveExisted(STREAM_ID)).thenReturn(false);
when(connection.remote().reservePushStream(anyInt(), ArgumentMatchers.<Http2Stream>isNull()))
.thenThrow(new Http2Exception.StreamException(PUSH_STREAM_ID, Http2Error.REFUSED_STREAM, "no stream"));
when(connection.stream(STREAM_ID)).thenReturn(null);
decode().onPushPromiseRead(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0);
}
@ -795,8 +786,12 @@ public class DefaultHttp2ConnectionDecoderTest {
}
private void mockGoAwaySent() {
mockGoAwaySent(STREAM_ID);
}
private void mockGoAwaySent(int streamId) {
when(connection.goAwaySent()).thenReturn(true);
when(remote.isValidStreamId(STREAM_ID)).thenReturn(true);
when(remote.isValidStreamId(streamId)).thenReturn(true);
when(remote.lastStreamKnownByPeer()).thenReturn(0);
}

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

@ -320,6 +320,18 @@ public class DefaultHttp2ConnectionTest {
assertEquals(2, server.local().lastStreamCreated());
}
@Test
public void reservePushStreamForUnknownParentStreamIncrementsExpectedStreamIdAndThrows() throws Http2Exception {
try {
assertEquals(0, server.local().lastStreamCreated());
server.local().reservePushStream(2, null);
fail("Expected StreamException");
} catch (Http2Exception.StreamException expected) {
// Next expected stream id should still be incremented.
assertEquals(2, server.local().lastStreamCreated());
}
}
@Test
public void clientReservePushStreamShouldSucceed() throws Http2Exception {
Http2Stream stream = server.remote().createStream(3, true);