From 2e12af64bc84d2e1a98cfde78f2565a7099c3aef Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Wed, 2 Jul 2025 20:17:20 +0100 Subject: [PATCH 1/2] Fix ArgumentNullException when creating a client with optional name Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- src/OpenFeature/EventExecutor.cs | 36 ++++++++++++++----- .../OpenFeatureClientTests.cs | 14 ++++++++ 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/OpenFeature/EventExecutor.cs b/src/OpenFeature/EventExecutor.cs index db2b6fb1..c4df4f63 100644 --- a/src/OpenFeature/EventExecutor.cs +++ b/src/OpenFeature/EventExecutor.cs @@ -14,6 +14,9 @@ internal sealed partial class EventExecutor : IAsyncDisposable private readonly Dictionary _namedProviderReferences = []; private readonly List _activeSubscriptions = []; + /// placeholder for anonymous clients + private static Guid _defaultClientName = Guid.NewGuid(); + private readonly Dictionary> _apiHandlers = []; private readonly Dictionary>> _clientHandlers = []; @@ -58,25 +61,27 @@ internal void RemoveApiLevelHandler(ProviderEventTypes type, EventHandlerDelegat internal void AddClientHandler(string client, ProviderEventTypes eventType, EventHandlerDelegate handler) { + var clientName = GetClientName(client); + lock (this._lockObj) { // check if there is already a list of handlers for the given client and event type - if (!this._clientHandlers.TryGetValue(client, out var registry)) + if (!this._clientHandlers.TryGetValue(clientName, out var registry)) { registry = []; - this._clientHandlers[client] = registry; + this._clientHandlers[clientName] = registry; } - if (!this._clientHandlers[client].TryGetValue(eventType, out var eventHandlers)) + if (!this._clientHandlers[clientName].TryGetValue(eventType, out var eventHandlers)) { eventHandlers = []; - this._clientHandlers[client][eventType] = eventHandlers; + this._clientHandlers[clientName][eventType] = eventHandlers; } - this._clientHandlers[client][eventType].Add(handler); + this._clientHandlers[clientName][eventType].Add(handler); this.EmitOnRegistration( - this._namedProviderReferences.TryGetValue(client, out var clientProviderReference) + this._namedProviderReferences.TryGetValue(clientName, out var clientProviderReference) ? clientProviderReference : this._defaultProvider, eventType, handler); } @@ -84,9 +89,11 @@ internal void AddClientHandler(string client, ProviderEventTypes eventType, Even internal void RemoveClientHandler(string client, ProviderEventTypes type, EventHandlerDelegate handler) { + var clientName = GetClientName(client); + lock (this._lockObj) { - if (this._clientHandlers.TryGetValue(client, out var clientEventHandlers)) + if (this._clientHandlers.TryGetValue(clientName, out var clientEventHandlers)) { if (clientEventHandlers.TryGetValue(type, out var eventHandlers)) { @@ -118,15 +125,18 @@ internal void RegisterClientFeatureProvider(string client, FeatureProvider? prov { return; } + + var clientName = GetClientName(client); + lock (this._lockObj) { FeatureProvider? oldProvider = null; - if (this._namedProviderReferences.TryGetValue(client, out var foundOldProvider)) + if (this._namedProviderReferences.TryGetValue(clientName, out var foundOldProvider)) { oldProvider = foundOldProvider; } - this._namedProviderReferences[client] = provider; + this._namedProviderReferences[clientName] = provider; this.StartListeningAndShutdownOld(provider, oldProvider); } @@ -303,6 +313,14 @@ private void ProcessDefaultProviderHandlers(Event e) } } + private static string GetClientName(string client) + { + if (string.IsNullOrEmpty(client)) + { + return _defaultClientName.ToString(); + } + return client; + } // map events to provider status as per spec: https://openfeature.dev/specification/sections/events/#requirement-535 private static void UpdateProviderStatus(FeatureProvider provider, ProviderEventPayload eventPayload) diff --git a/test/OpenFeature.Tests/OpenFeatureClientTests.cs b/test/OpenFeature.Tests/OpenFeatureClientTests.cs index cbecddc2..b1e95904 100644 --- a/test/OpenFeature.Tests/OpenFeatureClientTests.cs +++ b/test/OpenFeature.Tests/OpenFeatureClientTests.cs @@ -597,6 +597,20 @@ public async Task PassingABlankStringAsTrackingEventName_ThrowsArgumentException Assert.Throws(() => client.Track(" \n ")); } + [Theory] + [InlineData(null)] + [InlineData("")] + public async Task PassingBlankClientName_DoesNotThrowArgumentNullException(string? clientName) + { + var provider = new TestProvider(); + await Api.Instance.SetProviderAsync(provider); + var client = Api.Instance.GetClient(clientName); + + var ex = Record.Exception(() => client.AddHandler(ProviderEventTypes.ProviderReady, (args) => { })); + + Assert.Null(ex); + } + public static TheoryData GenerateMergeEvaluationContextTestData() { const string key = "key"; From 13a41d3127a9e78862d3bd45f93454e5d96efb77 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Wed, 2 Jul 2025 23:42:55 +0100 Subject: [PATCH 2/2] Swap to IsNullOrWhitespace and add test case Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- src/OpenFeature/EventExecutor.cs | 2 +- test/OpenFeature.Tests/OpenFeatureClientTests.cs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenFeature/EventExecutor.cs b/src/OpenFeature/EventExecutor.cs index c4df4f63..f65f41a1 100644 --- a/src/OpenFeature/EventExecutor.cs +++ b/src/OpenFeature/EventExecutor.cs @@ -315,7 +315,7 @@ private void ProcessDefaultProviderHandlers(Event e) private static string GetClientName(string client) { - if (string.IsNullOrEmpty(client)) + if (string.IsNullOrWhiteSpace(client)) { return _defaultClientName.ToString(); } diff --git a/test/OpenFeature.Tests/OpenFeatureClientTests.cs b/test/OpenFeature.Tests/OpenFeatureClientTests.cs index b1e95904..9ea3f0dc 100644 --- a/test/OpenFeature.Tests/OpenFeatureClientTests.cs +++ b/test/OpenFeature.Tests/OpenFeatureClientTests.cs @@ -600,6 +600,7 @@ public async Task PassingABlankStringAsTrackingEventName_ThrowsArgumentException [Theory] [InlineData(null)] [InlineData("")] + [InlineData(" ")] public async Task PassingBlankClientName_DoesNotThrowArgumentNullException(string? clientName) { var provider = new TestProvider();