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

Code before:

    @Test
    public void verifyValidRequestAfterMalformedPacketCausesStreamException() throws Http2Exception {
        final ByteBuf input = Unpooled.buffer();
        int priorityStreamId = 3, headerStreamId = 5;
        try {
            // Write a malformed priority header causing a stream exception in reader
            writeFrameHeader(input, 4, PRIORITY, new Http2Flags(), priorityStreamId);
            assertThrows(Http2Exception.class, new Executable() {
                @Override
                public void execute() throws Throwable {
                    try {
                        frameReader.readFrame(ctx, input, listener);
                    } catch (Exception e) {
                        if (e instanceof Http2Exception && Http2Exception.isStreamError((Http2Exception) e)) {
                            throw e;
                        }
                    }
                }
            });
            // Verify that after stream exception we accept new stream requests
            Http2Headers headers = new DefaultHttp2Headers()
                    .authority("foo")
                    .method("get")
                    .path("/")
                    .scheme("https");
            Http2Flags flags = new Http2Flags().endOfHeaders(true).endOfStream(true);
            writeHeaderFrame(input, headerStreamId, headers, flags);
            frameReader.readFrame(ctx, input, listener);
            verify(listener).onHeadersRead(ctx, 5, headers, 0, true);
        } finally {
            input.release();
        }
    }

    private void writeHeaderFrame(
            ByteBuf output, int streamId, Http2Headers headers,
            Http2Flags flags) throws Http2Exception {
        ByteBuf headerBlock = Unpooled.buffer();
        try {
            hpackEncoder.encodeHeaders(streamId, headerBlock, headers, Http2HeadersEncoder.NEVER_SENSITIVE);
            writeFrameHeader(output, headerBlock.readableBytes(), HEADERS, flags, streamId);
            output.writeBytes(headerBlock, headerBlock.readableBytes());
        } finally {
            headerBlock.release();
        }
    }

    private void writeHeaderFrameWithData(
            ByteBuf output, int streamId, Http2Headers headers,
            ByteBuf dataPayload) throws Http2Exception {

Review comment (suggestion):
@isaacrivriv I read description of #13788 to understand expected behavior, but it seems like this is not what RFC wants. [Section 4.2](https://datatracker.ietf.org/doc/html/rfc9113#section-4.2) says:

> An endpoint MUST send an error code of [FRAME_SIZE_ERROR](https://datatracker.ietf.org/doc/html/rfc9113#FRAME_SIZE_ERROR) if a frame exceeds the size defined in [SETTINGS_MAX_FRAME_SIZE](https://datatracker.ietf.org/doc/html/rfc9113#SETTINGS_MAX_FRAME_SIZE), exceeds any limit defined for the frame type, or is too small to contain mandatory frame data. A frame size error in a frame that could alter the state of the entire connection MUST be treated as a [connection error](https://datatracker.ietf.org/doc/html/rfc9113#ConnectionErrorHandler) ([Section 5.4.1](https://datatracker.ietf.org/doc/html/rfc9113#ConnectionErrorHandler))


Then section 5.4.1 describes that the state of the entire connection is expected to be corrupted at that point and we must close it. It's not expected that we could recover and process more frames on the same connection.

If we don't already have one, it will be helpful if you could implement an integration test with a full HTTP/2 server and a faked client that send a malformed frame, then assert that the server responds with `FRAME_SIZE_ERROR`, then closes connection. 

Code after:

    @Test
    public void verifyValidRequestAfterMalformedPacketCausesStreamException() throws Http2Exception {
        final ByteBuf input = Unpooled.buffer();
        int priorityStreamId = 3, headerStreamId = 5;
        try {
            // Write a malformed priority header causing a stream exception in reader
            writeFrameHeader(input, 4, PRIORITY, new Http2Flags(), priorityStreamId);
            // Fill buffer with dummy payload to be properly read by reader
            input.writeByte((byte) 0x80);
            input.writeByte((byte) 0x00);
            input.writeByte((byte) 0x00);
            input.writeByte((byte) 0x7f);
            assertThrows(Http2Exception.class, new Executable() {
                @Override
                public void execute() throws Throwable {
                    try {
                        frameReader.readFrame(ctx, input, listener);
                    } catch (Exception e) {
                        if (e instanceof Http2Exception && Http2Exception.isStreamError((Http2Exception) e)) {
                            throw e;
                        }
                    }
                }
            });
            // Verify that after stream exception we accept new stream requests
            Http2Headers headers = new DefaultHttp2Headers()
                    .authority("foo")
                    .method("get")
                    .path("/")
                    .scheme("https");
            Http2Flags flags = new Http2Flags().endOfHeaders(true).endOfStream(true);
            writeHeaderFrame(input, headerStreamId, headers, flags);
            frameReader.readFrame(ctx, input, listener);
            verify(listener).onHeadersRead(ctx, 5, headers, 0, true);
        } finally {
            input.release();
        }
    }

    private void writeHeaderFrame(
            ByteBuf output, int streamId, Http2Headers headers,
            Http2Flags flags) throws Http2Exception {
        ByteBuf headerBlock = Unpooled.buffer();
        try {
            hpackEncoder.encodeHeaders(streamId, headerBlock, headers, Http2HeadersEncoder.NEVER_SENSITIVE);
            writeFrameHeader(output, headerBlock.readableBytes(), HEADERS, flags, streamId);
            output.writeBytes(headerBlock, headerBlock.readableBytes());
        } finally {
            headerBlock.release();
        }