Merge pull request #1576 from rabbitmq/rabbitmq-dotnet-client-980

Truncate long client provided names

projects/RabbitMQ.Client/PublicAPI.Unshipped.txt

@@ -952,8 +952,8 @@ virtual RabbitMQ.Client.TcpClientAdapter.ReceiveTimeout.set -> void
 ~static RabbitMQ.Client.IConnectionExtensions.AbortAsync(this RabbitMQ.Client.IConnection connection, System.TimeSpan timeout) -> System.Threading.Tasks.Task
 ~static RabbitMQ.Client.IConnectionExtensions.AbortAsync(this RabbitMQ.Client.IConnection connection, ushort reasonCode, string reasonText) -> System.Threading.Tasks.Task
 ~static RabbitMQ.Client.IConnectionExtensions.AbortAsync(this RabbitMQ.Client.IConnection connection, ushort reasonCode, string reasonText, System.TimeSpan timeout) -> System.Threading.Tasks.Task
-~static RabbitMQ.Client.IConnectionExtensions.CloseAsync(this RabbitMQ.Client.IConnection connection) -> System.Threading.Tasks.Task
+~static RabbitMQ.Client.IConnectionExtensions.CloseAsync(this RabbitMQ.Client.IConnection connection, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task
 ~static RabbitMQ.Client.IConnectionExtensions.CloseAsync(this RabbitMQ.Client.IConnection connection, System.TimeSpan timeout) -> System.Threading.Tasks.Task
-~static RabbitMQ.Client.IConnectionExtensions.CloseAsync(this RabbitMQ.Client.IConnection connection, ushort reasonCode, string reasonText) -> System.Threading.Tasks.Task
+~static RabbitMQ.Client.IConnectionExtensions.CloseAsync(this RabbitMQ.Client.IConnection connection, ushort reasonCode, string reasonText, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task
 ~static RabbitMQ.Client.IConnectionExtensions.CloseAsync(this RabbitMQ.Client.IConnection connection, ushort reasonCode, string reasonText, System.TimeSpan timeout) -> System.Threading.Tasks.Task
 ~virtual RabbitMQ.Client.DefaultBasicConsumer.HandleBasicDeliverAsync(string consumerTag, ulong deliveryTag, bool redelivered, string exchange, string routingKey, RabbitMQ.Client.ReadOnlyBasicProperties properties, System.ReadOnlyMemory<byte> body) -> System.Threading.Tasks.Task

projects/RabbitMQ.Client/client/api/ConnectionFactory.cs

@@ -197,6 +197,7 @@ public sealed class ConnectionFactory : ConnectionFactoryBase, IConnectionFactor
 
         // just here to hold the value that was set through the setter
         private Uri _uri;
+        private string _clientProvidedName;
 
         /// <summary>
         /// Amount of time protocol handshake operations are allowed to take before
@@ -367,7 +368,14 @@ public Uri Uri
         /// <summary>
         /// Default client provided name to be used for connections.
         /// </summary>
-        public string ClientProvidedName { get; set; }
+        public string ClientProvidedName
+        {
+            get => _clientProvidedName;
+            set
+            {
+                _clientProvidedName = EnsureClientProvidedNameLength(value);
+            }
+        }
 
         /// <summary>
         /// Given a list of mechanism names supported by the server, select a preferred mechanism,
@@ -593,7 +601,7 @@ private ConnectionConfig CreateConfig(string clientProvidedName)
                 CredentialsRefresher,
                 AuthMechanisms,
                 ClientProperties,
-                clientProvidedName,
+                EnsureClientProvidedNameLength(clientProvidedName),
                 RequestedChannelMax,
                 RequestedFrameMax,
                 MaxInboundMessageBodySize,
@@ -712,5 +720,17 @@ private List<AmqpTcpEndpoint> LocalEndpoints()
         {
             return new List<AmqpTcpEndpoint> { Endpoint };
         }
+
+        private static string EnsureClientProvidedNameLength(string clientProvidedName)
+        {
+            if (clientProvidedName.Length > InternalConstants.DefaultRabbitMqMaxClientProvideNameLength)
+            {
+                return clientProvidedName.Substring(0, InternalConstants.DefaultRabbitMqMaxClientProvideNameLength);
+            }
+            else
+            {
+                return clientProvidedName;
+            }
+        }
     }
 }

projects/RabbitMQ.Client/client/api/IConnectionExtensions.cs

@@ -18,17 +18,17 @@ public static class IConnectionExtensions
         /// (or closing), then this method will do nothing.
         /// It can also throw <see cref="IOException"/> when socket was closed unexpectedly.
         /// </remarks>
