Repository: netty/netty
File: codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java (Java)

Code before:
    @Override
    public void readFrame(ChannelHandlerContext ctx, ByteBuf input, Http2FrameListener listener)
            throws Http2Exception {
        if (readError) {
            input.skipBytes(input.readableBytes());
            return;
        }
        try {
            do {
                if (readingHeaders && !preProcessFrame(input)) {
                    return;
                }
                verifyHeaderState();
                // The header is complete, fall into the next case to process the payload.
                // This is to ensure the proper handling of zero-length payloads. In this
                // case, we don't want to loop around because there may be no more data
                // available, causing us to exit the loop. Instead, we just want to perform
                // the first pass at payload processing now.

                // Wait until the entire payload has been read.
                if (input.readableBytes() < payloadLength) {
                    return;
                }
                // Slice to work only on the frame being read
                ByteBuf framePayload = input.readSlice(payloadLength);
                processPayloadState(ctx, framePayload, listener);
            } while (input.isReadable());
        } catch (Http2Exception e) {
            readError = !Http2Exception.isStreamError(e);
            // Stream error found, reset to reading headers once again for next request coming in
            if (!readError) {
                readingHeaders = true;
            }
            throw e;
        } catch (RuntimeException e) {
            readError = true;
            throw e;
        } catch (Throwable cause) {
            readError = true;
            PlatformDependent.throwException(cause);
        }
    }

    private boolean preProcessFrame(ByteBuf in) throws Http2Exception {
        // Start pre-processing the frame by reading the necessary data
        // in common between all frame types
        if (in.readableBytes() < FRAME_HEADER_LENGTH) {
            // Wait until the entire framing section has been read.
            return false;
        }
        payloadLength = in.readUnsignedMedium();

Review comment (suggestion):
Some of the verifications result in a stream error, not a connection error. If that happens I think we end up with a corrupt stream. To fix it we can simply move verification to after we've called input.readSlice(..) so that we've consumed the frame from the framing layer. We've already committed to reading frames this large so it doesn't seem like a potential attack vector. The only downside seems to be that for connection errors it can potentially delay closing the session.

Code after:
    @Override
    public void readFrame(ChannelHandlerContext ctx, ByteBuf input, Http2FrameListener listener)
            throws Http2Exception {
        if (readError) {
            input.skipBytes(input.readableBytes());
            return;
        }
        try {
            do {
                if (readingHeaders && !preProcessFrame(input)) {
                    return;
                }
                // The header is complete, fall into the next case to process the payload.
                // This is to ensure the proper handling of zero-length payloads. In this
                // case, we don't want to loop around because there may be no more data
                // available, causing us to exit the loop. Instead, we just want to perform
                // the first pass at payload processing now.
                // Wait until the entire payload has been read.
                if (input.readableBytes() < payloadLength) {
                    return;
                }
                // Slice to work only on the frame being read
                ByteBuf framePayload = input.readSlice(payloadLength);
                // We have consumed the data for this frame, next time we read,
                // we will be expecting to read a new frame header.
                readingHeaders = true;
                verifyFrameState();
                processPayloadState(ctx, framePayload, listener);
            } while (input.isReadable());
        } catch (Http2Exception e) {
            readError = !Http2Exception.isStreamError(e);
            throw e;
        } catch (RuntimeException e) {
            readError = true;
            throw e;
        } catch (Throwable cause) {
            readError = true;
            PlatformDependent.throwException(cause);
        }
    }

    private boolean preProcessFrame(ByteBuf in) throws Http2Exception {
        // Start pre-processing the frame by reading the necessary data
        // in common between all frame types
        if (in.readableBytes() < FRAME_HEADER_LENGTH) {
            // Wait until the entire framing section has been read.
            return false;
        }
        payloadLength = in.readUnsignedMedium();
        if (payloadLength > maxFrameSize) {
            throw connectionError(FRAME_SIZE_ERROR, "Frame length: %d exceeds maximum: %d", payloadLength,