Merge pull request #1887 from dagnir/gh1886

Safely declare Netty AttributeKeys

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);
+    }
+}