HTTP/2 Send GOAWAY if no more stream ids

Motivation:
If the http2 encoder has exhausted all available stream IDs a GOAWAY frame is not sent. Once the encoder detects the a new stream ID has rolled over past the last stream ID a GOAWAY should be sent as recommended in section [5.1.1](https://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-5.1.1).

Modifications:
-This condition is already detected but it just needs to result in a GOAWAY being sent.
-Add a subclass of Http2Exception so the encoder can detect this special case.
-Add a unit test which checks that the GOAWAY is sent/received.

Result:
Encoder attempting to use the first 'rolled over' stream id results in a GOAWAY being sent.
This commit is contained in:
Scott Mitchell 2014-10-21 20:16:23 -04:00
parent 7c43d9b84a
commit 99376c4391
6 changed files with 68 additions and 10 deletions

View File

@ -854,8 +854,8 @@ public class DefaultHttp2Connection implements Http2Connection {
}
private void verifyStreamId(int streamId) throws Http2Exception {
if (nextStreamId < 0) {
throw protocolError("No more streams can be created on this connection");
if (streamId < 0) {
throw new Http2NoMoreStreamIdsException();
}
if (streamId < nextStreamId) {
throw protocolError("Request stream %d is behind the next expected stream %d", streamId, nextStreamId);

View File

@ -239,6 +239,9 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder {
}
}
} catch (Throwable e) {
if (e instanceof Http2NoMoreStreamIdsException) {
lifecycleManager.onException(ctx, e);
}
return promise.setFailure(e);
}

View File

@ -329,7 +329,8 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
*/
public ChannelFuture writeGoAway(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData,
ChannelPromise promise) {
if (connection().isGoAway()) {
Http2Connection connection = connection();
if (connection.isGoAway()) {
debugData.release();
return ctx.newSucceededFuture();
}
@ -337,7 +338,7 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
ChannelFuture future = frameWriter().writeGoAway(ctx, lastStreamId, errorCode, debugData, promise);
ctx.flush();
connection().goAwaySent(lastStreamId);
connection.goAwaySent(lastStreamId);
return future;
}
@ -345,15 +346,16 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
* Sends a {@code GO_AWAY} frame appropriate for the given exception.
*/
private ChannelFuture writeGoAway(ChannelHandlerContext ctx, Http2Exception cause) {
if (connection().isGoAway()) {
Http2Connection connection = connection();
if (connection.isGoAway()) {
return ctx.newSucceededFuture();
}
// The connection isn't alredy going away, send the GO_AWAY frame now to start
// the process.
int errorCode = cause != null ? cause.error().code() : NO_ERROR.code();
long errorCode = cause != null ? cause.error().code() : NO_ERROR.code();
ByteBuf debugData = Http2CodecUtil.toByteBuf(ctx, cause);
int lastKnownStream = connection().remote().lastStreamCreated();
int lastKnownStream = connection.remote().lastStreamCreated();
return writeGoAway(ctx, lastKnownStream, errorCode, debugData, ctx.newPromise());
}

View File

@ -33,16 +33,16 @@ public enum Http2Error {
ENHANCE_YOUR_CALM(0xB),
INADEQUATE_SECURITY(0xC);
private final int code;
private final long code;
Http2Error(int code) {
Http2Error(long code) {
this.code = code;
}
/**
* Gets the code for this error used on the wire.
*/
public int code() {
public long code() {
return code;
}
}

View File

@ -0,0 +1,31 @@
/*
* Copyright 2014 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;
/**
* This exception is thrown when there are no more stream IDs available for the current connection
*/
public class Http2NoMoreStreamIdsException extends Http2Exception {
private static final long serialVersionUID = -7756236161274851110L;
private static final String ERROR_MESSAGE = "No more streams can be created on this connection";
public Http2NoMoreStreamIdsException() {
super(Http2Error.PROTOCOL_ERROR, ERROR_MESSAGE);
}
public Http2NoMoreStreamIdsException(Throwable cause) {
super(Http2Error.PROTOCOL_ERROR, ERROR_MESSAGE, cause);
}
}

View File

@ -217,6 +217,28 @@ public class Http2ConnectionRoundtripTest {
assertTrue(clientChannel.isOpen());
}
@Test
public void noMoreStreamIdsShouldSendGoAway() throws Exception {
bootstrapEnv(1, 3);
// Create a single stream by sending a HEADERS frame to the server.
final Http2Headers headers = dummyHeaders();
runInChannel(clientChannel, new Http2Runnable() {
@Override
public void run() {
http2Client.encoder().writeHeaders(ctx(), 3, headers, 0, (short) 16, false, 0,
true, newPromise());
http2Client.encoder().writeHeaders(ctx(), Integer.MAX_VALUE + 1, headers, 0, (short) 16, false, 0,
true, newPromise());
}
});
// Wait for the server to create the stream.
assertTrue(requestLatch.await(5, TimeUnit.SECONDS));
verify(serverListener).onGoAwayRead(any(ChannelHandlerContext.class), eq(0),
eq(Http2Error.PROTOCOL_ERROR.code()), any(ByteBuf.class));
}
@Test
public void flowControlProperlyChunksLargeMessage() throws Exception {
final Http2Headers headers = dummyHeaders();