Repository: netty/netty
Issue: DefaultHttp2FrameReader does not reset after malformed request comes in

### Expected behavior
After a request comes in which causes a stream error, (such as a priority frame with an invalid stream dependency or with an invalid payload length) the following frame must be treated as a new frame.

### Actual behavior
After such a malformed request comes in, the following frame after the malformed is parsed with leftover data from the previous request.

### Steps to reproduce
1. Start an HTTP2 request. 
2. Send a priority request on a stream with an invalid payload length
3. After failed stream send a valid stream request to the HTTP2 session
4. HTTP2 request should work but doesn't

As a side effect of this, I see a Go Away being sent by the server with the following message `Frame length: 16711680 exceeds maximum: 16384`

### Minimal yet complete reproducer code (or URL to code)
I expanded the HTTP2 client example code for a reproducer. Tested against the HTTP2 Server example https://github.com/netty/netty/compare/4.1...isaacrivriv:netty:test-bad-priority-frame

### Netty version
4.1.105

### JVM version (e.g. `java -version`)
openjdk version "17.0.7" 2023-04-18
IBM Semeru Runtime Open Edition 17.0.7.0 (build 17.0.7+7)
Eclipse OpenJ9 VM 17.0.7.0 (build openj9-0.38.0, JRE 17 Mac OS X amd64-64-Bit Compressed References 20230418_450 (JIT enabled, AOT enabled)
OpenJ9   - d57d05932
OMR      - 855813495
JCL      - 9d7a231edbc based on jdk-17.0.7+7)

### OS version (e.g. `uname -a`)
Darwin Kernel Version 23.0.0