-        public static Task CloseAsync(this IConnection connection)
+        public static Task CloseAsync(this IConnection connection, CancellationToken cancellationToken = default)
         {
             return connection.CloseAsync(Constants.ReplySuccess, "Goodbye", InternalConstants.DefaultConnectionCloseTimeout, false,
-                CancellationToken.None);
+                cancellationToken);
         }
 
         /// <summary>
         /// Asynchronously close this connection and all its channels.
         /// </summary>
         /// <remarks>
-        /// The method behaves in the same way as <see cref="CloseAsync(IConnection)"/>, with the only
+        /// The method behaves in the same way as <see cref="CloseAsync(IConnection, CancellationToken)"/>, with the only
         /// difference that the connection is closed with the given connection close code and message.
         /// <para>
         /// The close code (See under "Reply Codes" in the AMQP specification).
@@ -37,10 +37,11 @@ public static Task CloseAsync(this IConnection connection)
         /// A message indicating the reason for closing the connection.
         /// </para>
         /// </remarks>
-        public static Task CloseAsync(this IConnection connection, ushort reasonCode, string reasonText)
+        public static Task CloseAsync(this IConnection connection, ushort reasonCode, string reasonText,
+            CancellationToken cancellationToken = default)
         {
             return connection.CloseAsync(reasonCode, reasonText, InternalConstants.DefaultConnectionCloseTimeout, false,
-                CancellationToken.None);
+                cancellationToken);
         }
 
         /// <summary>
@@ -92,7 +93,7 @@ public static Task CloseAsync(this IConnection connection, ushort reasonCode, st
         /// </summary>
         /// <remarks>
         /// Note that all active channels and sessions will be closed if this method is called.
-        /// In comparison to normal <see cref="CloseAsync(IConnection)"/> method, <see cref="AbortAsync(IConnection)"/> will not throw
+        /// In comparison to normal <see cref="CloseAsync(IConnection, CancellationToken)"/> method, <see cref="AbortAsync(IConnection)"/> will not throw
         /// <see cref="IOException"/> during closing connection.
         ///This method waits infinitely for the in-progress close operation to complete.
         /// </remarks>

projects/RabbitMQ.Client/client/api/InternalConstants.cs

@@ -44,5 +44,12 @@ internal static class InternalConstants
         /// configures the largest message size which should be lower than this maximum of 128MiB.
         /// </summary>
         internal const uint DefaultRabbitMqMaxInboundMessageBodySize = 1_048_576 * 128;
+
+        /// <summary>
+        /// Largest client provide name, in characters, allowed in RabbitMQ.
+        /// This is not configurable, but was discovered while working on this issue:
+        /// https://github.com/rabbitmq/rabbitmq-dotnet-client/issues/980
+        /// </summary>
+        internal const int DefaultRabbitMqMaxClientProvideNameLength = 3652;
     }
 }

projects/Test/Integration/TestConnectionFactory.cs

@@ -436,7 +436,36 @@ public async Task TestCreateConnectionAsync_UsesValidEndpointWhenMultipleSupplie
                 var ep = new AmqpTcpEndpoint("localhost");
                 using (IConnection conn = await cf.CreateConnectionAsync(new List<AmqpTcpEndpoint> { invalidEp, ep }, cts.Token))
                 {
-                    await conn.CloseAsync();
+                    await conn.CloseAsync(cts.Token);
+                }
+            }
+        }
+
+        [Theory]
+        [InlineData(3650)]
+        [InlineData(3651)]
+        [InlineData(3652)]
+        [InlineData(3653)]
+        [InlineData(3654)]
+        public async Task TestCreateConnectionAsync_TruncatesWhenClientNameIsLong_GH980(ushort count)
+        {
+            string cpn = GetUniqueString(count);
+            using (var cts = new CancellationTokenSource(WaitSpan))
+            {
+                ConnectionFactory cf0 = new ConnectionFactory { ClientProvidedName = cpn };
+                using (IConnection conn = await cf0.CreateConnectionAsync(cts.Token))
+                {
+                    await conn.CloseAsync(cts.Token);
+                    Assert.True(cf0.ClientProvidedName.Length <= InternalConstants.DefaultRabbitMqMaxClientProvideNameLength);
+                    Assert.Contains(cf0.ClientProvidedName, cpn);
+                }
+
+                ConnectionFactory cf1 = new ConnectionFactory();
+                using (IConnection conn = await cf1.CreateConnectionAsync(cpn, cts.Token))
+                {
+                    await conn.CloseAsync(cts.Token);
+                    Assert.True(conn.ClientProvidedName.Length <= InternalConstants.DefaultRabbitMqMaxClientProvideNameLength);
+                    Assert.Contains(conn.ClientProvidedName, cpn);
                 }
             }
         }