From df41be6fc84adc116bc46ff703c4b9d7248fa099 Mon Sep 17 00:00:00 2001
From: Scott Mitchell <scott_mitchell@apple.com>
Date: Sat, 25 Jun 2016 13:34:10 -0700
Subject: [PATCH] HTTP/2 Decoder validate that GOAWAY lastStreamId doesn't
 increase

Motivation:
The HTTP/2 RFC states in https://tools.ietf.org/html/rfc7540#section-6.8 that Endpoints MUST NOT increase the value they send in the last stream identifier however we don't enforce this when decoding GOAWAY frames.

Modifications:
- Throw a connection error if the peer attempts to increase the lastStreamId in a GOAWAY frame

Result:
RFC is more strictly enforced.
---
 .../handler/codec/http2/DefaultHttp2ConnectionDecoder.java | 4 ++++
 .../codec/http2/DefaultHttp2ConnectionDecoderTest.java     | 7 +++++++
 2 files changed, 11 insertions(+)

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 bd9d2373b0..731a960177 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
@@ -182,6 +182,10 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
 
     void onGoAwayRead0(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData)
             throws Http2Exception {
+        if (connection.goAwayReceived() && connection.local().lastStreamKnownByPeer() < lastStreamId) {
+            throw connectionError(PROTOCOL_ERROR, "lastStreamId MUST NOT increase. Current value: %d new value: %d",
+                    connection.local().lastStreamKnownByPeer(), lastStreamId);
+        }
         listener.onGoAwayRead(ctx, lastStreamId, errorCode, debugData);
         connection.goAwayReceived(lastStreamId, errorCode, debugData);
     }
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
index cfca1f8437..50b74cc878 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
@@ -628,6 +628,13 @@ public class DefaultHttp2ConnectionDecoderTest {
         verify(listener).onRstStreamRead(eq(ctx), anyInt(), anyLong());
     }
 
+    @Test(expected = Http2Exception.class)
+    public void goawayIncreasedLastStreamIdShouldThrow() throws Exception {
+        when(local.lastStreamKnownByPeer()).thenReturn(1);
+        when(connection.goAwayReceived()).thenReturn(true);
+        decode().onGoAwayRead(ctx, 3, 2L, EMPTY_BUFFER);
+    }
+
     @Test(expected = Http2Exception.class)
     public void rstStreamReadForUnknownStreamShouldThrow() throws Exception {
         when(connection.streamMayHaveExisted(STREAM_ID)).thenReturn(false);