From 96e67ecb32cc15a63b2ce3d881e037d7ec9a9df9 Mon Sep 17 00:00:00 2001 From: Dongie Agnir Date: Wed, 10 Jun 2020 13:40:28 -0700 Subject: [PATCH] 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 --- .../software/amazon/awssdk/checkstyle.xml | 8 +++++ .../netty/internal/ChannelAttributeKey.java | 34 ++++++++++--------- .../internal/Http1TunnelConnectionPool.java | 3 +- .../internal/ReleaseOnceChannelPool.java | 4 ++- .../http2/Http2MultiplexedChannelPool.java | 7 ++-- .../nio/netty/internal/utils/NettyUtils.java | 13 +++++++ .../OrderedWriteChannelHandlerContext.java | 2 +- .../netty/internal/utils/NettyUtilsTest.java | 30 ++++++++++++++++ 8 files changed, 79 insertions(+), 22 deletions(-) create mode 100644 http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtilsTest.java diff --git a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml index 1d42ef9f8f7f..05001c3fba7b 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle.xml @@ -365,6 +365,14 @@ + + + + + + + + diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelAttributeKey.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelAttributeKey.java index f2feca135ffa..3a2b9e51d31b 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelAttributeKey.java +++ b/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> PROTOCOL_FUTURE = AttributeKey.newInstance( + public static final AttributeKey> 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 HTTP2_MULTIPLEXED_CHANNEL_POOL = AttributeKey.newInstance( - "aws.http.nio.netty.async.http2MultiplexedChannelPool"); + public static final AttributeKey HTTP2_MULTIPLEXED_CHANNEL_POOL = + NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.http2MultiplexedChannelPool"); public static final AttributeKey PING_TRACKER = - AttributeKey.newInstance("aws.http.nio.netty.async.h2.pingTracker"); + NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.h2.pingTracker"); public static final AttributeKey HTTP2_CONNECTION = - AttributeKey.newInstance("aws.http.nio.netty.async.http2Connection"); + NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.http2Connection"); public static final AttributeKey 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 MAX_CONCURRENT_STREAMS = AttributeKey.newInstance( + public static final AttributeKey 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 KEEP_ALIVE = AttributeKey.newInstance("aws.http.nio.netty.async.keepAlive"); + static final AttributeKey KEEP_ALIVE = NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.keepAlive"); /** * Attribute key for {@link RequestContext}. */ - static final AttributeKey REQUEST_CONTEXT_KEY = AttributeKey.newInstance( + static final AttributeKey REQUEST_CONTEXT_KEY = NettyUtils.getOrCreateAttributeKey( "aws.http.nio.netty.async.requestContext"); - static final AttributeKey> SUBSCRIBER_KEY = AttributeKey.newInstance( + static final AttributeKey> SUBSCRIBER_KEY = NettyUtils.getOrCreateAttributeKey( "aws.http.nio.netty.async.subscriber"); - static final AttributeKey RESPONSE_COMPLETE_KEY = AttributeKey.newInstance( + static final AttributeKey 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 LAST_HTTP_CONTENT_RECEIVED_KEY = AttributeKey.newInstance( + static final AttributeKey LAST_HTTP_CONTENT_RECEIVED_KEY = NettyUtils.getOrCreateAttributeKey( "aws.http.nio.netty.async.lastHttpContentReceived"); - static final AttributeKey> EXECUTE_FUTURE_KEY = AttributeKey.newInstance( + static final AttributeKey> EXECUTE_FUTURE_KEY = NettyUtils.getOrCreateAttributeKey( "aws.http.nio.netty.async.executeFuture"); - static final AttributeKey EXECUTION_ID_KEY = AttributeKey.newInstance( + static final AttributeKey EXECUTION_ID_KEY = NettyUtils.getOrCreateAttributeKey( "aws.http.nio.netty.async.executionId"); /** * Whether the channel is still in use */ - static final AttributeKey IN_USE = AttributeKey.newInstance("aws.http.nio.netty.async.inUse"); + static final AttributeKey IN_USE = NettyUtils.getOrCreateAttributeKey("aws.http.nio.netty.async.inUse"); /** * Whether the channel should be closed once it is released. */ - static final AttributeKey CLOSE_ON_RELEASE = AttributeKey.newInstance("aws.http.nio.netty.async.closeOnRelease"); + static final AttributeKey CLOSE_ON_RELEASE = NettyUtils.getOrCreateAttributeKey( + "aws.http.nio.netty.async.closeOnRelease"); private ChannelAttributeKey() { } diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/Http1TunnelConnectionPool.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/Http1TunnelConnectionPool.java index b2a416c0cafe..f687f7052c3d 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/Http1TunnelConnectionPool.java +++ b/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 TUNNEL_ESTABLISHED_KEY = AttributeKey.newInstance( + static final AttributeKey TUNNEL_ESTABLISHED_KEY = NettyUtils.getOrCreateAttributeKey( "aws.http.nio.netty.async.Http1TunnelConnectionPool.tunnelEstablished"); private static final Logger log = Logger.loggerFor(Http1TunnelConnectionPool.class); diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ReleaseOnceChannelPool.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ReleaseOnceChannelPool.java index e0fb07427704..dd9b92517cf6 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ReleaseOnceChannelPool.java +++ b/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 IS_RELEASED = AttributeKey.newInstance("isReleased"); + private static final AttributeKey IS_RELEASED = NettyUtils.getOrCreateAttributeKey( + "software.amazon.awssdk.http.nio.netty.internal.http2.ReleaseOnceChannelPool.isReleased"); private final ChannelPool delegate; diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/http2/Http2MultiplexedChannelPool.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/http2/Http2MultiplexedChannelPool.java index 6118ba0b1230..bfdfee4c26e0 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/http2/Http2MultiplexedChannelPool.java +++ b/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 MULTIPLEXED_CHANNEL = AttributeKey.newInstance( - "software.amazon.awssdk.http.nio.netty.internal.http2.Http2MultiplexedChannelPool.MULTIPLEXED_CHANNEL"); + private static final AttributeKey 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 RELEASED = AttributeKey.newInstance( + private static final AttributeKey RELEASED = NettyUtils.getOrCreateAttributeKey( "software.amazon.awssdk.http.nio.netty.internal.http2.Http2MultiplexedChannelPool.RELEASED"); private final ChannelPool connectionPool; diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtils.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtils.java index 4d0b20b4a128..584bdc635f8e 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtils.java +++ b/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 AttributeKey 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 + } } diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/OrderedWriteChannelHandlerContext.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/OrderedWriteChannelHandlerContext.java index 9e06a565e8e2..2a27c2dd32f0 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/OrderedWriteChannelHandlerContext.java +++ b/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 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); diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtilsTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtilsTest.java new file mode 100644 index 000000000000..680057886174 --- /dev/null +++ b/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 fooAttr = NettyUtils.getOrCreateAttributeKey(attr); + assertThat(NettyUtils.getOrCreateAttributeKey(attr)).isSameAs(fooAttr); + } +}