Safely declare Netty AttributeKeys

This commit prevents the Netty client from throwing an exception in cases where
it tries to declare an attribute key and the key already exists. This can happen
when separate instances of the SDK are loaded by different classloaders, but the
Netty classes loaded by a third and shared by the other classloaders.

Fixes #1886

Author: dagnir

build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml

@@ -365,6 +365,14 @@
             <property name="ignoreComments" value="true"/>
         </module>
 
+        <!-- Checks that we don't use AttributeKey.newInstance directly -->
+        <module name="Regexp">
+            <property name="format" value="AttributeKey\.newInstance"/>
+            <property name="illegalPattern" value="true"/>
+            <property name="message" value="Use NettyUtils.getOrCreateAttributeKey to safely declare AttributeKeys"/>
+            <property name="ignoreComments" value="true"/>
+        </module>
+
         <!-- Checks that we don't use plural enum names -->
         <module name="software.amazon.awssdk.buildtools.checkstyle.PluralEnumNames"/>
 

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

@@ -26,6 +26,7 @@
 import software.amazon.awssdk.http.Protocol;
 import software.amazon.awssdk.http.nio.netty.internal.http2.Http2MultiplexedChannelPool;
 import software.amazon.awssdk.http.nio.netty.internal.http2.PingTracker;
+import software.amazon.awssdk.http.nio.netty.internal.utils.NettyUtils;
 
 /**
  * Keys for attributes attached via {@link io.netty.channel.Channel#attr(AttributeKey)}.
@@ -36,70 +37,71 @@ public final class ChannelAttributeKey {
     /**
      * Future that when a protocol (http/1.1 or h2) has been selected.
      */
