From c3416d8ad2260f3c9bc98a9061d25bba92020f93 Mon Sep 17 00:00:00 2001 From: Boris Unckel Date: Thu, 22 Apr 2021 14:13:14 +0200 Subject: [PATCH] Utilize i.n.u.internal.ObjectUtil to assert Preconditions (transport*) (#11170) (#11181) 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/channel/epoll/TcpMd5Util.java | 19 ++++++++----------- .../netty/channel/kqueue/NativeLongArray.java | 6 ++---- .../rxtx/DefaultRxtxChannelConfig.java | 11 +++-------- .../netty/test/udt/util/TrafficControl.java | 6 +++--- .../netty/channel/DefaultChannelConfig.java | 4 ++-- .../io/netty/channel/local/LocalAddress.java | 12 ++++-------- .../netty/channel/pool/FixedChannelPool.java | 10 ++++------ 7 files changed, 26 insertions(+), 42 deletions(-) diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/TcpMd5Util.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/TcpMd5Util.java index e422d67c09..5f545db07c 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/TcpMd5Util.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/TcpMd5Util.java @@ -15,7 +15,9 @@ */ package io.netty.channel.epoll; -import io.netty.util.internal.ObjectUtil; +import static io.netty.util.internal.ObjectUtil.checkNotNull; +import static io.netty.util.internal.ObjectUtil.checkNotNullWithIAE; +import static io.netty.util.internal.ObjectUtil.checkNonEmpty; import java.io.IOException; import java.net.InetAddress; @@ -29,20 +31,15 @@ final class TcpMd5Util { static Collection newTcpMd5Sigs(AbstractEpollChannel channel, Collection current, Map newKeys) throws IOException { - ObjectUtil.checkNotNull(channel, "channel"); - ObjectUtil.checkNotNull(current, "current"); - ObjectUtil.checkNotNull(newKeys, "newKeys"); + checkNotNull(channel, "channel"); + checkNotNull(current, "current"); + checkNotNull(newKeys, "newKeys"); // Validate incoming values for (Entry e : newKeys.entrySet()) { final byte[] key = e.getValue(); - if (e.getKey() == null) { - throw new IllegalArgumentException("newKeys contains an entry with null address: " + newKeys); - } - ObjectUtil.checkNotNull(key, "newKeys[" + e.getKey() + ']'); - if (key.length == 0) { - throw new IllegalArgumentException("newKeys[" + e.getKey() + "] has an empty key."); - } + checkNotNullWithIAE(e.getKey(), "e.getKey"); + checkNonEmpty(key, e.getKey().toString()); if (key.length > Native.TCP_MD5SIG_MAXKEYLEN) { throw new IllegalArgumentException("newKeys[" + e.getKey() + "] has a key with invalid length; should not exceed the maximum length (" + diff --git a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/NativeLongArray.java b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/NativeLongArray.java index 8ac6f21f38..5c44c57ca4 100644 --- a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/NativeLongArray.java +++ b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/NativeLongArray.java @@ -21,6 +21,7 @@ import io.netty.util.internal.PlatformDependent; import java.nio.ByteBuffer; import static io.netty.channel.unix.Limits.SIZEOF_JLONG; +import static io.netty.util.internal.ObjectUtil.checkPositive; final class NativeLongArray { private ByteBuffer memory; @@ -29,12 +30,9 @@ final class NativeLongArray { private int size; NativeLongArray(int capacity) { - if (capacity < 1) { - throw new IllegalArgumentException("capacity must be >= 1 but was " + capacity); - } + this.capacity = checkPositive(capacity, "capacity"); memory = Buffer.allocateDirectWithNativeOrder(calculateBufferCapacity(capacity)); memoryAddress = Buffer.memoryAddress(memory); - this.capacity = capacity; } private static int idx(int index) { diff --git a/transport-rxtx/src/main/java/io/netty/channel/rxtx/DefaultRxtxChannelConfig.java b/transport-rxtx/src/main/java/io/netty/channel/rxtx/DefaultRxtxChannelConfig.java index 2a651c5c94..96fb19cc31 100644 --- a/transport-rxtx/src/main/java/io/netty/channel/rxtx/DefaultRxtxChannelConfig.java +++ b/transport-rxtx/src/main/java/io/netty/channel/rxtx/DefaultRxtxChannelConfig.java @@ -33,6 +33,7 @@ import static io.netty.channel.rxtx.RxtxChannelOption.READ_TIMEOUT; import static io.netty.channel.rxtx.RxtxChannelOption.RTS; import static io.netty.channel.rxtx.RxtxChannelOption.STOP_BITS; import static io.netty.channel.rxtx.RxtxChannelOption.WAIT_TIME; +import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; /** * Default configuration class for RXTX device connections. @@ -190,19 +191,13 @@ final class DefaultRxtxChannelConfig extends DefaultChannelConfig implements Rxt @Override public RxtxChannelConfig setWaitTimeMillis(final int waitTimeMillis) { - if (waitTimeMillis < 0) { - throw new IllegalArgumentException("Wait time must be >= 0"); - } - waitTime = waitTimeMillis; + this.waitTime = checkPositiveOrZero(waitTimeMillis, "waitTimeMillis"); return this; } @Override public RxtxChannelConfig setReadTimeout(int readTimeout) { - if (readTimeout < 0) { - throw new IllegalArgumentException("readTime must be >= 0"); - } - this.readTimeout = readTimeout; + this.readTimeout = checkPositiveOrZero(readTimeout, "readTimeout"); return this; } diff --git a/transport-udt/src/test/java/io/netty/test/udt/util/TrafficControl.java b/transport-udt/src/test/java/io/netty/test/udt/util/TrafficControl.java index 017e672004..3958e8a78a 100644 --- a/transport-udt/src/test/java/io/netty/test/udt/util/TrafficControl.java +++ b/transport-udt/src/test/java/io/netty/test/udt/util/TrafficControl.java @@ -16,6 +16,8 @@ package io.netty.test.udt.util; +import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; + import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -62,9 +64,7 @@ public final class TrafficControl { * @param time - delay in milliseconds; use zero to remove delay. */ public static void delay(final int time) throws Exception { - if (time < 0) { - throw new IllegalArgumentException("negative latency"); - } + checkPositiveOrZero(time, "time"); final int delay = time / 2; if (delay == 0) { UnitHelp.process(String.format(TC_RESET, "lo")); diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelConfig.java b/transport/src/main/java/io/netty/channel/DefaultChannelConfig.java index e303a668e1..c6506010e5 100644 --- a/transport/src/main/java/io/netty/channel/DefaultChannelConfig.java +++ b/transport/src/main/java/io/netty/channel/DefaultChannelConfig.java @@ -321,10 +321,10 @@ public class DefaultChannelConfig implements ChannelConfig { * is of type {@link MaxMessagesRecvByteBufAllocator}. */ private void setRecvByteBufAllocator(RecvByteBufAllocator allocator, ChannelMetadata metadata) { + checkNotNull(allocator, "allocator"); + checkNotNull(metadata, "metadata"); if (allocator instanceof MaxMessagesRecvByteBufAllocator) { ((MaxMessagesRecvByteBufAllocator) allocator).maxMessagesPerRead(metadata.defaultMaxMessagesPerRead()); - } else if (allocator == null) { - throw new NullPointerException("allocator"); } setRecvByteBufAllocator(allocator); } diff --git a/transport/src/main/java/io/netty/channel/local/LocalAddress.java b/transport/src/main/java/io/netty/channel/local/LocalAddress.java index b1699e1947..c45b5bc592 100644 --- a/transport/src/main/java/io/netty/channel/local/LocalAddress.java +++ b/transport/src/main/java/io/netty/channel/local/LocalAddress.java @@ -15,8 +15,9 @@ */ package io.netty.channel.local; +import static io.netty.util.internal.ObjectUtil.checkNonEmptyAfterTrim; + import io.netty.channel.Channel; -import io.netty.util.internal.ObjectUtil; import java.net.SocketAddress; @@ -51,13 +52,8 @@ public final class LocalAddress extends SocketAddress implements Comparable= 1)"); - } - if (maxPendingAcquires < 1) { - throw new IllegalArgumentException("maxPendingAcquires: " + maxPendingAcquires + " (expected: >= 1)"); - } + checkPositive(maxConnections, "maxConnections"); + checkPositive(maxPendingAcquires, "maxPendingAcquires"); if (action == null && acquireTimeoutMillis == -1) { timeoutTask = null; acquireTimeoutNanos = -1;