fix: address code quality issues from review
- Make ConnectReceived thread-safe with Volatile.Read/Write (accessed from auth timeout task and command pipeline) - Include authTimeoutTask in Task.WhenAny to propagate exceptions - Clear nonce after authentication with CryptographicOperations.ZeroMemory - Avoid closure allocation on publish permission cache hot path (method group) - Update AuthTimeout default to 2s to match Go server
This commit is contained in:
@@ -34,7 +34,7 @@ public sealed class ClientPermissions : IDisposable
|
|||||||
if (_publish == null)
|
if (_publish == null)
|
||||||
return true;
|
return true;
|
||||||
|
|
||||||
return _pubCache.GetOrAdd(subject, s => _publish.IsAllowed(s));
|
return _pubCache.GetOrAdd(subject, _publish.IsAllowed);
|
||||||
}
|
}
|
||||||
|
|
||||||
public bool IsSubscribeAllowed(string subject, string? queue = null)
|
public bool IsSubscribeAllowed(string subject, string? queue = null)
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
using System.Buffers;
|
using System.Buffers;
|
||||||
using System.IO.Pipelines;
|
using System.IO.Pipelines;
|
||||||
using System.Net.Sockets;
|
using System.Net.Sockets;
|
||||||
|
using System.Security.Cryptography;
|
||||||
using System.Text;
|
using System.Text;
|
||||||
using System.Text.Json;
|
using System.Text.Json;
|
||||||
using Microsoft.Extensions.Logging;
|
using Microsoft.Extensions.Logging;
|
||||||
@@ -40,9 +41,12 @@ public sealed class NatsClient : IDisposable
|
|||||||
public ulong Id { get; }
|
public ulong Id { get; }
|
||||||
public ClientOptions? ClientOpts { get; private set; }
|
public ClientOptions? ClientOpts { get; private set; }
|
||||||
public IMessageRouter? Router { get; set; }
|
public IMessageRouter? Router { get; set; }
|
||||||
public bool ConnectReceived { get; private set; }
|
|
||||||
public Account? Account { get; private set; }
|
public Account? Account { get; private set; }
|
||||||
|
|
||||||
|
// Thread-safe: read from auth timeout task on threadpool, written from command pipeline
|
||||||
|
private int _connectReceived;
|
||||||
|
public bool ConnectReceived => Volatile.Read(ref _connectReceived) != 0;
|
||||||
|
|
||||||
// Stats
|
// Stats
|
||||||
public long InMsgs;
|
public long InMsgs;
|
||||||
public long OutMsgs;
|
public long OutMsgs;
|
||||||
@@ -106,7 +110,10 @@ public sealed class NatsClient : IDisposable
|
|||||||
var processTask = ProcessCommandsAsync(pipe.Reader, _clientCts.Token);
|
var processTask = ProcessCommandsAsync(pipe.Reader, _clientCts.Token);
|
||||||
var pingTask = RunPingTimerAsync(_clientCts.Token);
|
var pingTask = RunPingTimerAsync(_clientCts.Token);
|
||||||
|
|
||||||
await Task.WhenAny(fillTask, processTask, pingTask);
|
if (authTimeoutTask != null)
|
||||||
|
await Task.WhenAny(fillTask, processTask, pingTask, authTimeoutTask);
|
||||||
|
else
|
||||||
|
await Task.WhenAny(fillTask, processTask, pingTask);
|
||||||
}
|
}
|
||||||
catch (OperationCanceledException)
|
catch (OperationCanceledException)
|
||||||
{
|
{
|
||||||
@@ -258,6 +265,10 @@ public sealed class NatsClient : IDisposable
|
|||||||
}
|
}
|
||||||
|
|
||||||
_logger.LogDebug("Client {ClientId} authenticated as {Identity}", Id, result.Identity);
|
_logger.LogDebug("Client {ClientId} authenticated as {Identity}", Id, result.Identity);
|
||||||
|
|
||||||
|
// Clear nonce after use — defense-in-depth against memory dumps
|
||||||
|
if (_nonce != null)
|
||||||
|
CryptographicOperations.ZeroMemory(_nonce);
|
||||||
}
|
}
|
||||||
|
|
||||||
// If no account was assigned by auth, assign global account
|
// If no account was assigned by auth, assign global account
|
||||||
@@ -267,7 +278,7 @@ public sealed class NatsClient : IDisposable
|
|||||||
Account.AddClient(Id);
|
Account.AddClient(Id);
|
||||||
}
|
}
|
||||||
|
|
||||||
ConnectReceived = true;
|
Volatile.Write(ref _connectReceived, 1);
|
||||||
_logger.LogDebug("CONNECT received from client {ClientId}, name={ClientName}", Id, ClientOpts?.Name);
|
_logger.LogDebug("CONNECT received from client {ClientId}, name={ClientName}", Id, ClientOpts?.Name);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -26,5 +26,5 @@ public sealed class NatsOptions
|
|||||||
public string? NoAuthUser { get; set; }
|
public string? NoAuthUser { get; set; }
|
||||||
|
|
||||||
// Auth timing
|
// Auth timing
|
||||||
public TimeSpan AuthTimeout { get; set; } = TimeSpan.FromSeconds(1);
|
public TimeSpan AuthTimeout { get; set; } = TimeSpan.FromSeconds(2);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -16,6 +16,6 @@ public class AuthConfigTests
|
|||||||
opts.Users.ShouldBeNull();
|
opts.Users.ShouldBeNull();
|
||||||
opts.NKeys.ShouldBeNull();
|
opts.NKeys.ShouldBeNull();
|
||||||
opts.NoAuthUser.ShouldBeNull();
|
opts.NoAuthUser.ShouldBeNull();
|
||||||
opts.AuthTimeout.ShouldBe(TimeSpan.FromSeconds(1));
|
opts.AuthTimeout.ShouldBe(TimeSpan.FromSeconds(2));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user