Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate long client provided names #1576

Merged
merged 1 commit into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions projects/RabbitMQ.Client/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
24 changes: 22 additions & 2 deletions projects/RabbitMQ.Client/client/api/ConnectionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -593,7 +601,7 @@ private ConnectionConfig CreateConfig(string clientProvidedName)
CredentialsRefresher,
AuthMechanisms,
ClientProperties,
clientProvidedName,
EnsureClientProvidedNameLength(clientProvidedName),
RequestedChannelMax,
RequestedFrameMax,
MaxInboundMessageBodySize,
Expand Down Expand Up @@ -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;
}
}
}
}
13 changes: 7 additions & 6 deletions projects/RabbitMQ.Client/client/api/IConnectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
7 changes: 7 additions & 0 deletions projects/RabbitMQ.Client/client/api/InternalConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
lukebakken marked this conversation as resolved.
Show resolved Hide resolved
}
}
31 changes: 30 additions & 1 deletion projects/Test/Integration/TestConnectionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
Loading