Fix possible NPEs and IndexOutOfBoundsExceptions in HTTP/2 Codec (#10640

) Motivation: There are possible NPEs and IndexOutOfBoundsExceptions in HTTP/2 code. Modification: Fixed possible NPEs and IOOBEs Result: Better code
netty · Oct 26, 2020 · 1bcfd7e · 1bcfd7e
1 parent 51db4c9
commit 1bcfd7e
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 17 deletions.
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2StreamChannel.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2StreamChannel.java
@@ -137,8 +137,8 @@ private enum ReadStatus {
         REQUESTED
     }
 
-    private final AbstractHttp2StreamChannel.Http2StreamChannelConfig config = new Http2StreamChannelConfig(this);
-    private final AbstractHttp2StreamChannel.Http2ChannelUnsafe unsafe = new Http2ChannelUnsafe();
+    private final Http2StreamChannelConfig config = new Http2StreamChannelConfig(this);
+    private final Http2ChannelUnsafe unsafe = new Http2ChannelUnsafe();
     private final ChannelId channelId;
     private final ChannelPipeline pipeline;
     private final DefaultHttp2FrameStream stream;
@@ -248,7 +248,7 @@ private void setUnwritable(boolean invokeLater) {
             final int oldValue = unwritable;
             final int newValue = oldValue | 1;
             if (UNWRITABLE_UPDATER.compareAndSet(this, oldValue, newValue)) {
-                if (oldValue == 0 && newValue != 0) {
+                if (oldValue == 0) {
                     fireChannelWritabilityChanged(invokeLater);
                 }
                 break;

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
@@ -498,10 +498,6 @@ public void onPushPromiseRead(ChannelHandlerContext ctx, int streamId, int promi
                 return;
             }
 
-            if (parentStream == null) {
-                throw connectionError(PROTOCOL_ERROR, "Stream %d does not exist", streamId);
-            }
-
             switch (parentStream.state()) {
               case OPEN:
               case HALF_CLOSED_LOCAL:

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java
@@ -218,7 +218,7 @@ public ChannelFuture writeData(ChannelHandlerContext ctx, int streamId, ByteBuf
                     ctx.write(frameHeader2, promiseAggregator.newPromise());
 
                     // Write the payload.
-                    if (frameDataBytes != 0) {
+                    if (frameDataBytes != 0 && data != null) { // Make sure Data is not null
                         if (remainingData == 0) {
                             ByteBuf lastFrame = data.readSlice(frameDataBytes);
                             data = null;

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDynamicTable.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackDynamicTable.java
@@ -183,12 +183,14 @@ public void setCapacity(long capacity) {
 
         // initially length will be 0 so there will be no copy
         int len = length();
-        int cursor = tail;
-        for (int i = 0; i < len; i++) {
-            HpackHeaderField entry = hpackHeaderFields[cursor++];
-            tmp[i] = entry;
-            if (cursor == hpackHeaderFields.length) {
-                cursor = 0;
+        if (hpackHeaderFields != null) {
+            int cursor = tail;
+            for (int i = 0; i < len; i++) {
+                HpackHeaderField entry = hpackHeaderFields[cursor++];
+                tmp[i] = entry;
+                if (cursor == hpackHeaderFields.length) {
+                    cursor = 0;
+                }
             }
         }
 

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/ReadOnlyHttp2Headers.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/ReadOnlyHttp2Headers.java
@@ -823,12 +823,14 @@ private void calculateNext() {
             for (; i < current.length; i += 2) {
                 AsciiString roName = current[i];
                 if (roName.hashCode() == nameHash && roName.contentEqualsIgnoreCase(name)) {
-                    next = current[i + 1];
-                    i += 2;
+                    if (i + 1 < current.length) {
+                        next = current[i + 1];
+                        i += 2;
+                    }
                     return;
                 }
             }
-            if (i >= current.length && current == pseudoHeaders) {
+            if (current == pseudoHeaders) {
                 i = 0;
                 current = otherHeaders;
                 calculateNext();