Add null check for channel when generating error message (#3574)

* Add check for channel being null before trying to access properties on it

* update changelog

* Update NettyUtils.java

* Update NettyUtils.java

* Update NettyUtils.java

* Update NettyUtils.java

* Update NettyUtilsTest.java

* Update NettyUtilsTest.java

* Added CLOSED_CHANNEL_ERROR_MESSAGE to fix compilation issues

Co-authored-by: John Viegas <[email protected]>
Co-authored-by: Zoe Wang <[email protected]>

Author: 3 people

.changes/next-release/bugfix-AWSSDKforJavav2-c258b00.json

@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "sigurjonviktorsson",
+    "description": "Prevent NullPointerException in Netty exception handler"
+}

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

@@ -53,6 +53,12 @@ public final class NettyUtils {
      */
     public static final SucceededFuture<?> SUCCEEDED_FUTURE = new SucceededFuture<>(null, null);
 
+    public static final String CLOSED_CHANNEL_ERROR_MESSAGE = "The connection was closed during the request. The request will "
+                                                              + "usually succeed on a retry, but if it does not: consider "
+                                                              + "disabling any proxies you have configured, enabling debug "
+                                                              + "logging, or performing a TCP dump to identify the root cause. "
+                                                              + "If this is a streaming operation, validate that data is being "
+                                                              + "read or written in a timely manner.";
     private static final Logger log = Logger.loggerFor(NettyUtils.class);
 
     private NettyUtils() {
@@ -133,15 +139,14 @@ private static String getMessageForTooManyAcquireOperationsError() {
     }
 
     public static String closedChannelMessage(Channel channel) {
-        ChannelDiagnostics channelDiagnostics = channel.attr(CHANNEL_DIAGNOSTICS).get();
-        ChannelDiagnostics parentChannelDiagnostics = channel.parent() != null ? channel.parent().attr(CHANNEL_DIAGNOSTICS).get()
-                                                                               : null;
+        ChannelDiagnostics channelDiagnostics = channel != null && channel.attr(CHANNEL_DIAGNOSTICS) != null ?
+                                                channel.attr(CHANNEL_DIAGNOSTICS).get() : null;
+        ChannelDiagnostics parentChannelDiagnostics = channel != null && channel.parent() != null && 
+                                                      channel.parent().attr(CHANNEL_DIAGNOSTICS) != null ?
+                                                      channel.parent().attr(CHANNEL_DIAGNOSTICS).get() : null;
 
         StringBuilder error = new StringBuilder();
-        error.append("The connection was closed during the request. The request will usually succeed on a retry, but if it does"
-                     + " not: consider disabling any proxies you have configured, enabling debug logging, or performing a TCP"
-                     + " dump to identify the root cause. If this is a streaming operation, validate that data is being read or"
-                     + " written in a timely manner.");
+        error.append(CLOSED_CHANNEL_ERROR_MESSAGE);
 
         if (channelDiagnostics != null) {
             error.append(" Channel Information: ").append(channelDiagnostics);

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

@@ -29,6 +29,7 @@
 import io.netty.handler.ssl.SslContext;
 import io.netty.handler.ssl.SslContextBuilder;
 import io.netty.handler.ssl.SslHandler;
+import io.netty.util.Attribute;
 import io.netty.util.AttributeKey;
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.Future;
@@ -41,6 +42,7 @@
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 import org.slf4j.Logger;
 import software.amazon.awssdk.http.nio.netty.internal.MockChannel;
 
@@ -222,4 +224,48 @@ public void runAndLogError_runnableThrows_loggerInvoked() {
 
         verify(delegateLogger).error(msg, error);
     }
+
+    @Test
+    public void closedChannelMessage_with_nullChannelAttribute() throws Exception {
+
+        Channel channel = Mockito.mock(Channel.class);
+        when(channel.parent()).thenReturn(null);
+
+        assertThat(NettyUtils.closedChannelMessage(channel))
+            .isEqualTo(NettyUtils.CLOSED_CHANNEL_ERROR_MESSAGE);
+    }
+
+    @Test
+    public void closedChannelMessage_with_nullChannel() throws Exception {
+        Channel channel = null;
+        assertThat(NettyUtils.closedChannelMessage(channel))
+            .isEqualTo(NettyUtils.CLOSED_CHANNEL_ERROR_MESSAGE);
+    }
+
+
+    @Test
+    public void closedChannelMessage_with_nullParentChannel() throws Exception {
+
+        Channel channel = mock(Channel.class);
+        Attribute attribute = mock(Attribute.class);
+        when(channel.parent()).thenReturn(null);
+        when(channel.attr(any())).thenReturn(attribute);
+
+        assertThat(NettyUtils.closedChannelMessage(channel))
+            .isEqualTo(NettyUtils.CLOSED_CHANNEL_ERROR_MESSAGE);
+    }
+
+    @Test
+    public void closedChannelMessage_with_nullParentChannelAttribute() throws Exception {
+
+        Channel channel = mock(Channel.class);
+        Attribute attribute = mock(Attribute.class);
+        Channel parentChannel = mock(Channel.class);
+        when(channel.parent()).thenReturn(parentChannel);
+        when(channel.attr(any())).thenReturn(attribute);
+        when(parentChannel.attr(any())).thenReturn(null);
+
+        assertThat(NettyUtils.closedChannelMessage(channel))
+            .isEqualTo(NettyUtils.CLOSED_CHANNEL_ERROR_MESSAGE);
+    }
 }