Issue comments:
Is this issue (https://github.com/grpc/grpc-java/issues/10830) resolved by this PR? 
@njimenezotto That issue is completely different than this.

Pull request review comments:
Does this work correctly if there are two different messages in the `input` ByteBuffer waiting to be processed? In my minds eye it will skip them both.
I think you're right, if there's more data already buffered it'll skip all of it. I'm trying to think of an alternative but if the length of the payload doesn't match the actual payload not sure how we can skip the correct number of bytes to read the next frame correctly. Maybe adding length checks while cumulating the data in the ByteToMessageDecoder?
Instead of using a boolean flag we could instead do something like passing `input.readSlice(payloadLength)` into the `processPayloadState` method. That would require us to move the 
```
if (input.readableBytes() < payloadLength) {
    // Wait until the entire payload has been read.
     return;
}
```
check up out of the `processPayloadState` method to right before we slice and call it but tbh that feels more readable anyway.

Doing that has two nice properties and one negative: it's nice because it 'slices' exactly one frame from `input` which (1) makes it hard for `processPayloadState` to corrupt other frames and (2) makes consumption of the frame from `input` happen eagerly so you don't have to worry about skipping bytes on exception. Downside is that it does cost an extra allocation. On balance, I'd happily pay the allocation of the slice wrapper: it isn't a drop in the ocean imo.
That makes sense to me, but I think we'd hit some issues if the advertised `payloadLength` of the frame doesn't match the actual payload. In the use case I'm looking at, I see two cases where we could see issues
- __Advertised Payload Length greater than payload__: More data skipped than actually included and next frame would not be properly read.
- __Advertised Payload Length less than payload__: Less data skipped than actually included and data will be left over and next frame would not be properly read.
@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. 
In this case, the behavior I'm seeing with the response from a Netty HTTP2 server matches the RFC specification where in some cases even though a malformed frame comes in, it doesn't actually shut down the connection but just resets a stream and more streams can be accepted after that. [Section 6.3](https://datatracker.ietf.org/doc/html/rfc9113#name-priority) is an example

> A PRIORITY frame with a length other than 5 octets MUST be treated as a [stream error](https://datatracker.ietf.org/doc/html/rfc9113#StreamErrorHandler) ([Section 5.4.2](https://datatracker.ietf.org/doc/html/rfc9113#StreamErrorHandler)) of type [FRAME_SIZE_ERROR](https://datatracker.ietf.org/doc/html/rfc9113#FRAME_SIZE_ERROR).

As it is currently working if I send a priority frame with length 4, I correctly get a reset frame back with a frame size error (which is what the RFC states) but starting another stream with a valid request after that, it is not parsed correctly due to leftover data from the previous request.

The error really happens in those cases where the RFC states that a stream reset is used rather than a connection error because when a connection error happens everything gets cleaned up where in a stream reset there could still be leftover data
The most important part is: 

> 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))

It's ok to send `RST_STREAM` frame first. But then, how can we trust the frame if it has incorrect format? If we can not trust it, we don't know how many bytes to skip. It could happen that we received total garbage that just happen to start with "priority frame header". So, if we don't know how to continue, we should send GO_AWAY and close connection. 
The RFC states the following after that statement which confuses me

> 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)); **this includes any frame carrying a [field block](https://datatracker.ietf.org/doc/html/rfc9113#FieldBlock) ([Section 4.3](https://datatracker.ietf.org/doc/html/rfc9113#FieldBlock)) (that is, [HEADERS](https://datatracker.ietf.org/doc/html/rfc9113#HEADERS), [PUSH_PROMISE](https://datatracker.ietf.org/doc/html/rfc9113#PUSH_PROMISE), and [CONTINUATION](https://datatracker.ietf.org/doc/html/rfc9113#CONTINUATION)), a [SETTINGS](https://datatracker.ietf.org/doc/html/rfc9113#SETTINGS) frame, and any frame with a stream identifier of 0.**

So in my head the RFC states the specifics of what "altering the state of the connection" is in that sentence which this particular case doesn't seem to apply. By sending a priority frame to a specific stream (not 0) we can hit this issue and doesn't really fall under those categories IMO. 

I think we should skip the bytes if the payload length matches the actual frame payload and send a reset stream which matches the RFC and continue reading for the cases where a stream reset is required. I also agree closing the connection on cases where the payload length doesn't actually match the frame payload considering that we are receiving garbage in those cases but I'm making sure this isn't against the RFC since it states otherwise about keeping the connection active.

The other question would be, how would we know if the frame that came in actually matches the payload length? By the time Netty start's parsing H2 data we could have more messages queued in the buffer which we're not aware of. I'm thinking maybe a check on every decode to peek at the payload length and compare to the actual frame length? I'm not aware of a way to cleanly distinguish frames from each other TBH.
It's convenient to think of the H2 protocol as having (at least) two layers: the framing layer (the first 9 octets of a frame) and then the actual frame payloads themselves which will have some consistency requirements. What I'm suggesting leverages the fact that the length from the framing layer should be authoritative in terms of frame boundaries.
What to do if we detect corruption/inconsistencies in the payload (which could be that the payload is short or long, for example) will be case specific and gets into the details of what @idelpivnitskiy pointed to in his suggestions. This is actually a third advantage of what I'm suggestion: too few/extra bytes can signal corruption and if we slice we can detect that. For example, a RST_STREAM frame with size other than 4 octets should be a connection error (end of [6.4](https://datatracker.ietf.org/doc/html/rfc9113#section-6.4)). However in main if we received a RST_STREAM frame of size 5 (as determined by the frame header) where the last byte is meaningless instead of sending a connection error and potentially alerting the peer to a bug in their implementation we'll instead end up with a corrupt framing layer: we won't fail because we don't enforce the size and we won't consume that extra byte from the data stream so it will end up being interpreted as the first byte of the next frames header. In this specific example we could also just read the `payloadLength` field instead of checking that we have a buffer of size 4, but having a sliced buffer ensures decoders don't try to use more data than what the frame was advertised to contain via the header.
Yeah that's exactly the issue I was seeing, leftover data is parsed as part of the next frame when malformed frames like that are parsed. So with @idelpivnitskiy comments and this thread I'm thinking we can implement the slice mechanism but there needs to be more checks implemented so that the payload length matches the actual payload and for those cases where we don't find a match we can alert by sending a Go Away frame? I'm trying to see what a path forward would be for this.
See my comment above but the TL;DR is that the length field from the frame header is authoritative on size of a frames payload. What to do if that payload size is too short or too long when decoding the frame is frame type specific and is described as part of the individual frame definitions.
That sounds right: whether to send a GOAWAY or a RST_STREAM is frame type specific and described as part of the frame definitions (usually at the ends). I don't think you have to implement all those checks as part of this PR if you don't want to: they could be done as a follow up. Finding this and making sure we keep a correct framing layer is already a huge step forward from where we are now.
Taking a closer look it seems most of them (if not all) already verify the frame length and do the right thing wrt RST vs GOAWAY but its done as part of the `processHeaderState` state so we haven't been able to slice before the exception is thrown, and we may not even have that many bytes available, so we'd need to move the verification to a new method and perform it right before we `processPayloadState(ctx, input.slice(..), ..)`.
All of these methods still work despite passing in a value with different meaning only because the writer index now matches the payload length. Now that you've sliced the buffer really we don't need to pass in the end length as each method could derive it from the passed in slice itself.
fwiw, now that you've sliced the buffer I don't think you need to do this re-ordering.
```suggestion
            // Stream error found, reset to reading headers once again for next frame coming in
```
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.
We should be able to simplify the management of this variable: we switch back to `readingHeaders = true` as soon as we consume the frame payload, so we can set it right after we `.readSlice(..)` and before we attempt to verify and process the payload and remove the mutations elsewhere.
True, I'll update the method signatures to make this cleaner.
Agree, I forgot that we aren't skipping the bytes anymore like the original solution if this happens. I'll move this verification after slicing
This makes sense, I'll simplify with this in mind
I think these can be even simpler: 
```suggestion
        int dataLength = lengthWithoutTrailingPadding(payload.readableBytes(), padding);
```
Do you think we could just adjust the writer index on `payload`?
Agree, I'll make the changes to all of these 
So instead of slicing the data buffer just move the writer index of the payload to dataLength?
Yes: I think we sliced the payload previously because we didn't want to leak the parent buffer but since we've already sliced it there shouldn't be any harm in sharing that. At a minimum, I expect padding to be rare so if there isn't any padding to slice out we can certainly just send `payload` along.

And to be more precise, I think we should do `payload.writeIndex(payload.readerIndex() + dataLength)`. `payload.readerIndex()` should always be 0 now, but it will make the code more resilient to future changes.
Ok cool I'll make that change and saw the payload being further sliced while reading a Go Away frame so I'm updating that as well
@bryce-anderson @idelpivnitskiy you are happy with what we have now ?
@normanmaurer, I am. This should prevent frame stream corruption for problems that result in a stream error at the validation phase.

One behavior change that I think is worth point out is that we'll now aggregate the data for the next frame before doing any of the per-frame validations other than max frame length. I think this is safe because we've already agreed to accept frames that big such as data and headers frames but it's worth a thought.
Consider naming it `verifyFrameState` because at the time we invoke this method, we already consumed the whole frame, not just its header. This will also align with other `verify*Frame()` methods it's calling. 
Consider adding an assertion (ideally with a comment) that `in.readableBytes() == payloadLength`. In case in the future someone invokes this method from an incorrect state, it will rise an error.
Consider adding a comment for future devs why we read the full header before throwing an excepting on case `payloadLength > maxFrameSize`
@isaacrivriv thanks for pointing to the continuation of that sentence. That makes sense, for the current case we don't need to close the entire connection. Great catch!
I agree, renaming it makes more sense I'll make this change
Sounds good with me, I'll make this change
I'm looking at this again and since it's a connectionError which closes the connection I don't think we have to read the full headers before throwing the exception so I'm undoing these changes

Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java
index dc0857ec6b8..dae497d9bb5 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java
@@ -143,24 +143,25 @@ public void readFrame(ChannelHandlerContext ctx, ByteBuf input, Http2FrameListen
         }
         try {
             do {
-                if (readingHeaders) {
-                    processHeaderState(input);
-                    if (readingHeaders) {
-                        // Wait until the entire header has arrived.
-                        return;
-                    }
+                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.
-                processPayloadState(ctx, input, listener);
-                if (!readingHeaders) {
-                    // Wait until the entire payload has arrived.
+                // 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);
@@ -174,13 +175,13 @@ public void readFrame(ChannelHandlerContext ctx, ByteBuf input, Http2FrameListen
         }
     }
 
-    private void processHeaderState(ByteBuf in) throws Http2Exception {
+    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 frame header has been read.
-            return;
+            // Wait until the entire framing section has been read.
+            return false;
         }
-
-        // Read the header and prepare the unmarshaller to read the frame.
         payloadLength = in.readUnsignedMedium();
         if (payloadLength > maxFrameSize) {
             throw connectionError(FRAME_SIZE_ERROR, "Frame length: %d exceeds maximum: %d", payloadLength,
@@ -189,10 +190,11 @@ private void processHeaderState(ByteBuf in) throws Http2Exception {
         frameType = in.readByte();
         flags = new Http2Flags(in.readUnsignedByte());
         streamId = readUnsignedInt(in);
-
-        // We have consumed the data, next time we read we will be expecting to read the frame payload.
         readingHeaders = false;
+        return true;
+    }
 
+    private void verifyFrameState() throws Http2Exception {
         switch (frameType) {
             case DATA:
                 verifyDataFrame();
@@ -233,24 +235,16 @@ private void processHeaderState(ByteBuf in) throws Http2Exception {
 
     private void processPayloadState(ChannelHandlerContext ctx, ByteBuf in, Http2FrameListener listener)
                     throws Http2Exception {
-        if (in.readableBytes() < payloadLength) {
-            // Wait until the entire payload has been read.
-            return;
-        }
-
-        // Only process up to payloadLength bytes.
-        int payloadEndIndex = in.readerIndex() + payloadLength;
-
-        // We have consumed the data, next time we read we will be expecting to read a frame header.
-        readingHeaders = true;
-
+        // When this method is called, we ensure that the payload buffer passed in
+        // matches what we expect to be reading for payloadLength
+        assert in.readableBytes() == payloadLength;
         // Read the payload and fire the frame event to the listener.
         switch (frameType) {
             case DATA:
-                readDataFrame(ctx, in, payloadEndIndex, listener);
+                readDataFrame(ctx, in, listener);
                 break;
             case HEADERS:
-                readHeadersFrame(ctx, in, payloadEndIndex, listener);
+                readHeadersFrame(ctx, in, listener);
                 break;
             case PRIORITY:
                 readPriorityFrame(ctx, in, listener);
@@ -262,25 +256,24 @@ private void processPayloadState(ChannelHandlerContext ctx, ByteBuf in, Http2Fra
                 readSettingsFrame(ctx, in, listener);
                 break;
             case PUSH_PROMISE:
-                readPushPromiseFrame(ctx, in, payloadEndIndex, listener);
+                readPushPromiseFrame(ctx, in, listener);
                 break;
             case PING:
                 readPingFrame(ctx, in.readLong(), listener);
                 break;
             case GO_AWAY:
-                readGoAwayFrame(ctx, in, payloadEndIndex, listener);
+                readGoAwayFrame(ctx, in, listener);
                 break;
             case WINDOW_UPDATE:
                 readWindowUpdateFrame(ctx, in, listener);
                 break;
             case CONTINUATION:
-                readContinuationFrame(in, payloadEndIndex, listener);
+                readContinuationFrame(in, listener);
                 break;
             default:
-                readUnknownFrame(ctx, in, payloadEndIndex, listener);
+                readUnknownFrame(ctx, in, listener);
                 break;
         }
-        in.readerIndex(payloadEndIndex);
     }
 
     private void verifyDataFrame() throws Http2Exception {
@@ -402,20 +395,20 @@ private void verifyUnknownFrame() throws Http2Exception {
         verifyNotProcessingHeaders();
     }
 
-    private void readDataFrame(ChannelHandlerContext ctx, ByteBuf payload, int payloadEndIndex,
+    private void readDataFrame(ChannelHandlerContext ctx, ByteBuf payload,
             Http2FrameListener listener) throws Http2Exception {
         int padding = readPadding(payload);
         verifyPadding(padding);
 
         // Determine how much data there is to read by removing the trailing
         // padding.
-        int dataLength = lengthWithoutTrailingPadding(payloadEndIndex - payload.readerIndex(), padding);
+        int dataLength = lengthWithoutTrailingPadding(payload.readableBytes(), padding);
 
-        ByteBuf data = payload.readSlice(dataLength);
-        listener.onDataRead(ctx, streamId, data, padding, flags.endOfStream());
+        payload.writerIndex(payload.readerIndex() + dataLength);
+        listener.onDataRead(ctx, streamId, payload, padding, flags.endOfStream());
     }
 
-    private void readHeadersFrame(final ChannelHandlerContext ctx, ByteBuf payload, int payloadEndIndex,
+    private void readHeadersFrame(final ChannelHandlerContext ctx, ByteBuf payload,
             Http2FrameListener listener) throws Http2Exception {
         final int headersStreamId = streamId;
         final Http2Flags headersFlags = flags;
@@ -432,7 +425,7 @@ private void readHeadersFrame(final ChannelHandlerContext ctx, ByteBuf payload,
                 throw streamError(streamId, PROTOCOL_ERROR, "A stream cannot depend on itself.");
             }
             final short weight = (short) (payload.readUnsignedByte() + 1);
-            final int lenToRead = lengthWithoutTrailingPadding(payloadEndIndex - payload.readerIndex(), padding);
+            final int lenToRead = lengthWithoutTrailingPadding(payload.readableBytes(), padding);
 
             // Create a handler that invokes the listener when the header block is complete.
             headersContinuation = new HeadersContinuation() {
@@ -480,7 +473,7 @@ public void processFragment(boolean endOfHeaders, ByteBuf fragment, int len,
         };
 
         // Process the initial fragment, invoking the listener's callback if end of headers.
-        int len = lengthWithoutTrailingPadding(payloadEndIndex - payload.readerIndex(), padding);
+        int len = lengthWithoutTrailingPadding(payload.readableBytes(), padding);
         headersContinuation.processFragment(flags.endOfHeaders(), payload, len, listener);
         resetHeadersContinuationIfEnd(flags.endOfHeaders());
     }
@@ -533,7 +526,7 @@ private void readSettingsFrame(ChannelHandlerContext ctx, ByteBuf payload,
         }
     }
 
-    private void readPushPromiseFrame(final ChannelHandlerContext ctx, ByteBuf payload, int payloadEndIndex,
+    private void readPushPromiseFrame(final ChannelHandlerContext ctx, ByteBuf payload,
             Http2FrameListener listener) throws Http2Exception {
         final int pushPromiseStreamId = streamId;
         final int padding = readPadding(payload);
@@ -559,7 +552,7 @@ public void processFragment(boolean endOfHeaders, ByteBuf fragment, int len,
         };
 
         // Process the initial fragment, invoking the listener's callback if end of headers.
-        int len = lengthWithoutTrailingPadding(payloadEndIndex - payload.readerIndex(), padding);
+        int len = lengthWithoutTrailingPadding(payload.readableBytes(), padding);
         headersContinuation.processFragment(flags.endOfHeaders(), payload, len, listener);
         resetHeadersContinuationIfEnd(flags.endOfHeaders());
     }
@@ -573,12 +566,11 @@ private void readPingFrame(ChannelHandlerContext ctx, long data,
         }
     }
 
-    private static void readGoAwayFrame(ChannelHandlerContext ctx, ByteBuf payload, int payloadEndIndex,
+    private void readGoAwayFrame(ChannelHandlerContext ctx, ByteBuf payload,
             Http2FrameListener listener) throws Http2Exception {
         int lastStreamId = readUnsignedInt(payload);
         long errorCode = payload.readUnsignedInt();
-        ByteBuf debugData = payload.readSlice(payloadEndIndex - payload.readerIndex());
-        listener.onGoAwayRead(ctx, lastStreamId, errorCode, debugData);
+        listener.onGoAwayRead(ctx, lastStreamId, errorCode, payload);
     }
 
     private void readWindowUpdateFrame(ChannelHandlerContext ctx, ByteBuf payload,
@@ -591,17 +583,16 @@ private void readWindowUpdateFrame(ChannelHandlerContext ctx, ByteBuf payload,
         listener.onWindowUpdateRead(ctx, streamId, windowSizeIncrement);
     }
 
-    private void readContinuationFrame(ByteBuf payload, int payloadEndIndex, Http2FrameListener listener)
+    private void readContinuationFrame(ByteBuf payload, Http2FrameListener listener)
             throws Http2Exception {
         // Process the initial fragment, invoking the listener's callback if end of headers.
         headersContinuation.processFragment(flags.endOfHeaders(), payload,
-                payloadEndIndex - payload.readerIndex(), listener);
+                payloadLength, listener);
         resetHeadersContinuationIfEnd(flags.endOfHeaders());
     }
 
     private void readUnknownFrame(ChannelHandlerContext ctx, ByteBuf payload,
-            int payloadEndIndex, Http2FrameListener listener) throws Http2Exception {
-        payload = payload.readSlice(payloadEndIndex - payload.readerIndex());
+            Http2FrameListener listener) throws Http2Exception {
         listener.onUnknownFrame(ctx, frameType, streamId, flags, payload);
     }
 


Test patch:
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameReaderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameReaderTest.java
index bfedf0ac619..8ba309cc617 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameReaderTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2FrameReaderTest.java
@@ -386,6 +386,45 @@ public void execute() throws Throwable {
         }
     }
 
+    @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 {