-    public static final AttributeKey<CompletableFuture<Protocol>> PROTOCOL_FUTURE = AttributeKey.newInstance(
+    public static final AttributeKey<CompletableFuture<Protocol>> PROTOCOL_FUTURE = NettyUtils.getOrCreateAttributeKey(
         "aws.http.nio.netty.async.protocolFuture");
 
     /**
      * Reference to {@link Http2MultiplexedChannelPool} which stores information about leased streams for a multiplexed
      * connection.
      */
-    public static final AttributeKey<Http2MultiplexedChannelPool> HTTP2_MULTIPLEXED_CHANNEL_POOL = AttributeKey.newInstance(
-        "aws.http.nio.netty.async.http2MultiplexedChannelPool");
+    public static final AttributeKey<Http2MultiplexedChannelPool> HTTP2_MULTIPLEXED_CHANNEL_POOL =
+        NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.http2MultiplexedChannelPool");
 
     public static final AttributeKey<PingTracker> PING_TRACKER =
-        AttributeKey.newInstance("aws.http.nio.netty.async.h2.pingTracker");
+        NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.h2.pingTracker");
 
     public static final AttributeKey<Http2Connection> HTTP2_CONNECTION =
-        AttributeKey.newInstance("aws.http.nio.netty.async.http2Connection");
+        NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.http2Connection");
 
     public static final AttributeKey<Integer> HTTP2_INITIAL_WINDOW_SIZE =
-        AttributeKey.newInstance("aws.http.nio.netty.async.http2InitialWindowSize");
+        NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.http2InitialWindowSize");
 
     /**
      * Value of the MAX_CONCURRENT_STREAMS from the server's SETTING frame.
      */
-    public static final AttributeKey<Long> MAX_CONCURRENT_STREAMS = AttributeKey.newInstance(
+    public static final AttributeKey<Long> MAX_CONCURRENT_STREAMS = NettyUtils.getOrCreateAttributeKey(
         "aws.http.nio.netty.async.maxConcurrentStreams");
 
     /**
      * {@link AttributeKey} to keep track of whether we should close the connection after this request
      * has completed.
      */
-    static final AttributeKey<Boolean> KEEP_ALIVE = AttributeKey.newInstance("aws.http.nio.netty.async.keepAlive");
+    static final AttributeKey<Boolean> KEEP_ALIVE = NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.keepAlive");
 
     /**
      * Attribute key for {@link RequestContext}.
      */
-    static final AttributeKey<RequestContext> REQUEST_CONTEXT_KEY = AttributeKey.newInstance(
+    static final AttributeKey<RequestContext> REQUEST_CONTEXT_KEY = NettyUtils.getOrCreateAttributeKey(
         "aws.http.nio.netty.async.requestContext");
 
-    static final AttributeKey<Subscriber<? super ByteBuffer>> SUBSCRIBER_KEY = AttributeKey.newInstance(
+    static final AttributeKey<Subscriber<? super ByteBuffer>> SUBSCRIBER_KEY = NettyUtils.getOrCreateAttributeKey(
         "aws.http.nio.netty.async.subscriber");
 
-    static final AttributeKey<Boolean> RESPONSE_COMPLETE_KEY = AttributeKey.newInstance(
+    static final AttributeKey<Boolean> RESPONSE_COMPLETE_KEY = NettyUtils.getOrCreateAttributeKey(
         "aws.http.nio.netty.async.responseComplete");
 
     /**
      * {@link AttributeKey} to keep track of whether we have received the {@link LastHttpContent}.
      */
-    static final AttributeKey<Boolean> LAST_HTTP_CONTENT_RECEIVED_KEY = AttributeKey.newInstance(
+    static final AttributeKey<Boolean> LAST_HTTP_CONTENT_RECEIVED_KEY = NettyUtils.getOrCreateAttributeKey(
         "aws.http.nio.netty.async.lastHttpContentReceived");
 
-    static final AttributeKey<CompletableFuture<Void>> EXECUTE_FUTURE_KEY = AttributeKey.newInstance(
+    static final AttributeKey<CompletableFuture<Void>> EXECUTE_FUTURE_KEY = NettyUtils.getOrCreateAttributeKey(
             "aws.http.nio.netty.async.executeFuture");
 
-    static final AttributeKey<Long> EXECUTION_ID_KEY = AttributeKey.newInstance(
+    static final AttributeKey<Long> EXECUTION_ID_KEY = NettyUtils.getOrCreateAttributeKey(
             "aws.http.nio.netty.async.executionId");
 
     /**
      * Whether the channel is still in use
      */
-    static final AttributeKey<Boolean> IN_USE = AttributeKey.newInstance("aws.http.nio.netty.async.inUse");
+    static final AttributeKey<Boolean> IN_USE = NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.inUse");
 
     /**
      * Whether the channel should be closed once it is released.
      */
-    static final AttributeKey<Boolean> CLOSE_ON_RELEASE = AttributeKey.newInstance("aws.http.nio.netty.async.closeOnRelease");
+    static final AttributeKey<Boolean> CLOSE_ON_RELEASE = NettyUtils.getOrCreateAttributeKey(
+            "aws.http.nio.netty.async.closeOnRelease");
 
     private ChannelAttributeKey() {
     }

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

@@ -29,6 +29,7 @@
 import java.net.URI;
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.annotations.SdkTestInternalApi;
+import software.amazon.awssdk.http.nio.netty.internal.utils.NettyUtils;
 import software.amazon.awssdk.utils.Logger;
 import software.amazon.awssdk.utils.StringUtils;
 
@@ -37,7 +38,7 @@
  */
 @SdkInternalApi
 public class Http1TunnelConnectionPool implements ChannelPool {
-    static final AttributeKey<Boolean> TUNNEL_ESTABLISHED_KEY = AttributeKey.newInstance(
+    static final AttributeKey<Boolean> TUNNEL_ESTABLISHED_KEY = NettyUtils.getOrCreateAttributeKey(
             "aws.http.nio.netty.async.Http1TunnelConnectionPool.tunnelEstablished");
 
     private static final Logger log = Logger.loggerFor(Http1TunnelConnectionPool.class);

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

@@ -26,6 +26,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.http.nio.netty.internal.http2.Http2MultiplexedChannelPool;
+import software.amazon.awssdk.http.nio.netty.internal.utils.NettyUtils;
 
 /**
  * Wrapper around a {@link ChannelPool} to protect it from having the same channel released twice. This can
@@ -35,7 +36,8 @@
 @SdkInternalApi
 public class ReleaseOnceChannelPool implements ChannelPool {
 
-    private static final AttributeKey<AtomicBoolean> IS_RELEASED = AttributeKey.newInstance("isReleased");
+    private static final AttributeKey<AtomicBoolean> IS_RELEASED = NettyUtils.getOrCreateAttributeKey(
+            "software.amazon.awssdk.http.nio.netty.internal.http2.ReleaseOnceChannelPool.isReleased");
 
     private final ChannelPool delegate;
 

http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/http2/Http2MultiplexedChannelPool.java

@@ -50,6 +50,7 @@
 import software.amazon.awssdk.http.Protocol;
 import software.amazon.awssdk.http.nio.netty.internal.ChannelAttributeKey;
 import software.amazon.awssdk.http.nio.netty.internal.utils.BetterFixedChannelPool;
+import software.amazon.awssdk.http.nio.netty.internal.utils.NettyUtils;
 import software.amazon.awssdk.utils.Logger;
 import software.amazon.awssdk.utils.Validate;
 
@@ -72,13 +73,13 @@ public class Http2MultiplexedChannelPool implements ChannelPool {
     /**
      * Reference to the {@link MultiplexedChannelRecord} on a channel.
      */
-    private static final AttributeKey<MultiplexedChannelRecord> MULTIPLEXED_CHANNEL = AttributeKey.newInstance(
-        "software.amazon.awssdk.http.nio.netty.internal.http2.Http2MultiplexedChannelPool.MULTIPLEXED_CHANNEL");
+    private static final AttributeKey<MultiplexedChannelRecord> MULTIPLEXED_CHANNEL = NettyUtils.getOrCreateAttributeKey(
+            "software.amazon.awssdk.http.nio.netty.internal.http2.Http2MultiplexedChannelPool.MULTIPLEXED_CHANNEL");
 
     /**
      * Whether a parent channel has been released yet. This guards against double-releasing to the delegate connection pool.
      */
-    private static final AttributeKey<Boolean> RELEASED = AttributeKey.newInstance(
+    private static final AttributeKey<Boolean> RELEASED = NettyUtils.getOrCreateAttributeKey(
         "software.amazon.awssdk.http.nio.netty.internal.http2.Http2MultiplexedChannelPool.RELEASED");
 
     private final ChannelPool connectionPool;

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

@@ -16,6 +16,7 @@
 package software.amazon.awssdk.http.nio.netty.internal.utils;
 
 import io.netty.channel.EventLoop;
+import io.netty.util.AttributeKey;
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.GenericFutureListener;
@@ -160,4 +161,16 @@ public static void warnIfNotInEventLoop(EventLoop loop) {
             log.warn(() -> "Execution is happening outside of the expected event loop.", exception);
         }
     }
+
+    /**
+     * @return an {@code AttributeKey} for {@code attr}. This returns an existing instance if it was previously created.
+     */
+    public static <T> AttributeKey<T> getOrCreateAttributeKey(String attr) {
+        if (AttributeKey.exists(attr)) {
+            return AttributeKey.valueOf(attr);
+        }
+        //CHECKSTYLE:OFF - This is the only place allowed to call AttributeKey.newInstance()
+        return AttributeKey.newInstance(attr);
+        //CHECKSTYLE:ON
+    }
 }

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

@@ -31,7 +31,7 @@
 @SdkInternalApi
 public class OrderedWriteChannelHandlerContext extends DelegatingChannelHandlerContext {
     private static final AttributeKey<Void> ORDERED =
-        AttributeKey.newInstance("aws.http.nio.netty.async.OrderedWriteChannelHandlerContext.ORDERED");
+        NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.OrderedWriteChannelHandlerContext.ORDERED");
 
     private OrderedWriteChannelHandlerContext(ChannelHandlerContext delegate) {
         super(delegate);

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

@@ -0,0 +1,30 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http.nio.netty.internal.utils;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.netty.util.AttributeKey;
+import org.junit.Test;
+
+public class NettyUtilsTest {
+    @Test
+    public void testGetOrCreateAttributeKey_calledTwiceWithSameName_returnsSameInstance() {
+        String attr = "NettyUtilsTest.Foo";
+        AttributeKey<String> fooAttr = NettyUtils.getOrCreateAttributeKey(attr);
+        assertThat(NettyUtils.getOrCreateAttributeKey(attr)).isSameAs(fooAttr);
+    }
+}