From ee3b9a5f7b1829e1095fdbbccb5490949ac3e94e Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Mon, 26 Oct 2020 19:11:49 +0530 Subject: [PATCH] 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 --- .../codec/http2/AbstractHttp2StreamChannel.java | 6 +++--- .../codec/http2/DefaultHttp2ConnectionDecoder.java | 4 ---- .../codec/http2/DefaultHttp2FrameWriter.java | 2 +- .../handler/codec/http2/HpackDynamicTable.java | 14 ++++++++------ .../handler/codec/http2/ReadOnlyHttp2Headers.java | 8 +++++--- 5 files changed, 17 insertions(+), 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 index 3252324d9d..7ee95ef618 100644 --- 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 @@ -147,8 +147,8 @@ abstract class AbstractHttp2StreamChannel extends DefaultAttributeMap implements 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; @@ -258,7 +258,7 @@ abstract class AbstractHttp2StreamChannel extends DefaultAttributeMap implements 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 index 3fa9bbf681..1bbaf632b5 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 @@ -507,10 +507,6 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { 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 index 4f1225eb70..e13a631c31 100644 --- 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 @@ -219,7 +219,7 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize 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 index 7377c9aaa9..4a712eed25 100644 --- 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 @@ final class HpackDynamicTable { // 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 index 8fa9a9c85b..56632c5ac1 100644 --- 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 @@ public final class ReadOnlyHttp2Headers implements Http2Headers { 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();