From db2f60c870cf24fca28d15ebac95dbfded912636 Mon Sep 17 00:00:00 2001 From: Boris Unckel Date: Fri, 23 Apr 2021 08:08:28 +0200 Subject: [PATCH] Utilize i.n.u.internal.ObjectUtil to assert Preconditions (codec-http) (#11170) (#11187) Motivation: NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx. Modifications: * import static relevant checks * Replace manual checks with ObjectUtil methods Result: All checks needed are done with ObjectUtil, some exception texts are improved. Fixes #11170 --- .../io/netty/handler/codec/http/HttpMethod.java | 6 ++---- .../io/netty/handler/codec/http/HttpUtil.java | 8 ++------ .../io/netty/handler/codec/http/HttpVersion.java | 15 +++------------ .../handler/codec/http/cookie/DefaultCookie.java | 7 ++----- .../codec/http/cors/CorsConfigBuilder.java | 7 +++---- .../codec/http/multipart/AbstractHttpData.java | 8 +++----- .../WebSocketClientExtensionHandler.java | 9 +++------ .../WebSocketServerExtensionHandler.java | 9 +++------ .../io/netty/handler/codec/rtsp/RtspMethods.java | 11 +++-------- .../netty/handler/codec/spdy/SpdyCodecUtil.java | 12 +++++------- .../codec/spdy/SpdyHeaderBlockJZlibEncoder.java | 6 +++--- .../codec/spdy/SpdyHeaderBlockZlibEncoder.java | 6 +++--- 12 files changed, 35 insertions(+), 69 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpMethod.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpMethod.java index cffb9a73fa..ee9fb267f7 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpMethod.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpMethod.java @@ -19,6 +19,7 @@ import io.netty.util.AsciiString; import static io.netty.util.internal.MathUtil.findNextPositivePowerOfTwo; import static io.netty.util.internal.ObjectUtil.checkNotNull; +import static io.netty.util.internal.ObjectUtil.checkNonEmptyAfterTrim; /** * The request method of HTTP or its derived protocols, such as @@ -120,10 +121,7 @@ public class HttpMethod implements Comparable { * ICAP */ public HttpMethod(String name) { - name = checkNotNull(name, "name").trim(); - if (name.isEmpty()) { - throw new IllegalArgumentException("empty name"); - } + name = checkNonEmptyAfterTrim(name, "name"); for (int i = 0; i < name.length(); i ++) { char c = name.charAt(i); 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 49ed25510f..c4d62286f7 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 @@ -32,6 +32,7 @@ import io.netty.util.internal.ObjectUtil; import io.netty.util.internal.UnstableApi; import static io.netty.util.internal.StringUtil.COMMA; +import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; /** * Utility methods useful in the HTTP context. @@ -604,12 +605,7 @@ public final class HttpUtil { } 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; + return checkPositiveOrZero(value, "Content-Length value"); } catch (NumberFormatException e) { // Reject the message as invalid throw new IllegalArgumentException( 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 09342b5f18..4ae3964620 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 @@ -16,6 +16,7 @@ package io.netty.handler.codec.http; import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; +import static io.netty.util.internal.ObjectUtil.checkNonEmptyAfterTrim; import io.netty.buffer.ByteBuf; import io.netty.util.CharsetUtil; @@ -108,12 +109,7 @@ public class HttpVersion implements Comparable { * the {@code "Connection"} header is set to {@code "close"} explicitly. */ public HttpVersion(String text, boolean keepAliveDefault) { - ObjectUtil.checkNotNull(text, "text"); - - text = text.trim().toUpperCase(); - if (text.isEmpty()) { - throw new IllegalArgumentException("empty text"); - } + text = checkNonEmptyAfterTrim(text, "text").toUpperCase(); Matcher m = VERSION_PATTERN.matcher(text); if (!m.matches()) { @@ -148,12 +144,7 @@ public class HttpVersion implements Comparable { private HttpVersion( String protocolName, int majorVersion, int minorVersion, boolean keepAliveDefault, boolean bytes) { - ObjectUtil.checkNotNull(protocolName, "protocolName"); - - protocolName = protocolName.trim().toUpperCase(); - if (protocolName.isEmpty()) { - throw new IllegalArgumentException("empty protocolName"); - } + protocolName = checkNonEmptyAfterTrim(protocolName, "protocolName").toUpperCase(); for (int i = 0; i < protocolName.length(); i ++) { if (Character.isISOControl(protocolName.charAt(i)) || diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cookie/DefaultCookie.java b/codec-http/src/main/java/io/netty/handler/codec/http/cookie/DefaultCookie.java index 22e66f5942..f40a0277d5 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/cookie/DefaultCookie.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/cookie/DefaultCookie.java @@ -20,6 +20,7 @@ import io.netty.handler.codec.http.cookie.CookieHeaderNames.SameSite; import static io.netty.handler.codec.http.cookie.CookieUtil.stringBuilder; import static io.netty.handler.codec.http.cookie.CookieUtil.validateAttributeValue; import static io.netty.util.internal.ObjectUtil.checkNotNull; +import static io.netty.util.internal.ObjectUtil.checkNonEmptyAfterTrim; /** * The default {@link Cookie} implementation. @@ -40,11 +41,7 @@ public class DefaultCookie implements Cookie { * Creates a new cookie with the specified name and value. */ public DefaultCookie(String name, String value) { - name = checkNotNull(name, "name").trim(); - if (name.isEmpty()) { - throw new IllegalArgumentException("empty name"); - } - this.name = name; + this.name = checkNonEmptyAfterTrim(name, "name"); setValue(value); } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java index 115d568f75..bbadfe99d4 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java @@ -15,6 +15,8 @@ */ package io.netty.handler.codec.http.cors; +import static io.netty.util.internal.ObjectUtil.checkNotNullWithIAE; + import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpMethod; @@ -378,10 +380,7 @@ public final class CorsConfigBuilder { * @param value the value that will be returned when the call method is invoked. */ private ConstantValueGenerator(final Object value) { - if (value == null) { - throw new IllegalArgumentException("value must not be null"); - } - this.value = value; + this.value = checkNotNullWithIAE(value, "value"); } @Override diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractHttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractHttpData.java index 3e084b0131..39cbde8ffd 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractHttpData.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractHttpData.java @@ -15,6 +15,8 @@ */ package io.netty.handler.codec.http.multipart; +import static io.netty.util.internal.ObjectUtil.checkNonEmpty; + import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelException; import io.netty.handler.codec.http.HttpConstants; @@ -46,11 +48,7 @@ public abstract class AbstractHttpData extends AbstractReferenceCounted implemen name = REPLACE_PATTERN.matcher(name).replaceAll(" "); name = STRIP_PATTERN.matcher(name).replaceAll(""); - if (name.isEmpty()) { - throw new IllegalArgumentException("empty name"); - } - - this.name = name; + this.name = checkNonEmpty(name, "name"); if (charset != null) { setCharset(charset); } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketClientExtensionHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketClientExtensionHandler.java index 47073a7b23..84ead3db80 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketClientExtensionHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketClientExtensionHandler.java @@ -15,6 +15,8 @@ */ package io.netty.handler.codec.http.websocketx.extensions; +import static io.netty.util.internal.ObjectUtil.checkNonEmpty; + import io.netty.channel.ChannelDuplexHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPromise; @@ -22,7 +24,6 @@ import io.netty.handler.codec.CodecException; import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponse; -import io.netty.util.internal.ObjectUtil; import java.util.ArrayList; import java.util.Arrays; @@ -51,11 +52,7 @@ public class WebSocketClientExtensionHandler extends ChannelDuplexHandler { * with fallback configuration. */ public WebSocketClientExtensionHandler(WebSocketClientExtensionHandshaker... extensionHandshakers) { - ObjectUtil.checkNotNull(extensionHandshakers, "extensionHandshakers"); - if (extensionHandshakers.length == 0) { - throw new IllegalArgumentException("extensionHandshakers must contains at least one handshaker"); - } - this.extensionHandshakers = Arrays.asList(extensionHandshakers); + this.extensionHandshakers = Arrays.asList(checkNonEmpty(extensionHandshakers, "extensionHandshakers")); } @Override diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketServerExtensionHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketServerExtensionHandler.java index 2013cafd69..70d6a12da6 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketServerExtensionHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketServerExtensionHandler.java @@ -15,6 +15,8 @@ */ package io.netty.handler.codec.http.websocketx.extensions; +import static io.netty.util.internal.ObjectUtil.checkNonEmpty; + import io.netty.channel.ChannelDuplexHandler; import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFutureListener; @@ -25,7 +27,6 @@ import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponse; import io.netty.handler.codec.http.HttpResponseStatus; -import io.netty.util.internal.ObjectUtil; import java.util.ArrayList; import java.util.Arrays; @@ -56,11 +57,7 @@ public class WebSocketServerExtensionHandler extends ChannelDuplexHandler { * with fallback configuration. */ public WebSocketServerExtensionHandler(WebSocketServerExtensionHandshaker... extensionHandshakers) { - ObjectUtil.checkNotNull(extensionHandshakers, "extensionHandshakers"); - if (extensionHandshakers.length == 0) { - throw new IllegalArgumentException("extensionHandshakers must contains at least one handshaker"); - } - this.extensionHandshakers = Arrays.asList(extensionHandshakers); + this.extensionHandshakers = Arrays.asList(checkNonEmpty(extensionHandshakers, "extensionHandshakers")); } @Override diff --git a/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspMethods.java b/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspMethods.java index 7a14ea8fa1..a0dca7618f 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspMethods.java +++ b/codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspMethods.java @@ -15,8 +15,9 @@ */ package io.netty.handler.codec.rtsp; +import static io.netty.util.internal.ObjectUtil.checkNonEmptyAfterTrim; + import io.netty.handler.codec.http.HttpMethod; -import io.netty.util.internal.ObjectUtil; import java.util.HashMap; import java.util.Map; @@ -118,13 +119,7 @@ public final class RtspMethods { * will be returned. Otherwise, a new instance will be returned. */ public static HttpMethod valueOf(String name) { - ObjectUtil.checkNotNull(name, "name"); - - name = name.trim().toUpperCase(); - if (name.isEmpty()) { - throw new IllegalArgumentException("empty name"); - } - + name = checkNonEmptyAfterTrim(name, "name").toUpperCase(); HttpMethod result = methodMap.get(name); if (result != null) { return result; diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyCodecUtil.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyCodecUtil.java index cc17c173ed..02a2f0417c 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyCodecUtil.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyCodecUtil.java @@ -15,8 +15,10 @@ */ package io.netty.handler.codec.spdy; +import static io.netty.util.internal.ObjectUtil.checkNonEmpty; +import static io.netty.util.internal.ObjectUtil.checkNotNull; + import io.netty.buffer.ByteBuf; -import io.netty.util.internal.ObjectUtil; final class SpdyCodecUtil { @@ -287,11 +289,7 @@ final class SpdyCodecUtil { * Validate a SPDY header name. */ static void validateHeaderName(CharSequence name) { - ObjectUtil.checkNotNull(name, "name"); - if (name.length() == 0) { - throw new IllegalArgumentException( - "name cannot be length zero"); - } + checkNonEmpty(name, "name"); // Since name may only contain ascii characters, for valid names // name.length() returns the number of bytes when UTF-8 encoded. if (name.length() > SPDY_MAX_NV_LENGTH) { @@ -318,7 +316,7 @@ final class SpdyCodecUtil { * Validate a SPDY header value. Does not validate max length. */ static void validateHeaderValue(CharSequence value) { - ObjectUtil.checkNotNull(value, "value"); + checkNotNull(value, "value"); for (int i = 0; i < value.length(); i ++) { char c = value.charAt(i); if (c == 0) { diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockJZlibEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockJZlibEncoder.java index 165bfe30aa..b5d348d97a 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockJZlibEncoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockJZlibEncoder.java @@ -23,6 +23,7 @@ import io.netty.buffer.Unpooled; import io.netty.handler.codec.compression.CompressionException; import static io.netty.handler.codec.spdy.SpdyCodecUtil.*; +import static io.netty.util.internal.ObjectUtil.checkNotNullWithIAE; class SpdyHeaderBlockJZlibEncoder extends SpdyHeaderBlockRawEncoder { @@ -122,9 +123,8 @@ class SpdyHeaderBlockJZlibEncoder extends SpdyHeaderBlockRawEncoder { @Override public ByteBuf encode(ByteBufAllocator alloc, SpdyHeadersFrame frame) throws Exception { - if (frame == null) { - throw new IllegalArgumentException("frame"); - } + checkNotNullWithIAE(alloc, "alloc"); + checkNotNullWithIAE(frame, "frame"); if (finished) { return Unpooled.EMPTY_BUFFER; diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockZlibEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockZlibEncoder.java index 4e5a1ea0d0..21a18f90a1 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockZlibEncoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHeaderBlockZlibEncoder.java @@ -24,6 +24,7 @@ import io.netty.util.internal.SuppressJava6Requirement; import java.util.zip.Deflater; import static io.netty.handler.codec.spdy.SpdyCodecUtil.*; +import static io.netty.util.internal.ObjectUtil.checkNotNullWithIAE; class SpdyHeaderBlockZlibEncoder extends SpdyHeaderBlockRawEncoder { @@ -89,9 +90,8 @@ class SpdyHeaderBlockZlibEncoder extends SpdyHeaderBlockRawEncoder { @Override public ByteBuf encode(ByteBufAllocator alloc, SpdyHeadersFrame frame) throws Exception { - if (frame == null) { - throw new IllegalArgumentException("frame"); - } + checkNotNullWithIAE(alloc, "alloc"); + checkNotNullWithIAE(frame, "frame"); if (finished) { return Unpooled.EMPTY_BUFFER;