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 f87e098d27..408f17dc4e 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 @@ -22,6 +22,7 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_WEIGHT; import static io.netty.handler.codec.http2.Http2CodecUtil.immediateRemovalPolicy; import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR; import static io.netty.handler.codec.http2.Http2Error.REFUSED_STREAM; +import static io.netty.handler.codec.http2.Http2Exception.closedStreamError; import static io.netty.handler.codec.http2.Http2Exception.connectionError; import static io.netty.handler.codec.http2.Http2Exception.streamError; import static io.netty.handler.codec.http2.Http2Stream.State.CLOSED; @@ -689,8 +690,7 @@ public class DefaultHttp2Connection implements Http2Connection { @Override public int nextStreamId() { - // For manually created client-side streams, 1 is reserved for HTTP upgrade, so - // start at 3. + // For manually created client-side streams, 1 is reserved for HTTP upgrade, so start at 3. return nextStreamId > 1 ? nextStreamId : nextStreamId + 2; } @@ -836,24 +836,22 @@ public class DefaultHttp2Connection implements Http2Connection { if (isGoAway()) { throw connectionError(PROTOCOL_ERROR, "Cannot create a stream since the connection is going away"); } - verifyStreamId(streamId); - if (!canCreateStream()) { - throw connectionError(REFUSED_STREAM, "Maximum streams exceeded for this endpoint."); - } - } - - private void verifyStreamId(int streamId) throws Http2Exception { if (streamId < 0) { throw new Http2NoMoreStreamIdsException(); } - if (streamId < nextStreamId) { - throw connectionError(PROTOCOL_ERROR, "Request stream %d is behind the next expected stream %d", - streamId, nextStreamId); - } if (!createdStreamId(streamId)) { throw connectionError(PROTOCOL_ERROR, "Request stream %d is not correct for %s connection", streamId, server ? "server" : "client"); } + // This check must be after all id validated checks, but before the max streams check because it may be + // recoverable to some degree for handling frames which can be sent on closed streams. + if (streamId < nextStreamId) { + throw closedStreamError(PROTOCOL_ERROR, "Request stream %d is behind the next expected stream %d", + streamId, nextStreamId); + } + if (!canCreateStream()) { + throw connectionError(REFUSED_STREAM, "Maximum streams exceeded for this endpoint."); + } } private boolean isLocal() { diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java index 5c21461704..33fbc1cf70 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java @@ -24,6 +24,7 @@ import static io.netty.handler.codec.http2.Http2Stream.State.CLOSED; import static io.netty.util.internal.ObjectUtil.checkNotNull; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; +import io.netty.handler.codec.http2.Http2Exception.ClosedStreamCreationException; import java.util.List; @@ -374,15 +375,20 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { return; } - if (stream == null) { - // PRIORITY frames always identify a stream. This means that if a PRIORITY frame is the - // first frame to be received for a stream that we must create the stream. - stream = connection.remote().createStream(streamId); - } + try { + if (stream == null) { + // PRIORITY frames always identify a stream. This means that if a PRIORITY frame is the + // first frame to be received for a stream that we must create the stream. + stream = connection.remote().createStream(streamId); + } - // This call will create a stream for streamDependency if necessary. - // For this reason it must be done before notifying the listener. - stream.setPriority(streamDependency, weight, exclusive); + // This call will create a stream for streamDependency if necessary. + // For this reason it must be done before notifying the listener. + stream.setPriority(streamDependency, weight, exclusive); + } catch (ClosedStreamCreationException ignored) { + // It is possible that either the stream for this frame or the parent stream is closed. + // In this case we should ignore the exception and allow the frame to be sent. + } listener.onPriorityRead(ctx, streamId, streamDependency, weight, exclusive); } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java index 37b723a03b..0e0bca4d80 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java @@ -24,6 +24,7 @@ import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPromise; +import io.netty.handler.codec.http2.Http2Exception.ClosedStreamCreationException; import io.netty.util.ReferenceCountUtil; import java.util.ArrayDeque; @@ -238,9 +239,14 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { stream = connection.local().createStream(streamId); } + // The set priority operation must be done before sending the frame. The parent may not yet exist + // and the priority tree may also be modified before sending. stream.setPriority(streamDependency, weight, exclusive); - } catch (Throwable e) { - return promise.setFailure(e); + } catch (ClosedStreamCreationException ignored) { + // It is possible that either the stream for this frame or the parent stream is closed. + // In this case we should ignore the exception and allow the frame to be sent. + } catch (Throwable t) { + return promise.setFailure(t); } ChannelFuture future = frameWriter.writePriority(ctx, streamId, streamDependency, weight, exclusive, promise); diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Exception.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Exception.java index 7f69fe3a58..3bfff8f1fe 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Exception.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Exception.java @@ -72,6 +72,18 @@ public class Http2Exception extends Exception { return new Http2Exception(error, String.format(fmt, args), cause); } + /** + * Use if an error has occurred which can not be isolated to a single stream, but instead applies + * to the entire connection. + * @param error The type of error as defined by the HTTP/2 specification. + * @param fmt String with the content and format for the additional debug data. + * @param args Objects which fit into the format defined by {@code fmt}. + * @return An exception which can be translated into a HTTP/2 error. + */ + public static Http2Exception closedStreamError(Http2Error error, String fmt, Object... args) { + return new ClosedStreamCreationException(error, String.format(fmt, args)); + } + /** * Use if an error which can be isolated to a single stream has occurred. If the {@code id} is not * {@link Http2CodecUtil#CONNECTION_STREAM_ID} then a {@link Http2Exception.StreamException} will be returned. @@ -130,6 +142,25 @@ public class Http2Exception extends Exception { return isStreamError(e) ? ((StreamException) e).streamId() : CONNECTION_STREAM_ID; } + /** + * Used when a stream creation attempt fails but may be because the stream was previously closed. + */ + public static final class ClosedStreamCreationException extends Http2Exception { + private static final long serialVersionUID = -1911637707391622439L; + + public ClosedStreamCreationException(Http2Error error) { + super(error); + } + + public ClosedStreamCreationException(Http2Error error, String message) { + super(error, message); + } + + public ClosedStreamCreationException(Http2Error error, String message, Throwable cause) { + super(error, message, cause); + } + } + /** * Represents an exception that can be isolated to a single stream (as opposed to the entire connection). */ diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameListener.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameListener.java index ab03d9e795..2bb9a2125f 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameListener.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameListener.java @@ -46,10 +46,10 @@ public interface Http2FrameListener { boolean endOfStream) throws Http2Exception; /** - * Handles an inbound HEADERS frame. + * Handles an inbound {@code HEADERS} frame. *

- * Only one of the following methods will be called for each HEADERS frame sequence. - * One will be called when the END_HEADERS flag has been received. + * Only one of the following methods will be called for each {@code HEADERS} frame sequence. + * One will be called when the {@code END_HEADERS} flag has been received. *