NettyUtils: added safe check for null pointers. Added regression tests. (#3938)

Author: martinKindall

.changes/next-release/bugfix-NettyNIOHTTPClient-b68759c.json

@@ -0,0 +1,6 @@
+{
+    "category": "Netty NIO HTTP Client", 
+    "contributor": "martinKindall", 
+    "type": "bugfix", 
+    "description": "The fix involves implementing a null-pointer check in the NettyUtils#isConnectionResetException() method, in case the\n    throwable of the original cause has no message."
+}

http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtils.java

@@ -81,7 +81,10 @@ public static Throwable decorateException(Channel channel, Throwable originalCau
     }
 
     private static boolean isConnectionResetException(Throwable originalCause) {
-        return originalCause instanceof IOException && originalCause.getMessage().contains("Connection reset by peer");
+        String message = originalCause.getMessage();
+        return originalCause instanceof IOException &&
+               message != null &&
+               message.contains("Connection reset by peer");
     }
 
     private static boolean isAcquireTimeoutException(Throwable originalCause) {
@@ -95,7 +98,7 @@ private static boolean isTooManyPendingAcquiresException(Throwable originalCause
         String message = originalCause.getMessage();
         return originalCause instanceof IllegalStateException &&
                message != null &&
-               originalCause.getMessage().contains("Too many outstanding acquire operations");
+               message.contains("Too many outstanding acquire operations");
     }
 
     private static String getMessageForAcquireTimeoutException() {

http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtilsTest.java

@@ -29,13 +29,18 @@
 import io.netty.handler.ssl.SslContext;
 import io.netty.handler.ssl.SslContextBuilder;
 import io.netty.handler.ssl.SslHandler;
+import io.netty.handler.timeout.ReadTimeoutException;
+import io.netty.handler.timeout.WriteTimeoutException;
 import io.netty.util.Attribute;
 import io.netty.util.AttributeKey;
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.GenericFutureListener;
 import io.netty.util.concurrent.Promise;
+import java.io.IOException;
+import java.nio.channels.ClosedChannelException;
 import java.time.Duration;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.net.ssl.SSLEngine;
@@ -268,4 +273,109 @@ public void closedChannelMessage_with_nullParentChannelAttribute() throws Except
         assertThat(NettyUtils.closedChannelMessage(channel))
             .isEqualTo(NettyUtils.CLOSED_CHANNEL_ERROR_MESSAGE);
     }
+
+    @Test
+    public void decorateException_with_TimeoutException() {
+
+        Channel channel = mock(Channel.class);
+        Throwable timeoutException = new TimeoutException("...Acquire operation took longer...");
+        Throwable output = NettyUtils.decorateException(channel, timeoutException);
+
+        assertThat(output).isInstanceOf(Throwable.class);
+        assertThat(output.getCause()).isInstanceOf(TimeoutException.class);
+        assertThat(output.getMessage()).isNotNull();
+    }
+
+    @Test
+    public void decorateException_with_TimeoutException_noMsg() {
+
+        Channel channel = mock(Channel.class);
+        Throwable timeoutException = new TimeoutException();
+        Throwable output = NettyUtils.decorateException(channel, timeoutException);
+
+        assertThat(output).isInstanceOf(TimeoutException.class);
+        assertThat(output.getCause()).isNull();
+    }
+
+    @Test
+    public void decorateException_with_IllegalStateException() {
+
+        Channel channel = mock(Channel.class);
+        Throwable illegalStateException = new IllegalStateException("...Too many outstanding acquire operations...");
+        Throwable output = NettyUtils.decorateException(channel, illegalStateException);
+
+        assertThat(output).isInstanceOf(Throwable.class);
+        assertThat(output.getCause()).isInstanceOf(IllegalStateException.class);
+        assertThat(output.getMessage()).isNotNull();
+    }
+
+    @Test
+    public void decorateException_with_IllegalStateException_noMsg() {
+
+        Channel channel = mock(Channel.class);
+        Throwable illegalStateException = new IllegalStateException();
+        Throwable output = NettyUtils.decorateException(channel, illegalStateException);
+
+        assertThat(output).isInstanceOf(IllegalStateException.class);
+        assertThat(output.getCause()).isNull();
+    }
+
+    @Test
+    public void decorateException_with_ReadTimeoutException() {
+
+        Channel channel = mock(Channel.class);
+        Throwable readTimeoutException = new ReadTimeoutException();
+        Throwable output = NettyUtils.decorateException(channel, readTimeoutException);
+
+        assertThat(output).isInstanceOf(IOException.class);
+        assertThat(output.getCause()).isInstanceOf(ReadTimeoutException.class);
+        assertThat(output.getMessage()).isNotNull();
+    }
+
+    @Test
+    public void decorateException_with_WriteTimeoutException() {
+
+        Channel channel = mock(Channel.class);
+        Throwable writeTimeoutException = new WriteTimeoutException();
+        Throwable output = NettyUtils.decorateException(channel, writeTimeoutException);
+
+        assertThat(output).isInstanceOf(IOException.class);
+        assertThat(output.getCause()).isInstanceOf(WriteTimeoutException.class);
+        assertThat(output.getMessage()).isNotNull();
+    }
+
+    @Test
+    public void decorateException_with_ClosedChannelException() {
+
+        Channel channel = mock(Channel.class);
+        Throwable closedChannelException = new ClosedChannelException();
+        Throwable output = NettyUtils.decorateException(channel, closedChannelException);
+
+        assertThat(output).isInstanceOf(IOException.class);
+        assertThat(output.getCause()).isInstanceOf(ClosedChannelException.class);
+        assertThat(output.getMessage()).isNotNull();
+    }
+
+    @Test
+    public void decorateException_with_IOException_reset() {
+
+        Channel channel = mock(Channel.class);
+        Throwable closedChannelException = new IOException("...Connection reset by peer...");
+        Throwable output = NettyUtils.decorateException(channel, closedChannelException);
+
+        assertThat(output).isInstanceOf(IOException.class);
+        assertThat(output.getCause()).isInstanceOf(IOException.class);
+        assertThat(output.getMessage()).isNotNull();
+    }
+
+    @Test
+    public void decorateException_with_IOException_noMsg() {
+
+        Channel channel = mock(Channel.class);
+        Throwable closedChannelException = new IOException();
+        Throwable output = NettyUtils.decorateException(channel, closedChannelException);
+
+        assertThat(output).isInstanceOf(IOException.class);
+        assertThat(output.getCause()).isNull();
+    }
 }