fix CVE-2021-21295 CVE-2021-21409

This commit is contained in:
wangxiao65 2021-04-06 15:28:11 +08:00
parent 55d1f70ec2
commit df460aec6e
7 changed files with 2861 additions and 1 deletions

240
CVE-2021-21295-pre1.patch Normal file
View File

@ -0,0 +1,240 @@
From bcb62be62bd989c0292e0f8e22a51127907cefdc Mon Sep 17 00:00:00 2001
From: Bennett Lynch <bennett.lynch@gmail.com>
Date: Thu, 11 Jun 2020 22:39:10 -0700
Subject: [PATCH] Consolidate HttpObjectDecoder default values into
constants (#10344)
Motivation
HttpObjectDecoder and its associated classes make frequent use of
default values for maxInitialLineLength, maxHeaderSize, maxChunkSize,
etc. Today, these defaults are defined in-line in constructors and
duplicated across many classes. This repetition is more prone to error
and inconsistencies.
Furthermore, due to the current lack of builder support, if a user wants
to change just one of these values (e.g., maxHeaderSize), they are also
required to know and repeat the other default values (e.g.,
maxInitialLineLength and maxChunkSize).
The primary motivation for this change is as we are considering adding
another constructor parameter (for multiple content length behavior),
appending this parameter may require some users to have prior knowledge
of the default initialBufferSize, and it would be cleaner to allow them
to reference the default constant.
Modifications
* Consolidate the HttpObjectDecoder default values into public constants
* Reference these constants where possible
Result
No functional change. Additional telescoping constructors will be easier
and safer to write. Users may have an easier experience changing single
parameters.
---
.../netty/handler/codec/http/HttpClientCodec.java | 6 +++++-
.../handler/codec/http/HttpObjectDecoder.java | 15 ++++++++++++---
.../handler/codec/http/HttpRequestDecoder.java | 7 ++++---
.../handler/codec/http/HttpResponseDecoder.java | 7 ++++---
.../netty/handler/codec/http/HttpServerCodec.java | 6 +++++-
.../io/netty/handler/codec/rtsp/RtspDecoder.java | 10 ----------
.../handler/codec/rtsp/RtspObjectDecoder.java | 4 +++-
7 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java
index da4c440466..a832bfdff3 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java
@@ -28,6 +28,10 @@ import java.util.List;
import java.util.Queue;
import java.util.concurrent.atomic.AtomicLong;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_CHUNK_SIZE;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_HEADER_SIZE;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_INITIAL_LINE_LENGTH;
+
/**
* A combination of {@link HttpRequestEncoder} and {@link HttpResponseDecoder}
* which enables easier client side HTTP implementation. {@link HttpClientCodec}
@@ -61,7 +65,7 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler<HttpResp
* {@code maxChunkSize (8192)}).
*/
public HttpClientCodec() {
- this(4096, 8192, 8192, false);
+ this(DEFAULT_MAX_INITIAL_LINE_LENGTH, DEFAULT_MAX_HEADER_SIZE, DEFAULT_MAX_CHUNK_SIZE, false);
}
/**
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
index e39ed9e48a..d4caf29c6d 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
@@ -100,6 +100,13 @@ import java.util.List;
* implement all abstract methods properly.
*/
public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
+ public static final int DEFAULT_MAX_INITIAL_LINE_LENGTH = 4096;
+ public static final int DEFAULT_MAX_HEADER_SIZE = 8192;
+ public static final boolean DEFAULT_CHUNKED_SUPPORTED = true;
+ public static final int DEFAULT_MAX_CHUNK_SIZE = 8192;
+ public static final boolean DEFAULT_VALIDATE_HEADERS = true;
+ public static final int DEFAULT_INITIAL_BUFFER_SIZE = 128;
+
private static final String EMPTY_VALUE = "";
private final int maxChunkSize;
@@ -145,7 +152,8 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
* {@code maxChunkSize (8192)}.
*/
protected HttpObjectDecoder() {
- this(4096, 8192, 8192, true);
+ this(DEFAULT_MAX_INITIAL_LINE_LENGTH, DEFAULT_MAX_HEADER_SIZE, DEFAULT_MAX_CHUNK_SIZE,
+ DEFAULT_CHUNKED_SUPPORTED);
}
/**
@@ -153,7 +161,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
*/
protected HttpObjectDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean chunkedSupported) {
- this(maxInitialLineLength, maxHeaderSize, maxChunkSize, chunkedSupported, true);
+ this(maxInitialLineLength, maxHeaderSize, maxChunkSize, chunkedSupported, DEFAULT_VALIDATE_HEADERS);
}
/**
@@ -162,7 +170,8 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
protected HttpObjectDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize,
boolean chunkedSupported, boolean validateHeaders) {
- this(maxInitialLineLength, maxHeaderSize, maxChunkSize, chunkedSupported, validateHeaders, 128);
+ this(maxInitialLineLength, maxHeaderSize, maxChunkSize, chunkedSupported, validateHeaders,
+ DEFAULT_INITIAL_BUFFER_SIZE);
}
protected HttpObjectDecoder(
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java
index 24252c7358..70c1db5540 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java
@@ -67,18 +67,19 @@ public class HttpRequestDecoder extends HttpObjectDecoder {
*/
public HttpRequestDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize) {
- super(maxInitialLineLength, maxHeaderSize, maxChunkSize, true);
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED);
}
public HttpRequestDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders) {
- super(maxInitialLineLength, maxHeaderSize, maxChunkSize, true, validateHeaders);
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED, validateHeaders);
}
public HttpRequestDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders,
int initialBufferSize) {
- super(maxInitialLineLength, maxHeaderSize, maxChunkSize, true, validateHeaders, initialBufferSize);
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED, validateHeaders,
+ initialBufferSize);
}
@Override
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java
index c6351c47bf..39d4d6a5ad 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java
@@ -98,18 +98,19 @@ public class HttpResponseDecoder extends HttpObjectDecoder {
*/
public HttpResponseDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize) {
- super(maxInitialLineLength, maxHeaderSize, maxChunkSize, true);
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED);
}
public HttpResponseDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders) {
- super(maxInitialLineLength, maxHeaderSize, maxChunkSize, true, validateHeaders);
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED, validateHeaders);
}
public HttpResponseDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders,
int initialBufferSize) {
- super(maxInitialLineLength, maxHeaderSize, maxChunkSize, true, validateHeaders, initialBufferSize);
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED, validateHeaders,
+ initialBufferSize);
}
@Override
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java
index a009df1145..8ae6295cf7 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java
@@ -23,6 +23,10 @@ import java.util.ArrayDeque;
import java.util.List;
import java.util.Queue;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_CHUNK_SIZE;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_HEADER_SIZE;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_INITIAL_LINE_LENGTH;
+
/**
* A combination of {@link HttpRequestDecoder} and {@link HttpResponseEncoder}
* which enables easier server side HTTP implementation.
@@ -41,7 +45,7 @@ public final class HttpServerCodec extends CombinedChannelDuplexHandler<HttpRequ
* {@code maxChunkSize (8192)}).
*/
public HttpServerCodec() {
- this(4096, 8192, 8192);
+ this(DEFAULT_MAX_INITIAL_LINE_LENGTH, DEFAULT_MAX_HEADER_SIZE, DEFAULT_MAX_CHUNK_SIZE);
}
/**
diff --git a/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspDecoder.java
index acc028978f..b2a353d298 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspDecoder.java
@@ -71,16 +71,6 @@ public class RtspDecoder extends HttpObjectDecoder {
*/
private static final Pattern versionPattern = Pattern.compile("RTSP/\\d\\.\\d");
- /**
- * Constant for default max initial line length.
- */
- public static final int DEFAULT_MAX_INITIAL_LINE_LENGTH = 4096;
-
- /**
- * Constant for default max header size.
- */
- public static final int DEFAULT_MAX_HEADER_SIZE = 8192;
-
/**
* Constant for default max content length.
*/
diff --git a/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspObjectDecoder.java
index e52c0ce51e..69b8ebb1f0 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspObjectDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspObjectDecoder.java
@@ -20,6 +20,8 @@ import io.netty.handler.codec.TooLongFrameException;
import io.netty.handler.codec.http.HttpMessage;
import io.netty.handler.codec.http.HttpObjectDecoder;
+import static io.netty.handler.codec.rtsp.RtspDecoder.DEFAULT_MAX_CONTENT_LENGTH;
+
/**
* Decodes {@link ByteBuf}s into RTSP messages represented in
* {@link HttpMessage}s.
@@ -59,7 +61,7 @@ public abstract class RtspObjectDecoder extends HttpObjectDecoder {
* {@code maxContentLength (8192)}.
*/
protected RtspObjectDecoder() {
- this(4096, 8192, 8192);
+ this(DEFAULT_MAX_INITIAL_LINE_LENGTH, DEFAULT_MAX_HEADER_SIZE, DEFAULT_MAX_CONTENT_LENGTH);
}
/**
--
2.23.0

676
CVE-2021-21295-pre2.patch Normal file
View File

@ -0,0 +1,676 @@
From 4c64c98f348131e0792ba4a92ce3d0003237d56a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E7=94=B0=E6=AC=A7?= <thinkerou@gmail.com>
Date: Mon, 4 Feb 2019 22:55:07 +0800
Subject: [PATCH] use checkPositive/checkPositiveOrZero (#8835)
Motivation:
We can replace some "hand-rolled" integer checks with our own static utility method to simplify the code.
Modifications:
Use methods provided by `ObjectUtil`.
Result:
Cleaner code and less duplication
---
.../handler/codec/dns/AbstractDnsRecord.java | 5 ++---
.../codec/http/DefaultHttpHeaders.java | 3 +--
.../handler/codec/http/HttpObjectDecoder.java | 21 ++++++-------------
.../codec/http/HttpResponseStatus.java | 7 +++----
.../netty/handler/codec/http/HttpVersion.java | 10 ++++-----
.../multipart/AbstractMemoryHttpData.java | 3 +--
.../codec/spdy/DefaultSpdyGoAwayFrame.java | 7 +++----
.../codec/spdy/DefaultSpdyStreamFrame.java | 7 +++----
.../codec/spdy/DefaultSpdySynReplyFrame.java | 3 +--
.../codec/spdy/DefaultSpdySynStreamFrame.java | 8 +++----
.../spdy/DefaultSpdyWindowUpdateFrame.java | 14 +++++--------
.../handler/codec/spdy/SpdyFrameDecoder.java | 7 +++----
.../handler/codec/spdy/SpdyHttpDecoder.java | 6 ++----
.../codec/spdy/SpdySessionHandler.java | 19 ++++++++---------
.../http2/DefaultHttp2ConnectionEncoder.java | 5 ++---
.../codec/http2/DefaultHttp2FrameWriter.java | 14 +++++--------
.../codec/http2/DefaultHttp2GoAwayFrame.java | 6 +++---
.../DefaultHttp2LocalFlowController.java | 5 ++---
.../DefaultHttp2RemoteFlowController.java | 5 ++---
.../DelegatingDecompressorFrameListener.java | 5 ++---
.../http2/UniformStreamByteDistributor.java | 5 ++---
.../WeightedFairQueueByteDistributor.java | 11 +++++-----
.../binary/AbstractBinaryMemcacheDecoder.java | 6 +++---
.../codec/stomp/StompSubframeDecoder.java | 13 +++---------
24 files changed, 75 insertions(+), 120 deletions(-)
diff --git a/codec-dns/src/main/java/io/netty/handler/codec/dns/AbstractDnsRecord.java b/codec-dns/src/main/java/io/netty/handler/codec/dns/AbstractDnsRecord.java
index 28b92c27f9..2ba6e573a7 100644
--- a/codec-dns/src/main/java/io/netty/handler/codec/dns/AbstractDnsRecord.java
+++ b/codec-dns/src/main/java/io/netty/handler/codec/dns/AbstractDnsRecord.java
@@ -21,6 +21,7 @@ import io.netty.util.internal.UnstableApi;
import java.net.IDN;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
/**
* A skeletal implementation of {@link DnsRecord}.
@@ -62,9 +63,7 @@ public abstract class AbstractDnsRecord implements DnsRecord {
* @param timeToLive the TTL value of the record
*/
protected AbstractDnsRecord(String name, DnsRecordType type, int dnsClass, long timeToLive) {
- if (timeToLive < 0) {
- throw new IllegalArgumentException("timeToLive: " + timeToLive + " (expected: >= 0)");
- }
+ checkPositiveOrZero(timeToLive, "timeToLive");
// Convert to ASCII which will also check that the length is not too big.
// See:
// - https://github.com/netty/netty/issues/4937
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java
index 6204f3ea7f..d18f196e8f 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java
@@ -341,8 +341,7 @@ public class DefaultHttpHeaders extends HttpHeaders {
default:
// Check to see if the character is not an ASCII character, or invalid
if (value < 0) {
- throw new IllegalArgumentException("a header name cannot contain non-ASCII character: " +
- value);
+ throw new IllegalArgumentException("a header name cannot contain non-ASCII character: " + value);
}
}
}
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
index d4caf29c6d..ed7caa7801 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
@@ -15,6 +15,8 @@
*/
package io.netty.handler.codec.http;
+import static io.netty.util.internal.ObjectUtil.checkPositive;
+
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelHandlerContext;
@@ -177,21 +179,10 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
protected HttpObjectDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize,
boolean chunkedSupported, boolean validateHeaders, int initialBufferSize) {
- if (maxInitialLineLength <= 0) {
- throw new IllegalArgumentException(
- "maxInitialLineLength must be a positive integer: " +
- maxInitialLineLength);
- }
- if (maxHeaderSize <= 0) {
- throw new IllegalArgumentException(
- "maxHeaderSize must be a positive integer: " +
- maxHeaderSize);
- }
- if (maxChunkSize <= 0) {
- throw new IllegalArgumentException(
- "maxChunkSize must be a positive integer: " +
- maxChunkSize);
- }
+ checkPositive(maxInitialLineLength, "maxInitialLineLength");
+ checkPositive(maxHeaderSize, "maxHeaderSize");
+ checkPositive(maxChunkSize, "maxChunkSize");
+
AppendableCharSequence seq = new AppendableCharSequence(initialBufferSize);
lineParser = new LineParser(seq, maxInitialLineLength);
headerParser = new HeaderParser(seq, maxHeaderSize);
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java
index 026866ebcc..9f24e0d3cc 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java
@@ -22,6 +22,8 @@ import io.netty.util.AsciiString;
import io.netty.util.ByteProcessor;
import io.netty.util.CharsetUtil;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
+
/**
* The response code and its description of HTTP or its derived protocols, such as
* <a href="http://en.wikipedia.org/wiki/Real_Time_Streaming_Protocol">RTSP</a> and
@@ -577,10 +579,7 @@ public class HttpResponseStatus implements Comparable<HttpResponseStatus> {
}
private HttpResponseStatus(int code, String reasonPhrase, boolean bytes) {
- if (code < 0) {
- throw new IllegalArgumentException(
- "code: " + code + " (expected: 0+)");
- }
+ checkPositiveOrZero(code, "code");
if (reasonPhrase == null) {
throw new NullPointerException("reasonPhrase");
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java
index a643f42458..7ba40eed90 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java
@@ -15,6 +15,8 @@
*/
package io.netty.handler.codec.http;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
+
import io.netty.buffer.ByteBuf;
import io.netty.util.CharsetUtil;
@@ -165,12 +167,8 @@ public class HttpVersion implements Comparable<HttpVersion> {
}
}
- if (majorVersion < 0) {
- throw new IllegalArgumentException("negative majorVersion");
- }
- if (minorVersion < 0) {
- throw new IllegalArgumentException("negative minorVersion");
- }
+ checkPositiveOrZero(majorVersion, "majorVersion");
+ checkPositiveOrZero(minorVersion, "minorVersion");
this.protocolName = protocolName;
this.majorVersion = majorVersion;
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java
index 31aa9ce64b..4cb7e567b2 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java
@@ -128,8 +128,7 @@ public abstract class AbstractMemoryHttpData extends AbstractHttpData {
}
long newsize = file.length();
if (newsize > Integer.MAX_VALUE) {
- throw new IllegalArgumentException(
- "File too big to be loaded in memory");
+ throw new IllegalArgumentException("File too big to be loaded in memory");
}
checkSize(newsize);
FileInputStream inputStream = new FileInputStream(file);
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyGoAwayFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyGoAwayFrame.java
index 4d88875a6e..79c21f2404 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyGoAwayFrame.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyGoAwayFrame.java
@@ -15,6 +15,8 @@
*/
package io.netty.handler.codec.spdy;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
+
import io.netty.util.internal.StringUtil;
/**
@@ -62,10 +64,7 @@ public class DefaultSpdyGoAwayFrame implements SpdyGoAwayFrame {
@Override
public SpdyGoAwayFrame setLastGoodStreamId(int lastGoodStreamId) {
- if (lastGoodStreamId < 0) {
- throw new IllegalArgumentException("Last-good-stream-ID"
- + " cannot be negative: " + lastGoodStreamId);
- }
+ checkPositiveOrZero(lastGoodStreamId, "lastGoodStreamId");
this.lastGoodStreamId = lastGoodStreamId;
return this;
}
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyStreamFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyStreamFrame.java
index 4618d4d4a9..487844ecd9 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyStreamFrame.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyStreamFrame.java
@@ -15,6 +15,8 @@
*/
package io.netty.handler.codec.spdy;
+import static io.netty.util.internal.ObjectUtil.checkPositive;
+
/**
* The default {@link SpdyStreamFrame} implementation.
*/
@@ -39,10 +41,7 @@ public abstract class DefaultSpdyStreamFrame implements SpdyStreamFrame {
@Override
public SpdyStreamFrame setStreamId(int streamId) {
- if (streamId <= 0) {
- throw new IllegalArgumentException(
- "Stream-ID must be positive: " + streamId);
- }
+ checkPositive(streamId, "streamId");
this.streamId = streamId;
return this;
}
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynReplyFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynReplyFrame.java
index 7efc905641..f757d1dbd6 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynReplyFrame.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynReplyFrame.java
@@ -20,8 +20,7 @@ import io.netty.util.internal.StringUtil;
/**
* The default {@link SpdySynReplyFrame} implementation.
*/
-public class DefaultSpdySynReplyFrame extends DefaultSpdyHeadersFrame
- implements SpdySynReplyFrame {
+public class DefaultSpdySynReplyFrame extends DefaultSpdyHeadersFrame implements SpdySynReplyFrame {
/**
* Creates a new instance.
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynStreamFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynStreamFrame.java
index f8adc1c5f1..46fe301636 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynStreamFrame.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynStreamFrame.java
@@ -15,6 +15,8 @@
*/
package io.netty.handler.codec.spdy;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
+
import io.netty.util.internal.StringUtil;
/**
@@ -77,11 +79,7 @@ public class DefaultSpdySynStreamFrame extends DefaultSpdyHeadersFrame
@Override
public SpdySynStreamFrame setAssociatedStreamId(int associatedStreamId) {
- if (associatedStreamId < 0) {
- throw new IllegalArgumentException(
- "Associated-To-Stream-ID cannot be negative: " +
- associatedStreamId);
- }
+ checkPositiveOrZero(associatedStreamId, "associatedStreamId");
this.associatedStreamId = associatedStreamId;
return this;
}
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyWindowUpdateFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyWindowUpdateFrame.java
index f14611bac6..22b0406c80 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyWindowUpdateFrame.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyWindowUpdateFrame.java
@@ -15,6 +15,9 @@
*/
package io.netty.handler.codec.spdy;
+import static io.netty.util.internal.ObjectUtil.checkPositive;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
+
import io.netty.util.internal.StringUtil;
/**
@@ -43,10 +46,7 @@ public class DefaultSpdyWindowUpdateFrame implements SpdyWindowUpdateFrame {
@Override
public SpdyWindowUpdateFrame setStreamId(int streamId) {
- if (streamId < 0) {
- throw new IllegalArgumentException(
- "Stream-ID cannot be negative: " + streamId);
- }
+ checkPositiveOrZero(streamId, "streamId");
this.streamId = streamId;
return this;
}
@@ -58,11 +58,7 @@ public class DefaultSpdyWindowUpdateFrame implements SpdyWindowUpdateFrame {
@Override
public SpdyWindowUpdateFrame setDeltaWindowSize(int deltaWindowSize) {
- if (deltaWindowSize <= 0) {
- throw new IllegalArgumentException(
- "Delta-Window-Size must be positive: " +
- deltaWindowSize);
- }
+ checkPositive(deltaWindowSize, "deltaWindowSize");
this.deltaWindowSize = deltaWindowSize;
return this;
}
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java
index e0d1112813..fc432b6830 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java
@@ -38,6 +38,8 @@ import static io.netty.handler.codec.spdy.SpdyCodecUtil.getSignedInt;
import static io.netty.handler.codec.spdy.SpdyCodecUtil.getUnsignedInt;
import static io.netty.handler.codec.spdy.SpdyCodecUtil.getUnsignedMedium;
import static io.netty.handler.codec.spdy.SpdyCodecUtil.getUnsignedShort;
+import static io.netty.util.internal.ObjectUtil.checkPositive;
+
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
@@ -95,10 +97,7 @@ public class SpdyFrameDecoder {
if (delegate == null) {
throw new NullPointerException("delegate");
}
- if (maxChunkSize <= 0) {
- throw new IllegalArgumentException(
- "maxChunkSize must be a positive integer: " + maxChunkSize);
- }
+ checkPositive(maxChunkSize, "maxChunkSize");
this.spdyVersion = spdyVersion.getVersion();
this.delegate = delegate;
this.maxChunkSize = maxChunkSize;
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java
index 366ad15b66..5e16a6f4f2 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java
@@ -38,6 +38,7 @@ import java.util.List;
import java.util.Map;
import static io.netty.handler.codec.spdy.SpdyHeaders.HttpNames.*;
+import static io.netty.util.internal.ObjectUtil.checkPositive;
/**
* Decodes {@link SpdySynStreamFrame}s, {@link SpdySynReplyFrame}s,
@@ -103,10 +104,7 @@ public class SpdyHttpDecoder extends MessageToMessageDecoder<SpdyFrame> {
if (version == null) {
throw new NullPointerException("version");
}
- if (maxContentLength <= 0) {
- throw new IllegalArgumentException(
- "maxContentLength must be a positive integer: " + maxContentLength);
- }
+ checkPositive(maxContentLength, "maxContentLength");
spdyVersion = version.getVersion();
this.maxContentLength = maxContentLength;
this.messageMap = messageMap;
diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java
index 394f6c2e9a..8f90864151 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java
@@ -26,6 +26,7 @@ import java.util.concurrent.atomic.AtomicInteger;
import static io.netty.handler.codec.spdy.SpdyCodecUtil.SPDY_SESSION_STREAM_ID;
import static io.netty.handler.codec.spdy.SpdyCodecUtil.isServerId;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
/**
* Manages streams within a SPDY session.
@@ -77,16 +78,14 @@ public class SpdySessionHandler extends ChannelDuplexHandler {
}
public void setSessionReceiveWindowSize(int sessionReceiveWindowSize) {
- if (sessionReceiveWindowSize < 0) {
- throw new IllegalArgumentException("sessionReceiveWindowSize");
- }
- // This will not send a window update frame immediately.
- // If this value increases the allowed receive window size,
- // a WINDOW_UPDATE frame will be sent when only half of the
- // session window size remains during data frame processing.
- // If this value decreases the allowed receive window size,
- // the window will be reduced as data frames are processed.
- initialSessionReceiveWindowSize = sessionReceiveWindowSize;
+ checkPositiveOrZero(sessionReceiveWindowSize, "sessionReceiveWindowSize");
+ // This will not send a window update frame immediately.
+ // If this value increases the allowed receive window size,
+ // a WINDOW_UPDATE frame will be sent when only half of the
+ // session window size remains during data frame processing.
+ // If this value decreases the allowed receive window size,
+ // the window will be reduced as data frames are processed.
+ initialSessionReceiveWindowSize = sessionReceiveWindowSize;
}
@Override
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
index f0af13b394..18375db76a 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
@@ -29,6 +29,7 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGH
import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
import static java.lang.Integer.MAX_VALUE;
import static java.lang.Math.min;
@@ -485,9 +486,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder {
FlowControlledBase(final Http2Stream stream, int padding, boolean endOfStream,
final ChannelPromise promise) {
- if (padding < 0) {
- throw new IllegalArgumentException("padding must be >= 0");
- }
+ checkPositiveOrZero(padding, "padding");
this.padding = padding;
this.endOfStream = endOfStream;
this.stream = stream;
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 c7277561d6..77270f8343 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
@@ -61,6 +61,8 @@ import static io.netty.handler.codec.http2.Http2FrameTypes.RST_STREAM;
import static io.netty.handler.codec.http2.Http2FrameTypes.SETTINGS;
import static io.netty.handler.codec.http2.Http2FrameTypes.WINDOW_UPDATE;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
+import static io.netty.util.internal.ObjectUtil.checkPositive;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
import static java.lang.Math.max;
import static java.lang.Math.min;
@@ -547,15 +549,11 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize
}
private static void verifyStreamId(int streamId, String argumentName) {
- if (streamId <= 0) {
- throw new IllegalArgumentException(argumentName + " must be > 0");
- }
+ checkPositive(streamId, "streamId");
}
private static void verifyStreamOrConnectionId(int streamId, String argumentName) {
- if (streamId < 0) {
- throw new IllegalArgumentException(argumentName + " must be >= 0");
- }
+ checkPositiveOrZero(streamId, "streamId");
}
private static void verifyWeight(short weight) {
@@ -571,9 +569,7 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize
}
private static void verifyWindowSizeIncrement(int windowSizeIncrement) {
- if (windowSizeIncrement < 0) {
- throw new IllegalArgumentException("WindowSizeIncrement must be >= 0");
- }
+ checkPositiveOrZero(windowSizeIncrement, "windowSizeIncrement");
}
private static void verifyPingPayload(ByteBuf data) {
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2GoAwayFrame.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2GoAwayFrame.java
index 8f54b8e329..dc01f37482 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2GoAwayFrame.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2GoAwayFrame.java
@@ -15,6 +15,8 @@
*/
package io.netty.handler.codec.http2;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
+
import io.netty.buffer.ByteBuf;
import io.netty.buffer.DefaultByteBufHolder;
import io.netty.buffer.Unpooled;
@@ -97,9 +99,7 @@ public final class DefaultHttp2GoAwayFrame extends DefaultByteBufHolder implemen
@Override
public Http2GoAwayFrame setExtraStreamIds(int extraStreamIds) {
- if (extraStreamIds < 0) {
- throw new IllegalArgumentException("extraStreamIds must be non-negative");
- }
+ checkPositiveOrZero(extraStreamIds, "extraStreamIds");
this.extraStreamIds = extraStreamIds;
return this;
}
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowController.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowController.java
index 74dc3ae31c..cac715614d 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowController.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowController.java
@@ -24,6 +24,7 @@ import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
import static io.netty.handler.codec.http2.Http2Exception.streamError;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
import static java.lang.Math.max;
import static java.lang.Math.min;
import io.netty.buffer.ByteBuf;
@@ -173,9 +174,7 @@ public class DefaultHttp2LocalFlowController implements Http2LocalFlowController
@Override
public boolean consumeBytes(Http2Stream stream, int numBytes) throws Http2Exception {
assert ctx != null && ctx.executor().inEventLoop();
- if (numBytes < 0) {
- throw new IllegalArgumentException("numBytes must not be negative");
- }
+ checkPositiveOrZero(numBytes, "numBytes");
if (numBytes == 0) {
return false;
}
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java
index 034140c81f..125a394cae 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java
@@ -32,6 +32,7 @@ import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.streamError;
import static io.netty.handler.codec.http2.Http2Stream.State.HALF_CLOSED_LOCAL;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
import static java.lang.Math.max;
import static java.lang.Math.min;
@@ -652,9 +653,7 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
}
void initialWindowSize(int newWindowSize) throws Http2Exception {
- if (newWindowSize < 0) {
- throw new IllegalArgumentException("Invalid initial window size: " + newWindowSize);
- }
+ checkPositiveOrZero(newWindowSize, "newWindowSize");
final int delta = newWindowSize - initialWindowSize;
initialWindowSize = newWindowSize;
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java
index 78ef230c62..3e73bd68dd 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java
@@ -33,6 +33,7 @@ import static io.netty.handler.codec.http.HttpHeaderValues.X_GZIP;
import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.streamError;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
/**
* A HTTP2 frame listener that will decompress data frames according to the {@code content-encoding} header for each
@@ -398,9 +399,7 @@ public class DelegatingDecompressorFrameListener extends Http2FrameListenerDecor
* @return The number of pre-decompressed bytes that have been consumed.
*/
int consumeBytes(int streamId, int decompressedBytes) throws Http2Exception {
- if (decompressedBytes < 0) {
- throw new IllegalArgumentException("decompressedBytes must not be negative: " + decompressedBytes);
- }
+ checkPositiveOrZero(decompressedBytes, "decompressedBytes");
if (decompressed - decompressedBytes < 0) {
throw streamError(streamId, INTERNAL_ERROR,
"Attempting to return too many bytes for stream %d. decompressed: %d " +
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/UniformStreamByteDistributor.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/UniformStreamByteDistributor.java
index c3e5e2faaa..6204c7bb9c 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/UniformStreamByteDistributor.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/UniformStreamByteDistributor.java
@@ -24,6 +24,7 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.streamableBytes;
import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
+import static io.netty.util.internal.ObjectUtil.checkPositive;
import static java.lang.Math.max;
import static java.lang.Math.min;
@@ -72,9 +73,7 @@ public final class UniformStreamByteDistributor implements StreamByteDistributor
* Must be > 0.
*/
public void minAllocationChunk(int minAllocationChunk) {
- if (minAllocationChunk <= 0) {
- throw new IllegalArgumentException("minAllocationChunk must be > 0");
- }
+ checkPositive(minAllocationChunk, "minAllocationChunk");
this.minAllocationChunk = minAllocationChunk;
}
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/WeightedFairQueueByteDistributor.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/WeightedFairQueueByteDistributor.java
index c215376c72..d26c088c62 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/WeightedFairQueueByteDistributor.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/WeightedFairQueueByteDistributor.java
@@ -36,6 +36,8 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGH
import static io.netty.handler.codec.http2.Http2CodecUtil.streamableBytes;
import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
+import static io.netty.util.internal.ObjectUtil.checkPositive;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
import static java.lang.Integer.MAX_VALUE;
import static java.lang.Math.max;
import static java.lang.Math.min;
@@ -95,9 +97,8 @@ public final class WeightedFairQueueByteDistributor implements StreamByteDistrib
}
public WeightedFairQueueByteDistributor(Http2Connection connection, int maxStateOnlySize) {
- if (maxStateOnlySize < 0) {
- throw new IllegalArgumentException("maxStateOnlySize: " + maxStateOnlySize + " (expected: >0)");
- } else if (maxStateOnlySize == 0) {
+ checkPositiveOrZero(maxStateOnlySize, "maxStateOnlySize");
+ if (maxStateOnlySize == 0) {
stateOnlyMap = IntCollections.emptyMap();
stateOnlyRemovalQueue = EmptyPriorityQueue.instance();
} else {
@@ -280,9 +281,7 @@ public final class WeightedFairQueueByteDistributor implements StreamByteDistrib
* @param allocationQuantum the amount of bytes that will be allocated to each stream. Must be &gt; 0.
*/
public void allocationQuantum(int allocationQuantum) {
- if (allocationQuantum <= 0) {
- throw new IllegalArgumentException("allocationQuantum must be > 0");
- }
+ checkPositive(allocationQuantum, "allocationQuantum");
this.allocationQuantum = allocationQuantum;
}
diff --git a/codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/AbstractBinaryMemcacheDecoder.java b/codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/AbstractBinaryMemcacheDecoder.java
index 2c90382829..bec754afbd 100644
--- a/codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/AbstractBinaryMemcacheDecoder.java
+++ b/codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/AbstractBinaryMemcacheDecoder.java
@@ -15,6 +15,8 @@
*/
package io.netty.handler.codec.memcache.binary;
+import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
+
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelHandlerContext;
@@ -59,9 +61,7 @@ public abstract class AbstractBinaryMemcacheDecoder<M extends BinaryMemcacheMess
* @param chunkSize the maximum chunk size of the payload.
*/
protected AbstractBinaryMemcacheDecoder(int chunkSize) {
- if (chunkSize < 0) {
- throw new IllegalArgumentException("chunkSize must be a positive integer: " + chunkSize);
- }
+ checkPositiveOrZero(chunkSize, "chunkSize");
this.chunkSize = chunkSize;
}
diff --git a/codec-stomp/src/main/java/io/netty/handler/codec/stomp/StompSubframeDecoder.java b/codec-stomp/src/main/java/io/netty/handler/codec/stomp/StompSubframeDecoder.java
index e25c15447a..ca59f8494e 100644
--- a/codec-stomp/src/main/java/io/netty/handler/codec/stomp/StompSubframeDecoder.java
+++ b/codec-stomp/src/main/java/io/netty/handler/codec/stomp/StompSubframeDecoder.java
@@ -30,6 +30,7 @@ import java.util.Locale;
import static io.netty.buffer.ByteBufUtil.indexOf;
import static io.netty.buffer.ByteBufUtil.readBytes;
+import static io.netty.util.internal.ObjectUtil.checkPositive;
/**
* Decodes {@link ByteBuf}s into {@link StompHeadersSubframe}s and
@@ -81,16 +82,8 @@ public class StompSubframeDecoder extends ReplayingDecoder<State> {
public StompSubframeDecoder(int maxLineLength, int maxChunkSize) {
super(State.SKIP_CONTROL_CHARACTERS);
- if (maxLineLength <= 0) {
- throw new IllegalArgumentException(
- "maxLineLength must be a positive integer: " +
- maxLineLength);
- }
- if (maxChunkSize <= 0) {
- throw new IllegalArgumentException(
- "maxChunkSize must be a positive integer: " +
- maxChunkSize);
- }
+ checkPositive(maxLineLength, "maxLineLength");
+ checkPositive(maxChunkSize, "maxChunkSize");
this.maxChunkSize = maxChunkSize;
this.maxLineLength = maxLineLength;
}
--
2.23.0

566
CVE-2021-21295-pre3.patch Normal file
View File

@ -0,0 +1,566 @@
From 9557c88da2aaa49b3c3fd7525462dc0694681c19 Mon Sep 17 00:00:00 2001
From: Bennett Lynch <bennett.lynch@gmail.com>
Date: Mon, 6 Jul 2020 01:25:13 -0700
Subject: [PATCH] Add option to HttpObjectDecoder to allow duplicate
Content-Lengths (#10349)
Motivation:
Since https://github.com/netty/netty/pull/9865 (Netty 4.1.44) the
default behavior of the HttpObjectDecoder has been to reject any HTTP
message that is found to have multiple Content-Length headers when
decoding. This behavior is well-justified as per the risks outlined in
https://github.com/netty/netty/issues/9861, however, we can see from the
cited RFC section that there are multiple possible options offered for
responding to this scenario:
> If a message is received that has multiple Content-Length header
> fields with field-values consisting of the same decimal value, or a
> single Content-Length header field with a field value containing a
> list of identical decimal values (e.g., "Content-Length: 42, 42"),
> indicating that duplicate Content-Length header fields have been
> generated or combined by an upstream message processor, then the
> recipient MUST either reject the message as invalid or replace the
> duplicated field-values with a single valid Content-Length field
> containing that decimal value prior to determining the message body
> length or forwarding the message.
https://tools.ietf.org/html/rfc7230#section-3.3.2
Netty opted for the first option (rejecting as invalid), which seems
like the safest, but the second option (replacing duplicate values with
a single value) is also valid behavior.
Modifications:
* Introduce "allowDuplicateContentLengths" parameter to
HttpObjectDecoder (defaulting to false).
* When set to true, will allow multiple Content-Length headers only if
they are all the same value. The duplicated field-values will be
replaced with a single valid Content-Length field.
* Add new parameterized test class for testing different variations of
multiple Content-Length headers.
Result:
This is a backwards-compatible change with no functional change to the
existing behavior.
Note that the existing logic would result in NumberFormatExceptions
for header values like "Content-Length: 42, 42". The new logic correctly
reports these as IllegalArgumentException with the proper error message.
Additionally note that this behavior is only applied to HTTP/1.1, but I
suspect that we may want to expand that to include HTTP/1.0 as well...
That behavior is not modified here to minimize the scope of this change.
---
.../handler/codec/http/HttpClientCodec.java | 35 ++++-
.../handler/codec/http/HttpObjectDecoder.java | 75 ++++++++-
.../codec/http/HttpRequestDecoder.java | 7 +
.../codec/http/HttpResponseDecoder.java | 7 +
.../handler/codec/http/HttpServerCodec.java | 16 ++
.../MultipleContentLengthHeadersTest.java | 144 ++++++++++++++++++
6 files changed, 268 insertions(+), 16 deletions(-)
create mode 100644 codec-http/src/test/java/io/netty/handler/codec/http/MultipleContentLengthHeadersTest.java
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java
index a832bfdff3..9a99fff97f 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java
@@ -28,9 +28,11 @@ import java.util.List;
import java.util.Queue;
import java.util.concurrent.atomic.AtomicLong;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS;
import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_CHUNK_SIZE;
import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_HEADER_SIZE;
import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_INITIAL_LINE_LENGTH;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_VALIDATE_HEADERS;
/**
* A combination of {@link HttpRequestEncoder} and {@link HttpResponseDecoder}
@@ -48,6 +50,8 @@ import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_INITIAL_
*/
public final class HttpClientCodec extends CombinedChannelDuplexHandler<HttpResponseDecoder, HttpRequestEncoder>
implements HttpClientUpgradeHandler.SourceCodec {
+ public static final boolean DEFAULT_FAIL_ON_MISSING_RESPONSE = false;
+ public static final boolean DEFAULT_PARSE_HTTP_AFTER_CONNECT_REQUEST = false;
/** A queue that is used for correlating a request and a response. */
private final Queue<HttpMethod> queue = new ArrayDeque<HttpMethod>();
@@ -65,14 +69,15 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler<HttpResp
* {@code maxChunkSize (8192)}).
*/
public HttpClientCodec() {
- this(DEFAULT_MAX_INITIAL_LINE_LENGTH, DEFAULT_MAX_HEADER_SIZE, DEFAULT_MAX_CHUNK_SIZE, false);
+ this(DEFAULT_MAX_INITIAL_LINE_LENGTH, DEFAULT_MAX_HEADER_SIZE, DEFAULT_MAX_CHUNK_SIZE,
+ DEFAULT_FAIL_ON_MISSING_RESPONSE);
}
/**
* Creates a new instance with the specified decoder options.
*/
public HttpClientCodec(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize) {
- this(maxInitialLineLength, maxHeaderSize, maxChunkSize, false);
+ this(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_FAIL_ON_MISSING_RESPONSE);
}
/**
@@ -80,7 +85,7 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler<HttpResp
*/
public HttpClientCodec(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean failOnMissingResponse) {
- this(maxInitialLineLength, maxHeaderSize, maxChunkSize, failOnMissingResponse, true);
+ this(maxInitialLineLength, maxHeaderSize, maxChunkSize, failOnMissingResponse, DEFAULT_VALIDATE_HEADERS);
}
/**
@@ -89,7 +94,8 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler<HttpResp
public HttpClientCodec(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean failOnMissingResponse,
boolean validateHeaders) {
- this(maxInitialLineLength, maxHeaderSize, maxChunkSize, failOnMissingResponse, validateHeaders, false);
+ this(maxInitialLineLength, maxHeaderSize, maxChunkSize, failOnMissingResponse, validateHeaders,
+ DEFAULT_PARSE_HTTP_AFTER_CONNECT_REQUEST);
}
/**
@@ -110,7 +116,7 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler<HttpResp
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean failOnMissingResponse,
boolean validateHeaders, int initialBufferSize) {
this(maxInitialLineLength, maxHeaderSize, maxChunkSize, failOnMissingResponse, validateHeaders,
- initialBufferSize, false);
+ initialBufferSize, DEFAULT_PARSE_HTTP_AFTER_CONNECT_REQUEST);
}
/**
@@ -119,7 +125,19 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler<HttpResp
public HttpClientCodec(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean failOnMissingResponse,
boolean validateHeaders, int initialBufferSize, boolean parseHttpAfterConnectRequest) {
- init(new Decoder(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders, initialBufferSize),
+ this(maxInitialLineLength, maxHeaderSize, maxChunkSize, failOnMissingResponse, validateHeaders,
+ initialBufferSize, parseHttpAfterConnectRequest, DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS);
+ }
+
+ /**
+ * Creates a new instance with the specified decoder options.
+ */
+ public HttpClientCodec(
+ int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean failOnMissingResponse,
+ boolean validateHeaders, int initialBufferSize, boolean parseHttpAfterConnectRequest,
+ boolean allowDuplicateContentLengths) {
+ init(new Decoder(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders, initialBufferSize,
+ allowDuplicateContentLengths),
new Encoder());
this.parseHttpAfterConnectRequest = parseHttpAfterConnectRequest;
this.failOnMissingResponse = failOnMissingResponse;
@@ -186,8 +204,9 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler<HttpResp
}
Decoder(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders,
- int initialBufferSize) {
- super(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders, initialBufferSize);
+ int initialBufferSize, boolean allowDuplicateContentLengths) {
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders, initialBufferSize,
+ allowDuplicateContentLengths);
}
@Override
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
index ed7caa7801..b52e36ac92 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
@@ -16,6 +16,7 @@
package io.netty.handler.codec.http;
import static io.netty.util.internal.ObjectUtil.checkPositive;
+import static io.netty.util.internal.StringUtil.COMMA;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
@@ -29,6 +30,7 @@ import io.netty.util.ByteProcessor;
import io.netty.util.internal.AppendableCharSequence;
import java.util.List;
+import java.util.regex.Pattern;
/**
* Decodes {@link ByteBuf}s into {@link HttpMessage}s and
@@ -37,10 +39,11 @@ import java.util.List;
* <h3>Parameters that prevents excessive memory consumption</h3>
* <table border="1">
* <tr>
- * <th>Name</th><th>Meaning</th>
+ * <th>Name</th><th>Default value</th><th>Meaning</th>
* </tr>
* <tr>
* <td>{@code maxInitialLineLength}</td>
+ * <td>{@value #DEFAULT_MAX_INITIAL_LINE_LENGTH}</td>
* <td>The maximum length of the initial line
* (e.g. {@code "GET / HTTP/1.0"} or {@code "HTTP/1.0 200 OK"})
* If the length of the initial line exceeds this value, a
@@ -48,11 +51,13 @@ import java.util.List;
* </tr>
* <tr>
* <td>{@code maxHeaderSize}</td>
+ * <td>{@value #DEFAULT_MAX_HEADER_SIZE}</td>
* <td>The maximum length of all headers. If the sum of the length of each
* header exceeds this value, a {@link TooLongFrameException} will be raised.</td>
* </tr>
* <tr>
* <td>{@code maxChunkSize}</td>
+ * <td>{@value #DEFAULT_MAX_CHUNK_SIZE}</td>
* <td>The maximum length of the content or each chunk. If the content length
* (or the length of each chunk) exceeds this value, the content or chunk
* will be split into multiple {@link HttpContent}s whose length is
@@ -60,6 +65,21 @@ import java.util.List;
* </tr>
* </table>
*
+ * <h3>Parameters that control parsing behavior</h3>
+ * <table border="1">
+ * <tr>
+ * <th>Name</th><th>Default value</th><th>Meaning</th>
+ * </tr>
+ * <tr>
+ * <td>{@code allowDuplicateContentLengths}</td>
+ * <td>{@value #DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS}</td>
+ * <td>When set to {@code false}, will reject any messages that contain multiple Content-Length header fields.
+ * When set to {@code true}, will allow multiple Content-Length headers only if they are all the same decimal value.
+ * The duplicated field-values will be replaced with a single valid Content-Length field.
+ * See <a href="https://tools.ietf.org/html/rfc7230#section-3.3.2">RFC 7230, Section 3.3.2</a>.</td>
+ * </tr>
+ * </table>
+ *
* <h3>Chunked Content</h3>
*
* If the content of an HTTP message is greater than {@code maxChunkSize} or
@@ -108,12 +128,15 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
public static final int DEFAULT_MAX_CHUNK_SIZE = 8192;
public static final boolean DEFAULT_VALIDATE_HEADERS = true;
public static final int DEFAULT_INITIAL_BUFFER_SIZE = 128;
+ public static final boolean DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS = false;
private static final String EMPTY_VALUE = "";
+ private static final Pattern COMMA_PATTERN = Pattern.compile(",");
private final int maxChunkSize;
private final boolean chunkedSupported;
protected final boolean validateHeaders;
+ private final boolean allowDuplicateContentLengths;
private final HeaderParser headerParser;
private final LineParser lineParser;
@@ -176,9 +199,20 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
DEFAULT_INITIAL_BUFFER_SIZE);
}
+ /**
+ * Creates a new instance with the specified parameters.
+ */
protected HttpObjectDecoder(
int maxInitialLineLength, int maxHeaderSize, int maxChunkSize,
boolean chunkedSupported, boolean validateHeaders, int initialBufferSize) {
+ this(maxInitialLineLength, maxHeaderSize, maxChunkSize, chunkedSupported, validateHeaders, initialBufferSize,
+ DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS);
+ }
+
+ protected HttpObjectDecoder(
+ int maxInitialLineLength, int maxHeaderSize, int maxChunkSize,
+ boolean chunkedSupported, boolean validateHeaders, int initialBufferSize,
+ boolean allowDuplicateContentLengths) {
checkPositive(maxInitialLineLength, "maxInitialLineLength");
checkPositive(maxHeaderSize, "maxHeaderSize");
checkPositive(maxChunkSize, "maxChunkSize");
@@ -189,6 +223,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
this.maxChunkSize = maxChunkSize;
this.chunkedSupported = chunkedSupported;
this.validateHeaders = validateHeaders;
+ this.allowDuplicateContentLengths = allowDuplicateContentLengths;
}
@Override
@@ -602,10 +637,9 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
name = null;
value = null;
- List<String> values = headers.getAll(HttpHeaderNames.CONTENT_LENGTH);
- int contentLengthValuesCount = values.size();
+ List<String> contentLengthFields = headers.getAll(HttpHeaderNames.CONTENT_LENGTH);
- if (contentLengthValuesCount > 0) {
+ if (!contentLengthFields.isEmpty()) {
// Guard against multiple Content-Length headers as stated in
// https://tools.ietf.org/html/rfc7230#section-3.3.2:
//
@@ -619,17 +653,42 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
// duplicated field-values with a single valid Content-Length field
// containing that decimal value prior to determining the message body
// length or forwarding the message.
- if (contentLengthValuesCount > 1 && message.protocolVersion() == HttpVersion.HTTP_1_1) {
- throw new IllegalArgumentException("Multiple Content-Length headers found");
+ boolean multipleContentLengths =
+ contentLengthFields.size() > 1 || contentLengthFields.get(0).indexOf(COMMA) >= 0;
+ if (multipleContentLengths && message.protocolVersion() == HttpVersion.HTTP_1_1) {
+ if (allowDuplicateContentLengths) {
+ // Find and enforce that all Content-Length values are the same
+ String firstValue = null;
+ for (String field : contentLengthFields) {
+ String[] tokens = COMMA_PATTERN.split(field, -1);
+ for (String token : tokens) {
+ String trimmed = token.trim();
+ if (firstValue == null) {
+ firstValue = trimmed;
+ } else if (!trimmed.equals(firstValue)) {
+ throw new IllegalArgumentException(
+ "Multiple Content-Length values found: " + contentLengthFields);
+ }
+ }
+ }
+ // Replace the duplicated field-values with a single valid Content-Length field
+ headers.set(HttpHeaderNames.CONTENT_LENGTH, firstValue);
+ contentLength = Long.parseLong(firstValue);
+ } else {
+ // Reject the message as invalid
+ throw new IllegalArgumentException(
+ "Multiple Content-Length values found: " + contentLengthFields);
+ }
+ } else {
+ contentLength = Long.parseLong(contentLengthFields.get(0));
}
- contentLength = Long.parseLong(values.get(0));
}
if (isContentAlwaysEmpty(message)) {
HttpUtil.setTransferEncodingChunked(message, false);
return State.SKIP_CONTROL_CHARS;
} else if (HttpUtil.isTransferEncodingChunked(message)) {
- if (contentLengthValuesCount > 0 && message.protocolVersion() == HttpVersion.HTTP_1_1) {
+ if (!contentLengthFields.isEmpty() && message.protocolVersion() == HttpVersion.HTTP_1_1) {
handleTransferEncodingChunkedWithContentLength(message);
}
return State.READ_CHUNK_SIZE;
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java
index 70c1db5540..ba2d79ecb4 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java
@@ -82,6 +82,13 @@ public class HttpRequestDecoder extends HttpObjectDecoder {
initialBufferSize);
}
+ public HttpRequestDecoder(
+ int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders,
+ int initialBufferSize, boolean allowDuplicateContentLengths) {
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED, validateHeaders,
+ initialBufferSize, allowDuplicateContentLengths);
+ }
+
@Override
protected HttpMessage createMessage(String[] initialLine) throws Exception {
return new DefaultHttpRequest(
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java
index 39d4d6a5ad..62f6dd3554 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java
@@ -113,6 +113,13 @@ public class HttpResponseDecoder extends HttpObjectDecoder {
initialBufferSize);
}
+ public HttpResponseDecoder(
+ int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders,
+ int initialBufferSize, boolean allowDuplicateContentLengths) {
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED, validateHeaders,
+ initialBufferSize, allowDuplicateContentLengths);
+ }
+
@Override
protected HttpMessage createMessage(String[] initialLine) {
return new DefaultHttpResponse(
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java
index 8ae6295cf7..b2b905e083 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java
@@ -75,6 +75,16 @@ public final class HttpServerCodec extends CombinedChannelDuplexHandler<HttpRequ
new HttpServerResponseEncoder());
}
+ /**
+ * Creates a new instance with the specified decoder options.
+ */
+ public HttpServerCodec(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders,
+ int initialBufferSize, boolean allowDuplicateContentLengths) {
+ init(new HttpServerRequestDecoder(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders,
+ initialBufferSize, allowDuplicateContentLengths),
+ new HttpServerResponseEncoder());
+ }
+
/**
* Upgrades to another protocol from HTTP. Removes the {@link HttpRequestDecoder} and
* {@link HttpResponseEncoder} from the pipeline.
@@ -99,6 +109,12 @@ public final class HttpServerCodec extends CombinedChannelDuplexHandler<HttpRequ
super(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders, initialBufferSize);
}
+ HttpServerRequestDecoder(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize,
+ boolean validateHeaders, int initialBufferSize, boolean allowDuplicateContentLengths) {
+ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, validateHeaders, initialBufferSize,
+ allowDuplicateContentLengths);
+ }
+
@Override
protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List<Object> out) throws Exception {
int oldSize = out.size();
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/MultipleContentLengthHeadersTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/MultipleContentLengthHeadersTest.java
new file mode 100644
index 0000000000..29c7d84b71
--- /dev/null
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/MultipleContentLengthHeadersTest.java
@@ -0,0 +1,144 @@
+/*
+ * Copyright 2020 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.handler.codec.http;
+
+import io.netty.buffer.Unpooled;
+import io.netty.channel.embedded.EmbeddedChannel;
+import io.netty.util.CharsetUtil;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_INITIAL_BUFFER_SIZE;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_CHUNK_SIZE;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_HEADER_SIZE;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_INITIAL_LINE_LENGTH;
+import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_VALIDATE_HEADERS;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
+@RunWith(Parameterized.class)
+public class MultipleContentLengthHeadersTest {
+
+ private final boolean allowDuplicateContentLengths;
+ private final boolean sameValue;
+ private final boolean singleField;
+
+ private EmbeddedChannel channel;
+
+ @Parameters
+ public static Collection<Object[]> parameters() {
+ return Arrays.asList(new Object[][] {
+ { false, false, false },
+ { false, false, true },
+ { false, true, false },
+ { false, true, true },
+ { true, false, false },
+ { true, false, true },
+ { true, true, false },
+ { true, true, true }
+ });
+ }
+
+ public MultipleContentLengthHeadersTest(
+ boolean allowDuplicateContentLengths, boolean sameValue, boolean singleField) {
+ this.allowDuplicateContentLengths = allowDuplicateContentLengths;
+ this.sameValue = sameValue;
+ this.singleField = singleField;
+ }
+
+ @Before
+ public void setUp() {
+ HttpRequestDecoder decoder = new HttpRequestDecoder(
+ DEFAULT_MAX_INITIAL_LINE_LENGTH,
+ DEFAULT_MAX_HEADER_SIZE,
+ DEFAULT_MAX_CHUNK_SIZE,
+ DEFAULT_VALIDATE_HEADERS,
+ DEFAULT_INITIAL_BUFFER_SIZE,
+ allowDuplicateContentLengths);
+ channel = new EmbeddedChannel(decoder);
+ }
+
+ @Test
+ public void testMultipleContentLengthHeadersBehavior() {
+ String requestStr = setupRequestString();
+ assertThat(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)), is(true));
+ HttpRequest request = channel.readInbound();
+
+ if (allowDuplicateContentLengths) {
+ if (sameValue) {
+ assertValid(request);
+ List<String> contentLengths = request.headers().getAll(HttpHeaderNames.CONTENT_LENGTH);
+ assertThat(contentLengths, contains("1"));
+ LastHttpContent body = channel.readInbound();
+ assertThat(body.content().readableBytes(), is(1));
+ assertThat(body.content().readCharSequence(1, CharsetUtil.US_ASCII).toString(), is("a"));
+ } else {
+ assertInvalid(request);
+ }
+ } else {
+ assertInvalid(request);
+ }
+ assertThat(channel.finish(), is(false));
+ }
+
+ private String setupRequestString() {
+ String firstValue = "1";
+ String secondValue = sameValue ? firstValue : "2";
+ String contentLength;
+ if (singleField) {
+ contentLength = "Content-Length: " + firstValue + ", " + secondValue + "\r\n\r\n";
+ } else {
+ contentLength = "Content-Length: " + firstValue + "\r\n" +
+ "Content-Length: " + secondValue + "\r\n\r\n";
+ }
+ return "PUT /some/path HTTP/1.1\r\n" +
+ contentLength +
+ "ab";
+ }
+
+ @Test
+ public void testDanglingComma() {
+ String requestStr = "GET /some/path HTTP/1.1\r\n" +
+ "Content-Length: 1,\r\n" +
+ "Connection: close\n\n" +
+ "ab";
+ assertThat(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)), is(true));
+ HttpRequest request = channel.readInbound();
+ assertInvalid(request);
+ assertThat(channel.finish(), is(false));
+ }
+
+ private static void assertValid(HttpRequest request) {
+ assertThat(request.decoderResult().isFailure(), is(false));
+ }
+
+ private static void assertInvalid(HttpRequest request) {
+ assertThat(request.decoderResult().isFailure(), is(true));
+ assertThat(request.decoderResult().cause(), instanceOf(IllegalArgumentException.class));
+ assertThat(request.decoderResult().cause().getMessage(),
+ containsString("Multiple Content-Length values found"));
+ }
+}
--
2.23.0

771
CVE-2021-21295-pre4.patch Normal file
View File

@ -0,0 +1,771 @@
From a91df58ca17d5b30c57c46dde5b1d60bb659b029 Mon Sep 17 00:00:00 2001
From: Scott Mitchell <scott_mitchell@apple.com>
Date: Tue, 11 Jul 2017 14:53:49 -0700
Subject: [PATCH] HTTP/2 enforce HTTP message flow
Motivation:
codec-http2 currently does not strictly enforce the HTTP/1.x semantics with respect to the number of headers defined in RFC 7540 Section 8.1 [1]. We currently don't validate the number of headers nor do we validate that the trailing headers should indicate EOS.
[1] https://tools.ietf.org/html/rfc7540#section-8.1
Modifications:
- DefaultHttp2ConnectionDecoder should only allow decoding of a single headers and a single trailers
- DefaultHttp2ConnectionEncoder should only allow encoding of a single headers and optionally a single trailers
Result:
Constraints of RFC 7540 restricting the number of headers/trailers is enforced.
---
.../handler/codec/http/HttpStatusClass.java | 21 ++
.../codec/http2/DefaultHttp2Connection.java | 52 +++--
.../http2/DefaultHttp2ConnectionDecoder.java | 11 +
.../http2/DefaultHttp2ConnectionEncoder.java | 18 +-
.../handler/codec/http2/Http2Stream.java | 32 ++-
.../DefaultHttp2ConnectionDecoderTest.java | 103 ++++++++-
.../DefaultHttp2ConnectionEncoderTest.java | 203 +++++++++++++++++-
.../http2/InboundHttp2ToHttpAdapterTest.java | 43 ----
8 files changed, 419 insertions(+), 64 deletions(-)
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpStatusClass.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpStatusClass.java
index 9f57e18984..0a4f4c11ab 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpStatusClass.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpStatusClass.java
@@ -74,6 +74,27 @@ public enum HttpStatusClass {
return UNKNOWN;
}
+ /**
+ * Returns the class of the specified HTTP status code.
+ * @param code Just the numeric portion of the http status code.
+ */
+ public static HttpStatusClass valueOf(CharSequence code) {
+ if (code != null && code.length() == 3) {
+ char c0 = code.charAt(0);
+ return isDigit(c0) && isDigit(code.charAt(1)) && isDigit(code.charAt(2)) ? valueOf(digit(c0) * 100)
+ : UNKNOWN;
+ }
+ return UNKNOWN;
+ }
+
+ private static int digit(char c) {
+ return c - '0';
+ }
+
+ private static boolean isDigit(char c) {
+ return c >= '0' && c <= '9';
+ }
+
private final int min;
private final int max;
private final AsciiString defaultReasonPhrase;
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java
index 2789423bc7..12815c225c 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java
@@ -373,13 +373,16 @@ public class DefaultHttp2Connection implements Http2Connection {
* Simple stream implementation. Streams can be compared to each other by priority.
*/
private class DefaultStream implements Http2Stream {
- private static final byte SENT_STATE_RST = 0x1;
- private static final byte SENT_STATE_HEADERS = 0x2;
- private static final byte SENT_STATE_PUSHPROMISE = 0x4;
+ private static final byte META_STATE_SENT_RST = 1;
+ private static final byte META_STATE_SENT_HEADERS = 1 << 1;
+ private static final byte META_STATE_SENT_TRAILERS = 1 << 2;
+ private static final byte META_STATE_SENT_PUSHPROMISE = 1 << 3;
+ private static final byte META_STATE_RECV_HEADERS = 1 << 4;
+ private static final byte META_STATE_RECV_TRAILERS = 1 << 5;
private final int id;
private final PropertyMap properties = new PropertyMap();
private State state;
- private byte sentState;
+ private byte metaState;
DefaultStream(int id, State state) {
this.id = id;
@@ -398,35 +401,60 @@ public class DefaultHttp2Connection implements Http2Connection {
@Override
public boolean isResetSent() {
- return (sentState & SENT_STATE_RST) != 0;
+ return (metaState & META_STATE_SENT_RST) != 0;
}
@Override
public Http2Stream resetSent() {
- sentState |= SENT_STATE_RST;
+ metaState |= META_STATE_SENT_RST;
return this;
}
@Override
- public Http2Stream headersSent() {
- sentState |= SENT_STATE_HEADERS;
+ public Http2Stream headersSent(boolean isInformational) {
+ if (!isInformational) {
+ metaState |= isHeadersSent() ? META_STATE_SENT_TRAILERS : META_STATE_SENT_HEADERS;
+ }
return this;
}
@Override
public boolean isHeadersSent() {
- return (sentState & SENT_STATE_HEADERS) != 0;
+ return (metaState & META_STATE_SENT_HEADERS) != 0;
+ }
+
+ @Override
+ public boolean isTrailersSent() {
+ return (metaState & META_STATE_SENT_TRAILERS) != 0;
+ }
+
+ @Override
+ public Http2Stream headersReceived(boolean isInformational) {
+ if (!isInformational) {
+ metaState |= isHeadersReceived() ? META_STATE_RECV_TRAILERS : META_STATE_RECV_HEADERS;
+ }
+ return this;
+ }
+
+ @Override
+ public boolean isHeadersReceived() {
+ return (metaState & META_STATE_RECV_HEADERS) != 0;
+ }
+
+ @Override
+ public boolean isTrailersReceived() {
+ return (metaState & META_STATE_RECV_TRAILERS) != 0;
}
@Override
public Http2Stream pushPromiseSent() {
- sentState |= SENT_STATE_PUSHPROMISE;
+ metaState |= META_STATE_SENT_PUSHPROMISE;
return this;
}
@Override
public boolean isPushPromiseSent() {
- return (sentState & SENT_STATE_PUSHPROMISE) != 0;
+ return (metaState & META_STATE_SENT_PUSHPROMISE) != 0;
}
@Override
@@ -599,7 +627,7 @@ public class DefaultHttp2Connection implements Http2Connection {
}
@Override
- public Http2Stream headersSent() {
+ public Http2Stream headersSent(boolean isInformational) {
throw new UnsupportedOperationException();
}
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 ef643fafad..4027978651 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
@@ -16,6 +16,7 @@ package io.netty.handler.codec.http2;
import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.http.HttpStatusClass;
import io.netty.handler.codec.http2.Http2Connection.Endpoint;
import io.netty.util.internal.UnstableApi;
import io.netty.util.internal.logging.InternalLogger;
@@ -23,6 +24,7 @@ import io.netty.util.internal.logging.InternalLoggerFactory;
import java.util.List;
+import static io.netty.handler.codec.http.HttpStatusClass.INFORMATIONAL;
import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGHT;
import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR;
import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
@@ -282,6 +284,14 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
return;
}
+ boolean isInformational = !connection.isServer() &&
+ HttpStatusClass.valueOf(headers.status()) == INFORMATIONAL;
+ if ((isInformational || !endOfStream) && stream.isHeadersReceived() || stream.isTrailersReceived()) {
+ throw streamError(streamId, PROTOCOL_ERROR,
+ "Stream %d received too many headers EOS: %s state: %s",
+ streamId, endOfStream, stream.state());
+ }
+
switch (stream.state()) {
case RESERVED_REMOTE:
stream.open(endOfStream);
@@ -305,6 +315,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
stream.state());
}
+ stream.headersReceived(isInformational);
encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive);
listener.onHeadersRead(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream);
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
index 18375db76a..f129f63fd8 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
@@ -21,10 +21,12 @@ import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPromise;
import io.netty.channel.CoalescingBufferQueue;
+import io.netty.handler.codec.http.HttpStatusClass;
import io.netty.util.internal.UnstableApi;
import java.util.ArrayDeque;
+import static io.netty.handler.codec.http.HttpStatusClass.INFORMATIONAL;
import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGHT;
import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
@@ -146,6 +148,15 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder {
return writeHeaders(ctx, streamId, headers, 0, DEFAULT_PRIORITY_WEIGHT, false, padding, endStream, promise);
}
+ private static boolean validateHeadersSentState(Http2Stream stream, Http2Headers headers, boolean isServer,
+ boolean endOfStream) {
+ boolean isInformational = isServer && HttpStatusClass.valueOf(headers.status()) == INFORMATIONAL;
+ if ((isInformational || !endOfStream) && stream.isHeadersSent() || stream.isTrailersSent()) {
+ throw new IllegalStateException("Stream " + stream.id() + " sent too many headers EOS: " + endOfStream);
+ }
+ return isInformational;
+ }
+
@Override
public ChannelFuture writeHeaders(final ChannelHandlerContext ctx, final int streamId,
final Http2Headers headers, final int streamDependency, final short weight,
@@ -181,6 +192,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder {
// for this stream.
Http2RemoteFlowController flowController = flowController();
if (!endOfStream || !flowController.hasFlowControlled(stream)) {
+ boolean isInformational = validateHeadersSentState(stream, headers, connection.isServer(), endOfStream);
if (endOfStream) {
final Http2Stream finalStream = stream;
final ChannelFutureListener closeStreamLocalListener = new ChannelFutureListener() {
@@ -191,6 +203,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder {
};
promise = promise.unvoid().addListener(closeStreamLocalListener);
}
+
ChannelFuture future = frameWriter.writeHeaders(ctx, streamId, headers, streamDependency,
weight, exclusive, padding, endOfStream, promise);
// Writing headers may fail during the encode state if they violate HPACK limits.
@@ -198,7 +211,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder {
if (failureCause == null) {
// Synchronously set the headersSent flag to ensure that we do not subsequently write
// other headers containing pseudo-header fields.
- stream.headersSent();
+ stream.headersSent(isInformational);
} else {
lifecycleManager.onError(ctx, failureCause);
}
@@ -452,6 +465,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder {
@Override
public void write(ChannelHandlerContext ctx, int allowedBytes) {
+ boolean isInformational = validateHeadersSentState(stream, headers, connection.isServer(), endOfStream);
if (promise.isVoid()) {
promise = ctx.newPromise();
}
@@ -462,7 +476,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder {
// Writing headers may fail during the encode state if they violate HPACK limits.
Throwable failureCause = f.cause();
if (failureCause == null) {
- stream.headersSent();
+ stream.headersSent(isInformational);
} else {
lifecycleManager.onError(ctx, failureCause);
}
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
index 167087551b..3b654425cf 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java
@@ -130,15 +130,41 @@ public interface Http2Stream {
<V> V removeProperty(Http2Connection.PropertyKey key);
/**
- * Indicates that headers has been sent to the remote on this stream.
+ * Indicates that headers have been sent to the remote endpoint on this stream. The first call to this method would
+ * be for the initial headers (see {@link #isHeadersSent()}} and the second call would indicate the trailers
+ * (see {@link #isTrailersReceived()}).
+ * @param isInformational {@code true} if the headers contain an informational status code (for responses only).
*/
- Http2Stream headersSent();
+ Http2Stream headersSent(boolean isInformational);
/**
- * Indicates whether or not headers was sent to the remote endpoint.
+ * Indicates whether or not headers were sent to the remote endpoint.
*/
boolean isHeadersSent();
+ /**
+ * Indicates whether or not trailers were sent to the remote endpoint.
+ */
+ boolean isTrailersSent();
+
+ /**
+ * Indicates that headers have been received. The first call to this method would be for the initial headers
+ * (see {@link #isHeadersReceived()}} and the second call would indicate the trailers
+ * (see {@link #isTrailersReceived()}).
+ * @param isInformational {@code true} if the headers contain an informational status code (for responses only).
+ */
+ Http2Stream headersReceived(boolean isInformational);
+
+ /**
+ * Indicates whether or not the initial headers have been received.
+ */
+ boolean isHeadersReceived();
+
+ /**
+ * Indicates whether or not the trailers have been received.
+ */
+ boolean isTrailersReceived();
+
/**
* Indicates that a push promise was sent to the remote endpoint.
*/
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 6dc4266799..3fcf560eff 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
@@ -21,6 +21,7 @@ import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPromise;
import io.netty.channel.DefaultChannelPromise;
+import io.netty.handler.codec.http.HttpResponseStatus;
import junit.framework.AssertionFailedError;
import org.junit.Before;
import org.junit.Test;
@@ -50,10 +51,10 @@ import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.anyLong;
import static org.mockito.Mockito.anyShort;
-import static org.mockito.Mockito.eq;
-import static org.mockito.Mockito.isNull;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.isNull;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -67,6 +68,8 @@ public class DefaultHttp2ConnectionDecoderTest {
private static final int STREAM_ID = 3;
private static final int PUSH_STREAM_ID = 2;
private static final int STREAM_DEPENDENCY_ID = 5;
+ private static final int STATE_RECV_HEADERS = 1;
+ private static final int STATE_RECV_TRAILERS = 1 << 1;
private Http2ConnectionDecoder decoder;
private ChannelPromise promise;
@@ -122,11 +125,49 @@ public class DefaultHttp2ConnectionDecoderTest {
promise = new DefaultChannelPromise(channel);
+ final AtomicInteger headersReceivedState = new AtomicInteger();
when(channel.isActive()).thenReturn(true);
when(stream.id()).thenReturn(STREAM_ID);
when(stream.state()).thenReturn(OPEN);
when(stream.open(anyBoolean())).thenReturn(stream);
when(pushStream.id()).thenReturn(PUSH_STREAM_ID);
+ doAnswer(new Answer<Boolean>() {
+ @Override
+ public Boolean answer(InvocationOnMock in) throws Throwable {
+ return (headersReceivedState.get() & STATE_RECV_HEADERS) != 0;
+ }
+ }).when(stream).isHeadersReceived();
+ doAnswer(new Answer<Boolean>() {
+ @Override
+ public Boolean answer(InvocationOnMock in) throws Throwable {
+ return (headersReceivedState.get() & STATE_RECV_TRAILERS) != 0;
+ }
+ }).when(stream).isTrailersReceived();
+ doAnswer(new Answer<Http2Stream>() {
+ @Override
+ public Http2Stream answer(InvocationOnMock in) throws Throwable {
+ boolean isInformational = in.getArgument(0);
+ if (isInformational) {
+ return stream;
+ }
+ for (;;) {
+ int current = headersReceivedState.get();
+ int next = current;
+ if ((current & STATE_RECV_HEADERS) != 0) {
+ if ((current & STATE_RECV_TRAILERS) != 0) {
+ throw new IllegalStateException("already sent headers!");
+ }
+ next |= STATE_RECV_TRAILERS;
+ } else {
+ next |= STATE_RECV_HEADERS;
+ }
+ if (headersReceivedState.compareAndSet(current, next)) {
+ break;
+ }
+ }
+ return stream;
+ }
+ }).when(stream).headersReceived(anyBoolean());
doAnswer(new Answer<Http2Stream>() {
@Override
public Http2Stream answer(InvocationOnMock in) throws Throwable {
@@ -452,7 +493,65 @@ public class DefaultHttp2ConnectionDecoderTest {
eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false));
}
+ @Test(expected = Http2Exception.class)
+ public void trailersDoNotEndStreamThrows() throws Exception {
+ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false);
+ // Trailers must end the stream!
+ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false);
+ }
+
+ @Test(expected = Http2Exception.class)
+ public void tooManyHeadersEOSThrows() throws Exception {
+ tooManyHeaderThrows(true);
+ }
+
+ @Test(expected = Http2Exception.class)
+ public void tooManyHeadersNoEOSThrows() throws Exception {
+ tooManyHeaderThrows(false);
+ }
+
+ private void tooManyHeaderThrows(boolean eos) throws Exception {
+ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false);
+ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, true);
+ // We already received the trailers!
+ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, eos);
+ }
+
+ private static Http2Headers informationalHeaders() {
+ Http2Headers headers = new DefaultHttp2Headers();
+ headers.status(HttpResponseStatus.CONTINUE.codeAsText());
+ return headers;
+ }
+
+ @Test
+ public void infoHeadersAndTrailersAllowed() throws Exception {
+ infoHeadersAndTrailersAllowed(true, 1);
+ }
+
@Test
+ public void multipleInfoHeadersAndTrailersAllowed() throws Exception {
+ infoHeadersAndTrailersAllowed(true, 10);
+ }
+
+ @Test(expected = Http2Exception.class)
+ public void infoHeadersAndTrailersNoEOSThrows() throws Exception {
+ infoHeadersAndTrailersAllowed(false, 1);
+ }
+
+ @Test(expected = Http2Exception.class)
+ public void multipleInfoHeadersAndTrailersNoEOSThrows() throws Exception {
+ infoHeadersAndTrailersAllowed(false, 10);
+ }
+
+ private void infoHeadersAndTrailersAllowed(boolean eos, int infoHeaderCount) throws Exception {
+ for (int i = 0; i < infoHeaderCount; ++i) {
+ decode().onHeadersRead(ctx, STREAM_ID, informationalHeaders(), 0, false);
+ }
+ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false);
+ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, eos);
+ }
+
+ @Test()
public void headersReadForPromisedStreamShouldCloseStream() throws Exception {
when(stream.state()).thenReturn(RESERVED_REMOTE);
decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, true);
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java
index ac63f6dce9..4c5482ba9e 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java
@@ -24,6 +24,7 @@ import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPipeline;
import io.netty.channel.ChannelPromise;
import io.netty.channel.DefaultChannelPromise;
+import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.handler.codec.http2.Http2RemoteFlowController.FlowControlled;
import io.netty.util.concurrent.ImmediateEventExecutor;
import junit.framework.AssertionFailedError;
@@ -59,11 +60,12 @@ import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.anyLong;
import static org.mockito.Mockito.anyShort;
-import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
@@ -342,11 +344,208 @@ public class DefaultHttp2ConnectionEncoderTest {
eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise));
}
+ @Test
+ public void trailersDoNotEndStreamThrows() {
+ writeAllFlowControlledFrames();
+ final int streamId = 6;
+ ChannelPromise promise = newPromise();
+ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise);
+
+ ChannelPromise promise2 = newPromise();
+ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise2);
+ assertTrue(future.isDone());
+ assertFalse(future.isSuccess());
+
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise));
+ }
+
+ @Test
+ public void trailersDoNotEndStreamWithDataThrows() {
+ writeAllFlowControlledFrames();
+ final int streamId = 6;
+ ChannelPromise promise = newPromise();
+ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise);
+
+ Http2Stream stream = connection.stream(streamId);
+ when(remoteFlow.hasFlowControlled(eq(stream))).thenReturn(true);
+
+ ChannelPromise promise2 = newPromise();
+ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise2);
+ assertTrue(future.isDone());
+ assertFalse(future.isSuccess());
+
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise));
+ }
+
+ @Test
+ public void tooManyHeadersNoEOSThrows() {
+ tooManyHeadersThrows(false);
+ }
+
+ @Test
+ public void tooManyHeadersEOSThrows() {
+ tooManyHeadersThrows(true);
+ }
+
+ private void tooManyHeadersThrows(boolean eos) {
+ writeAllFlowControlledFrames();
+ final int streamId = 6;
+ ChannelPromise promise = newPromise();
+ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise);
+ ChannelPromise promise2 = newPromise();
+ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, true, promise2);
+
+ ChannelPromise promise3 = newPromise();
+ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, eos, promise3);
+ assertTrue(future.isDone());
+ assertFalse(future.isSuccess());
+
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise));
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(true), eq(promise2));
+ }
+
+ @Test
+ public void infoHeadersAndTrailersAllowed() throws Exception {
+ infoHeadersAndTrailers(true, 1);
+ }
+
+ @Test
+ public void multipleInfoHeadersAndTrailersAllowed() throws Exception {
+ infoHeadersAndTrailers(true, 10);
+ }
+
+ @Test
+ public void infoHeadersAndTrailersNoEOSThrows() throws Exception {
+ infoHeadersAndTrailers(false, 1);
+ }
+
+ @Test
+ public void multipleInfoHeadersAndTrailersNoEOSThrows() throws Exception {
+ infoHeadersAndTrailers(false, 10);
+ }
+
+ private void infoHeadersAndTrailers(boolean eos, int infoHeaderCount) {
+ writeAllFlowControlledFrames();
+ final int streamId = 6;
+ Http2Headers infoHeaders = informationalHeaders();
+ for (int i = 0; i < infoHeaderCount; ++i) {
+ encoder.writeHeaders(ctx, streamId, infoHeaders, 0, false, newPromise());
+ }
+ ChannelPromise promise2 = newPromise();
+ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise2);
+
+ ChannelPromise promise3 = newPromise();
+ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, eos, promise3);
+ assertTrue(future.isDone());
+ assertEquals(eos, future.isSuccess());
+
+ verify(writer, times(infoHeaderCount)).writeHeaders(eq(ctx), eq(streamId), eq(infoHeaders), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), any(ChannelPromise.class));
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise2));
+ if (eos) {
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(true), eq(promise3));
+ }
+ }
+
+ private static Http2Headers informationalHeaders() {
+ Http2Headers headers = new DefaultHttp2Headers();
+ headers.status(HttpResponseStatus.CONTINUE.codeAsText());
+ return headers;
+ }
+
+ @Test
+ public void tooManyHeadersWithDataNoEOSThrows() {
+ tooManyHeadersWithDataThrows(false);
+ }
+
+ @Test
+ public void tooManyHeadersWithDataEOSThrows() {
+ tooManyHeadersWithDataThrows(true);
+ }
+
+ private void tooManyHeadersWithDataThrows(boolean eos) {
+ writeAllFlowControlledFrames();
+ final int streamId = 6;
+ ChannelPromise promise = newPromise();
+ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise);
+
+ Http2Stream stream = connection.stream(streamId);
+ when(remoteFlow.hasFlowControlled(eq(stream))).thenReturn(true);
+
+ ChannelPromise promise2 = newPromise();
+ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, true, promise2);
+
+ ChannelPromise promise3 = newPromise();
+ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, eos, promise3);
+ assertTrue(future.isDone());
+ assertFalse(future.isSuccess());
+
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise));
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(true), eq(promise2));
+ }
+
+ @Test
+ public void infoHeadersAndTrailersWithDataAllowed() {
+ infoHeadersAndTrailersWithData(true, 1);
+ }
+
+ @Test
+ public void multipleInfoHeadersAndTrailersWithDataAllowed() {
+ infoHeadersAndTrailersWithData(true, 10);
+ }
+
+ @Test
+ public void infoHeadersAndTrailersWithDataNoEOSThrows() {
+ infoHeadersAndTrailersWithData(false, 1);
+ }
+
+ @Test
+ public void multipleInfoHeadersAndTrailersWithDataNoEOSThrows() {
+ infoHeadersAndTrailersWithData(false, 10);
+ }
+
+ private void infoHeadersAndTrailersWithData(boolean eos, int infoHeaderCount) {
+ writeAllFlowControlledFrames();
+ final int streamId = 6;
+ Http2Headers infoHeaders = informationalHeaders();
+ for (int i = 0; i < infoHeaderCount; ++i) {
+ encoder.writeHeaders(ctx, streamId, infoHeaders, 0, false, newPromise());
+ }
+
+ Http2Stream stream = connection.stream(streamId);
+ when(remoteFlow.hasFlowControlled(eq(stream))).thenReturn(true);
+
+ ChannelPromise promise2 = newPromise();
+ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise2);
+
+ ChannelPromise promise3 = newPromise();
+ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, eos, promise3);
+ assertTrue(future.isDone());
+ assertEquals(eos, future.isSuccess());
+
+ verify(writer, times(infoHeaderCount)).writeHeaders(eq(ctx), eq(streamId), eq(infoHeaders), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), any(ChannelPromise.class));
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise2));
+ if (eos) {
+ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0),
+ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(true), eq(promise3));
+ }
+ }
+
@Test
public void pushPromiseWriteAfterGoAwayReceivedShouldFail() throws Exception {
createStream(STREAM_ID, false);
goAwayReceived(0);
- ChannelFuture future = encoder.writePushPromise(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0,
+ ChannelFuture future = encoder.writePushPromise(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0,
newPromise());
assertTrue(future.isDone());
assertFalse(future.isSuccess());
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java
index 28a5c5b44b..33393afc0a 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java
@@ -360,49 +360,6 @@ public class InboundHttp2ToHttpAdapterTest {
}
}
- @Test
- public void clientRequestMultipleHeaders() throws Exception {
- boostrapEnv(1, 1, 1);
- // writeHeaders will implicitly add an END_HEADERS tag each time and so this test does not follow the HTTP
- // message flow. We currently accept this message flow and just add the second headers to the trailing headers.
- final String text = "";
- final ByteBuf content = Unpooled.copiedBuffer(text.getBytes());
- final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
- "/some/path/resource2", content, true);
- try {
- HttpHeaders httpHeaders = request.headers();
- httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 3);
- httpHeaders.setInt(HttpHeaderNames.CONTENT_LENGTH, text.length());
- httpHeaders.setShort(HttpConversionUtil.ExtensionHeaderNames.STREAM_WEIGHT.text(), (short) 16);
- HttpHeaders trailingHeaders = request.trailingHeaders();
- trailingHeaders.set(of("FoO"), of("goo"));
- trailingHeaders.set(of("foO2"), of("goo2"));
- trailingHeaders.add(of("fOo2"), of("goo3"));
- final Http2Headers http2Headers = new DefaultHttp2Headers().method(new AsciiString("GET")).path(
- new AsciiString("/some/path/resource2"));
- final Http2Headers http2Headers2 = new DefaultHttp2Headers()
- .set(new AsciiString("foo"), new AsciiString("goo"))
- .set(new AsciiString("foo2"), new AsciiString("goo2"))
- .add(new AsciiString("foo2"), new AsciiString("goo3"));
- runInChannel(clientChannel, new Http2Runnable() {
- @Override
- public void run() throws Http2Exception {
- clientHandler.encoder().writeHeaders(ctxClient(), 3, http2Headers, 0, false, newPromiseClient());
- clientHandler.encoder().writeHeaders(ctxClient(), 3, http2Headers2, 0, false, newPromiseClient());
- clientHandler.encoder().writeData(ctxClient(), 3, content.retain(), 0, true, newPromiseClient());
- clientChannel.flush();
- }
- });
- awaitRequests();
- ArgumentCaptor<FullHttpMessage> requestCaptor = ArgumentCaptor.forClass(FullHttpMessage.class);
- verify(serverListener).messageReceived(requestCaptor.capture());
- capturedRequests = requestCaptor.getAllValues();
- assertEquals(request, capturedRequests.get(0));
- } finally {
- request.release();
- }
- }
-
@Test
public void clientRequestTrailingHeaders() throws Exception {
boostrapEnv(1, 1, 1);
--
2.23.0

547
CVE-2021-21295.patch Normal file
View File

@ -0,0 +1,547 @@
From 89c241e3b1795ff257af4ad6eadc616cb2fb3dc4 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Tue, 9 Mar 2021 08:20:09 +0100
Subject: [PATCH] Merge pull request from GHSA-wm47-8v5p-wjpj
Motivation:
As stated by https://tools.ietf.org/html/rfc7540#section-8.1.2.6 we should report a stream error if the content-length does not match the sum of all data frames.
Modifications:
- Verify that the sum of data frames match if a content-length header was send.
- Handle multiple content-length headers and also handle negative values
- Add io.netty.http2.validateContentLength system property which allows to disable the more strict validation
- Add unit tests
Result:
Correctly handle the case when the content-length header was included but not match what is send and also when content-length header is invalid
---
.../handler/codec/http/HttpObjectDecoder.java | 48 +------
.../io/netty/handler/codec/http/HttpUtil.java | 87 ++++++++++++
.../http2/DefaultHttp2ConnectionDecoder.java | 100 ++++++++++++--
.../DefaultHttp2ConnectionDecoderTest.java | 128 ++++++++++++++++++
4 files changed, 313 insertions(+), 50 deletions(-)
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
index b52e36ac92..82b0c365c1 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
@@ -16,7 +16,6 @@
package io.netty.handler.codec.http;
import static io.netty.util.internal.ObjectUtil.checkPositive;
-import static io.netty.util.internal.StringUtil.COMMA;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
@@ -638,49 +637,16 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
value = null;
List<String> contentLengthFields = headers.getAll(HttpHeaderNames.CONTENT_LENGTH);
-
if (!contentLengthFields.isEmpty()) {
+ HttpVersion version = message.protocolVersion();
+ boolean isHttp10OrEarlier = version.majorVersion() < 1 || (version.majorVersion() == 1
+ && version.minorVersion() == 0);
// Guard against multiple Content-Length headers as stated in
// https://tools.ietf.org/html/rfc7230#section-3.3.2:
- //
- // If a message is received that has multiple Content-Length header
- // fields with field-values consisting of the same decimal value, or a
- // single Content-Length header field with a field value containing a
- // list of identical decimal values (e.g., "Content-Length: 42, 42"),
- // indicating that duplicate Content-Length header fields have been
- // generated or combined by an upstream message processor, then the
- // recipient MUST either reject the message as invalid or replace the
- // duplicated field-values with a single valid Content-Length field
- // containing that decimal value prior to determining the message body
- // length or forwarding the message.
- boolean multipleContentLengths =
- contentLengthFields.size() > 1 || contentLengthFields.get(0).indexOf(COMMA) >= 0;
- if (multipleContentLengths && message.protocolVersion() == HttpVersion.HTTP_1_1) {
- if (allowDuplicateContentLengths) {
- // Find and enforce that all Content-Length values are the same
- String firstValue = null;
- for (String field : contentLengthFields) {
- String[] tokens = COMMA_PATTERN.split(field, -1);
- for (String token : tokens) {
- String trimmed = token.trim();
- if (firstValue == null) {
- firstValue = trimmed;
- } else if (!trimmed.equals(firstValue)) {
- throw new IllegalArgumentException(
- "Multiple Content-Length values found: " + contentLengthFields);
- }
- }
- }
- // Replace the duplicated field-values with a single valid Content-Length field
- headers.set(HttpHeaderNames.CONTENT_LENGTH, firstValue);
- contentLength = Long.parseLong(firstValue);
- } else {
- // Reject the message as invalid
- throw new IllegalArgumentException(
- "Multiple Content-Length values found: " + contentLengthFields);
- }
- } else {
- contentLength = Long.parseLong(contentLengthFields.get(0));
+ contentLength = HttpUtil.normalizeAndGetContentLength(contentLengthFields,
+ isHttp10OrEarlier, allowDuplicateContentLengths);
+ if (contentLength != -1) {
+ headers.set(HttpHeaderNames.CONTENT_LENGTH, contentLength);
}
}
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java
index 035cd754cf..3d1c57f5ac 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java
@@ -25,6 +25,11 @@ import java.nio.charset.UnsupportedCharsetException;
import java.util.Iterator;
import java.util.List;
+import io.netty.handler.codec.Headers;
+import io.netty.util.internal.UnstableApi;
+
+import static io.netty.util.internal.StringUtil.COMMA;
+
/**
* Utility methods useful in the HTTP context.
*/
@@ -40,6 +45,7 @@ public final class HttpUtil {
static final EmptyHttpHeaders EMPTY_HEADERS = new EmptyHttpHeaders();
private static final AsciiString CHARSET_EQUALS = AsciiString.of(HttpHeaderValues.CHARSET + "=");
private static final AsciiString SEMICOLON = AsciiString.of(";");
+ private static final String COMMA_STRING = String.valueOf(COMMA);
private HttpUtil() { }
@@ -519,4 +525,85 @@ public final class HttpUtil {
return contentTypeValue.length() > 0 ? contentTypeValue : null;
}
}
+
+ /**
+ * Validates, and optionally extracts the content length from headers. This method is not intended for
+ * general use, but is here to be shared between HTTP/1 and HTTP/2 parsing.
+ *
+ * @param contentLengthFields the content-length header fields.
+ * @param isHttp10OrEarlier {@code true} if we are handling HTTP/1.0 or earlier
+ * @param allowDuplicateContentLengths {@code true} if multiple, identical-value content lengths should be allowed.
+ * @return the normalized content length from the headers or {@code -1} if the fields were empty.
+ * @throws IllegalArgumentException if the content-length fields are not valid
+ */
+ @UnstableApi
+ public static long normalizeAndGetContentLength(
+ List<? extends CharSequence> contentLengthFields, boolean isHttp10OrEarlier,
+ boolean allowDuplicateContentLengths) {
+ if (contentLengthFields.isEmpty()) {
+ return -1;
+ }
+
+ // Guard against multiple Content-Length headers as stated in
+ // https://tools.ietf.org/html/rfc7230#section-3.3.2:
+ //
+ // If a message is received that has multiple Content-Length header
+ // fields with field-values consisting of the same decimal value, or a
+ // single Content-Length header field with a field value containing a
+ // list of identical decimal values (e.g., "Content-Length: 42, 42"),
+ // indicating that duplicate Content-Length header fields have been
+ // generated or combined by an upstream message processor, then the
+ // recipient MUST either reject the message as invalid or replace the
+ // duplicated field-values with a single valid Content-Length field
+ // containing that decimal value prior to determining the message body
+ // length or forwarding the message.
+ String firstField = contentLengthFields.get(0).toString();
+ boolean multipleContentLengths =
+ contentLengthFields.size() > 1 || firstField.indexOf(COMMA) >= 0;
+
+ if (multipleContentLengths && !isHttp10OrEarlier) {
+ if (allowDuplicateContentLengths) {
+ // Find and enforce that all Content-Length values are the same
+ String firstValue = null;
+ for (CharSequence field : contentLengthFields) {
+ String[] tokens = field.toString().split(COMMA_STRING, -1);
+ for (String token : tokens) {
+ String trimmed = token.trim();
+ if (firstValue == null) {
+ firstValue = trimmed;
+ } else if (!trimmed.equals(firstValue)) {
+ throw new IllegalArgumentException(
+ "Multiple Content-Length values found: " + contentLengthFields);
+ }
+ }
+ }
+ // Replace the duplicated field-values with a single valid Content-Length field
+ firstField = firstValue;
+ } else {
+ // Reject the message as invalid
+ throw new IllegalArgumentException(
+ "Multiple Content-Length values found: " + contentLengthFields);
+ }
+ }
+ // Ensure we not allow sign as part of the content-length:
+ // See https://github.com/squid-cache/squid/security/advisories/GHSA-qf3v-rc95-96j5
+ if (!Character.isDigit(firstField.charAt(0))) {
+ // Reject the message as invalid
+ throw new IllegalArgumentException(
+ "Content-Length value is not a number: " + firstField);
+ }
+ try {
+ final long value = Long.parseLong(firstField);
+ if (value < 0) {
+ // Reject the message as invalid
+ throw new IllegalArgumentException(
+ "Content-Length value must be >=0: " + value);
+ }
+ return value;
+ } catch (NumberFormatException e) {
+ // Reject the message as invalid
+ throw new IllegalArgumentException(
+ "Content-Length value is not a number: " + firstField, e);
+ }
+ }
}
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 4027978651..f04a0b5a69 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
@@ -16,8 +16,11 @@ package io.netty.handler.codec.http2;
import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpStatusClass;
+import io.netty.handler.codec.http.HttpUtil;
import io.netty.handler.codec.http2.Http2Connection.Endpoint;
+import io.netty.util.internal.SystemPropertyUtil;
import io.netty.util.internal.UnstableApi;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;
@@ -49,6 +52,8 @@ import static java.lang.Math.min;
*/
@UnstableApi
public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
+ private static final boolean VALIDATE_CONTENT_LENGTH =
+ SystemPropertyUtil.getBoolean("io.netty.http2.validateContentLength", true);
private static final InternalLogger logger = InternalLoggerFactory.getInstance(DefaultHttp2ConnectionDecoder.class);
private Http2FrameListener internalFrameListener = new PrefaceFrameListener();
private final Http2Connection connection;
@@ -57,6 +62,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
private final Http2FrameReader frameReader;
private Http2FrameListener listener;
private final Http2PromisedRequestVerifier requestVerifier;
+ private final Http2Connection.PropertyKey contentLengthKey;
public DefaultHttp2ConnectionDecoder(Http2Connection connection,
Http2ConnectionEncoder encoder,
@@ -69,6 +75,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
Http2FrameReader frameReader,
Http2PromisedRequestVerifier requestVerifier) {
this.connection = checkNotNull(connection, "connection");
+ contentLengthKey = this.connection.newKey();
this.frameReader = checkNotNull(frameReader, "frameReader");
this.encoder = checkNotNull(encoder, "encoder");
this.requestVerifier = checkNotNull(requestVerifier, "requestVerifier");
@@ -171,6 +178,23 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
listener.onUnknownFrame(ctx, frameType, streamId, flags, payload);
}
+ // See https://tools.ietf.org/html/rfc7540#section-8.1.2.6
+ private void verifyContentLength(Http2Stream stream, int data, boolean isEnd) throws Http2Exception {
+ if (!VALIDATE_CONTENT_LENGTH) {
+ return;
+ }
+ ContentLength contentLength = stream.getProperty(contentLengthKey);
+ if (contentLength != null) {
+ try {
+ contentLength.increaseReceivedBytes(connection.isServer(), stream.id(), data, isEnd);
+ } finally {
+ if (isEnd) {
+ stream.removeProperty(contentLengthKey);
+ }
+ }
+ }
+ }
+
/**
* Handles all inbound frames from the network.
*/
@@ -180,7 +204,8 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
boolean endOfStream) throws Http2Exception {
Http2Stream stream = connection.stream(streamId);
Http2LocalFlowController flowController = flowController();
- int bytesToReturn = data.readableBytes() + padding;
+ int readable = data.readableBytes();
+ int bytesToReturn = readable + padding;
final boolean shouldIgnore;
try {
@@ -207,7 +232,6 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
// All bytes have been consumed.
return bytesToReturn;
}
-
Http2Exception error = null;
switch (stream.state()) {
case OPEN:
@@ -235,6 +259,8 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
throw error;
}
+ verifyContentLength(stream, readable, endOfStream);
+
// Call back the application and retrieve the number of bytes that have been
// immediately processed.
bytesToReturn = listener.onDataRead(ctx, streamId, data, padding, endOfStream);
@@ -315,14 +341,34 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
stream.state());
}
- stream.headersReceived(isInformational);
- encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive);
-
- listener.onHeadersRead(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream);
+ if (!stream.isHeadersReceived()) {
+ // extract the content-length header
+ List<? extends CharSequence> contentLength = headers.getAll(HttpHeaderNames.CONTENT_LENGTH);
+ if (contentLength != null && !contentLength.isEmpty()) {
+ try {
+ long cLength = HttpUtil.normalizeAndGetContentLength(contentLength, false, true);
+ if (cLength != -1) {
+ headers.setLong(HttpHeaderNames.CONTENT_LENGTH, cLength);
+ stream.setProperty(contentLengthKey, new ContentLength(cLength));
+ }
+ } catch (IllegalArgumentException e) {
+ throw streamError(stream.id(), PROTOCOL_ERROR,
+ "Multiple content-length headers received", e);
+ }
+ }
+ }
- // If the headers completes this stream, close it.
- if (endOfStream) {
- lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture());
+ stream.headersReceived(isInformational);
+ try {
+ verifyContentLength(stream, 0, endOfStream);
+ encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive);
+ listener.onHeadersRead(ctx, streamId, headers, streamDependency,
+ weight, exclusive, padding, endOfStream);
+ } finally {
+ // If the headers completes this stream, close it.
+ if (endOfStream) {
+ lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture());
+ }
}
}
@@ -673,4 +719,40 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
onUnknownFrame0(ctx, frameType, streamId, flags, payload);
}
}
+
+ private static final class ContentLength {
+ private final long expected;
+ private long seen;
+
+ ContentLength(long expected) {
+ this.expected = expected;
+ }
+
+ void increaseReceivedBytes(boolean server, int streamId, int bytes, boolean isEnd) throws Http2Exception {
+ seen += bytes;
+ // Check for overflow
+ if (seen < 0) {
+ throw streamError(streamId, PROTOCOL_ERROR,
+ "Received amount of data did overflow and so not match content-length header %d", expected);
+ }
+ // Check if we received more data then what was advertised via the content-length header.
+ if (seen > expected) {
+ throw streamError(streamId, PROTOCOL_ERROR,
+ "Received amount of data %d does not match content-length header %d", seen, expected);
+ }
+
+ if (isEnd) {
+ if (seen == 0 && !server) {
+ // This may be a response to a HEAD request, let's just allow it.
+ return;
+ }
+
+ // Check that we really saw what was told via the content-length header.
+ if (expected > seen) {
+ throw streamError(streamId, PROTOCOL_ERROR,
+ "Received amount of data %d does not match content-length header %d", seen, expected);
+ }
+ }
+ }
+ }
}
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 3fcf560eff..467c6a9904 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
@@ -21,17 +21,21 @@ import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPromise;
import io.netty.channel.DefaultChannelPromise;
+import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpResponseStatus;
import junit.framework.AssertionFailedError;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import static io.netty.buffer.Unpooled.EMPTY_BUFFER;
@@ -130,6 +134,21 @@ public class DefaultHttp2ConnectionDecoderTest {
when(stream.id()).thenReturn(STREAM_ID);
when(stream.state()).thenReturn(OPEN);
when(stream.open(anyBoolean())).thenReturn(stream);
+
+ final Map<Object, Object> properties = new IdentityHashMap<Object, Object>();
+ when(stream.getProperty(ArgumentMatchers.<Http2Connection.PropertyKey>any())).thenAnswer(new Answer<Object>() {
+ @Override
+ public Object answer(InvocationOnMock invocationOnMock) {
+ return properties.get(invocationOnMock.getArgument(0));
+ }
+ });
+ when(stream.setProperty(ArgumentMatchers.<Http2Connection.PropertyKey>any(), any())).then(new Answer<Object>() {
+ @Override
+ public Object answer(InvocationOnMock invocationOnMock) {
+ return properties.put(invocationOnMock.getArgument(0), invocationOnMock.getArgument(1));
+ }
+ });
+
when(pushStream.id()).thenReturn(PUSH_STREAM_ID);
doAnswer(new Answer<Boolean>() {
@Override
@@ -751,6 +770,115 @@ public class DefaultHttp2ConnectionDecoderTest {
verify(listener).onGoAwayRead(eq(ctx), eq(1), eq(2L), eq(EMPTY_BUFFER));
}
+ @Test(expected = Http2Exception.StreamException.class)
+ public void dataContentLengthMissmatch() throws Exception {
+ dataContentLengthInvalid(false);
+ }
+
+ @Test(expected = Http2Exception.StreamException.class)
+ public void dataContentLengthInvalid() throws Exception {
+ dataContentLengthInvalid(true);
+ }
+
+ private void dataContentLengthInvalid(boolean negative) throws Exception {
+ final ByteBuf data = dummyData();
+ int padding = 10;
+ int processedBytes = data.readableBytes() + padding;
+ mockFlowControl(processedBytes);
+ try {
+ decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers()
+ .setLong(HttpHeaderNames.CONTENT_LENGTH, negative ? -1L : 1L), padding, false);
+ decode().onDataRead(ctx, STREAM_ID, data, padding, true);
+ verify(localFlow).receiveFlowControlledFrame(eq(stream), eq(data), eq(padding), eq(true));
+ verify(localFlow).consumeBytes(eq(stream), eq(processedBytes));
+
+ verify(listener, times(1)).onHeadersRead(eq(ctx), anyInt(),
+ any(Http2Headers.class), eq(0), eq(DEFAULT_PRIORITY_WEIGHT), eq(false),
+ eq(padding), eq(false));
+ // Verify that the event was absorbed and not propagated to the observer.
+ verify(listener, never()).onDataRead(eq(ctx), anyInt(), any(ByteBuf.class), anyInt(), anyBoolean());
+ } finally {
+ data.release();
+ }
+ }
+
+ @Test(expected = Http2Exception.StreamException.class)
+ public void headersContentLengthPositiveSign() throws Exception {
+ headersContentLengthSign("+1");
+ }
+
+ @Test(expected = Http2Exception.StreamException.class)
+ public void headersContentLengthNegativeSign() throws Exception {
+ headersContentLengthSign("-1");
+ }
+
+ private void headersContentLengthSign(String length) throws Exception {
+ int padding = 10;
+ when(connection.isServer()).thenReturn(true);
+ decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers()
+ .set(HttpHeaderNames.CONTENT_LENGTH, length), padding, false);
+
+ // Verify that the event was absorbed and not propagated to the observer.
+ verify(listener, never()).onHeadersRead(eq(ctx), anyInt(),
+ any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
+ }
+
+ @Test(expected = Http2Exception.StreamException.class)
+ public void headersContentLengthMissmatch() throws Exception {
+ headersContentLength(false);
+ }
+
+ @Test(expected = Http2Exception.StreamException.class)
+ public void headersContentLengthInvalid() throws Exception {
+ headersContentLength(true);
+ }
+
+ private void headersContentLength(boolean negative) throws Exception {
+ int padding = 10;
+ when(connection.isServer()).thenReturn(true);
+ decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers()
+ .setLong(HttpHeaderNames.CONTENT_LENGTH, negative ? -1L : 1L), padding, true);
+
+ // Verify that the event was absorbed and not propagated to the observer.
+ verify(listener, never()).onHeadersRead(eq(ctx), anyInt(),
+ any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
+ }
+
+ @Test
+ public void multipleHeadersContentLengthSame() throws Exception {
+ multipleHeadersContentLength(true);
+ }
+
+ @Test(expected = Http2Exception.StreamException.class)
+ public void multipleHeadersContentLengthDifferent() throws Exception {
+ multipleHeadersContentLength(false);
+ }
+
+ private void multipleHeadersContentLength(boolean same) throws Exception {
+ int padding = 10;
+ when(connection.isServer()).thenReturn(true);
+ Http2Headers headers = new DefaultHttp2Headers();
+ if (same) {
+ headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
+ headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
+ } else {
+ headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
+ headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 1);
+ }
+
+ decode().onHeadersRead(ctx, STREAM_ID, headers, padding, true);
+
+ if (same) {
+ verify(listener, times(1)).onHeadersRead(eq(ctx), anyInt(),
+ any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
+ assertEquals(1, headers.getAll(HttpHeaderNames.CONTENT_LENGTH).size());
+ } else {
+ // Verify that the event was absorbed and not propagated to the observer.
+ verify(listener, never()).onHeadersRead(eq(ctx), anyInt(),
+ any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
+ }
+ }
+
private static ByteBuf dummyData() {
// The buffer is purposely 8 bytes so it will even work for a ping frame.
return wrappedBuffer("abcdefgh".getBytes(UTF_8));
--
2.23.0

51
CVE-2021-21409.patch Normal file
View File

@ -0,0 +1,51 @@
From b0fa4d5aab4215f3c22ce6123dd8dd5f38dc0432 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Tue, 30 Mar 2021 09:40:47 +0200
Subject: [PATCH] Merge pull request from GHSA-f256-j965-7f32
Motivation:
We also need to ensure that all the header validation is done when a single header with the endStream flag is received
Modifications:
- Adjust code to always enforce the validation
- Add more unit tests
Result:
Always correctly validate
---
.../handler/codec/http2/DefaultHttp2ConnectionDecoder.java | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
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 f04a0b5a69..097ac8cdad 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
@@ -300,10 +300,13 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
short weight, boolean exclusive, int padding, boolean endOfStream) throws Http2Exception {
Http2Stream stream = connection.stream(streamId);
boolean allowHalfClosedRemote = false;
+ boolean isTrailers = false;
if (stream == null && !connection.streamMayHaveExisted(streamId)) {
stream = connection.remote().createStream(streamId, endOfStream);
// Allow the state to be HALF_CLOSE_REMOTE if we're creating it in that state.
allowHalfClosedRemote = stream.state() == HALF_CLOSED_REMOTE;
+ } else if (stream != null) {
+ isTrailers = stream.isHeadersReceived();
}
if (shouldIgnoreHeadersOrDataFrame(ctx, streamId, stream, "HEADERS")) {
@@ -341,7 +344,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
stream.state());
}
- if (!stream.isHeadersReceived()) {
+ if (!isTrailers) {
// extract the content-length header
List<? extends CharSequence> contentLength = headers.getAll(HttpHeaderNames.CONTENT_LENGTH);
if (contentLength != null && !contentLength.isEmpty()) {
--
2.23.0

View File

@ -2,7 +2,7 @@
Name: netty Name: netty
Version: 4.1.13 Version: 4.1.13
Release: 11 Release: 12
Summary: Asynchronous event-driven network application Java framework Summary: Asynchronous event-driven network application Java framework
License: ASL 2.0 License: ASL 2.0
URL: https://netty.io/ URL: https://netty.io/
@ -18,6 +18,12 @@ Patch0006: CVE-2019-20445-2.patch
Patch0007: CVE-2019-20445-3.patch Patch0007: CVE-2019-20445-3.patch
Patch0008: CVE-2020-11612.patch Patch0008: CVE-2020-11612.patch
Patch0009: CVE-2021-21290.patch Patch0009: CVE-2021-21290.patch
Patch0010: CVE-2021-21295-pre1.patch
Patch0011: CVE-2021-21295-pre2.patch
Patch0012: CVE-2021-21295-pre3.patch
Patch0013: CVE-2021-21295-pre4.patch
Patch0014: CVE-2021-21295.patch
Patch0015: CVE-2021-21409.patch
BuildRequires: maven-local mvn(ant-contrib:ant-contrib) BuildRequires: maven-local mvn(ant-contrib:ant-contrib)
BuildRequires: mvn(com.jcraft:jzlib) mvn(commons-logging:commons-logging) BuildRequires: mvn(com.jcraft:jzlib) mvn(commons-logging:commons-logging)
@ -139,6 +145,9 @@ export CFLAGS="$RPM_OPT_FLAGS" LDFLAGS="$RPM_LD_FLAGS"
%changelog %changelog
* Tue Apr 06 2021 wangxiao <wangxiao65@huawei.com> - 4.1.13-12
- Fix CVE-2021-21295 CVE-2021-21409
* Tue Mar 09 2021 wangyue <wangyue92@huawei.com> - 4.1.13-11 * Tue Mar 09 2021 wangyue <wangyue92@huawei.com> - 4.1.13-11
- fix CVE-2021-21290 - fix CVE-2021-21290