diff --git a/DevCycle.SDK.Server.Cloud/Api/DevCycleCloudClient.cs b/DevCycle.SDK.Server.Cloud/Api/DevCycleCloudClient.cs index d81602b..acb6204 100644 --- a/DevCycle.SDK.Server.Cloud/Api/DevCycleCloudClient.cs +++ b/DevCycle.SDK.Server.Cloud/Api/DevCycleCloudClient.cs @@ -103,11 +103,6 @@ public override async Task> Variable(DevCycleUser user, string ke throw new ArgumentException("key cannot be null or empty"); } - if (defaultValue == null) - { - throw new ArgumentNullException(nameof(defaultValue)); - } - AddDefaults(user); string lowerKey = key.ToLower(); diff --git a/DevCycle.SDK.Server.Common/Model/Variable.cs b/DevCycle.SDK.Server.Common/Model/Variable.cs index 7cbdc2e..85539a5 100644 --- a/DevCycle.SDK.Server.Common/Model/Variable.cs +++ b/DevCycle.SDK.Server.Common/Model/Variable.cs @@ -129,7 +129,9 @@ public static TypeEnum DetermineType(T variableValue) try { - var baseType = variableValue.GetType(); + // A null default value is legitimate for JSON variables (and nullable strings), + // so fall back to the declared type rather than dereferencing the value. + var baseType = variableValue?.GetType() ?? typeof(T); if (baseType == typeof(string)) { diff --git a/DevCycle.SDK.Server.Local.MSTests/DevCycleTest.cs b/DevCycle.SDK.Server.Local.MSTests/DevCycleTest.cs index a276a47..06921bf 100644 --- a/DevCycle.SDK.Server.Local.MSTests/DevCycleTest.cs +++ b/DevCycle.SDK.Server.Local.MSTests/DevCycleTest.cs @@ -1,9 +1,10 @@ -using System; +using System; using System.Threading.Tasks; using DevCycle.SDK.Server.Local.Api; using DevCycle.SDK.Server.Common.Model; using DevCycle.SDK.Server.Common.Model.Local; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Newtonsoft.Json.Linq; using Environment = System.Environment; using System.Collections.Generic; using System.Text.Json; @@ -233,6 +234,72 @@ public void Variable_NullUser_ThrowsException() }); } + [TestMethod] + public void Variable_NullKey_ThrowsArgumentException() + { + // Reaching the WASM bucketing engine with a null/empty flag key + // triggers an internal abort() and corrupts the WASM heap. Match + // the Java/Python SDKs (and Cloud client) by failing fast with a + // clear ArgumentException before we ever enter WASM. + using DevCycleLocalClient api = DevCycleTestClient.getTestClient(); + var user = new DevCycleUser("test_user"); + + Assert.Throws(() => api.Variable(user, null, true).Result); + } + + [TestMethod] + public void Variable_EmptyKey_ThrowsArgumentException() + { + using DevCycleLocalClient api = DevCycleTestClient.getTestClient(); + var user = new DevCycleUser("test_user"); + + Assert.Throws(() => api.Variable(user, "", true).Result); + } + + [TestMethod] + public async Task VariableAsync_NullKey_ThrowsArgumentException() + { + using DevCycleLocalClient api = DevCycleTestClient.getTestClient(); + var user = new DevCycleUser("test_user"); + + await Assert.ThrowsExactlyAsync(async () => + await api.VariableAsync(user, null, true)); + } + + [TestMethod] + public async Task VariableAsync_EmptyKey_ThrowsArgumentException() + { + using DevCycleLocalClient api = DevCycleTestClient.getTestClient(); + var user = new DevCycleUser("test_user"); + + await Assert.ThrowsExactlyAsync(async () => + await api.VariableAsync(user, "", true)); + } + + [TestMethod] + public async Task Variable_NullJsonDefaultValue_IsAllowed() + { + // A null default is legitimate for JSON variables, so the key validation + // above must not be extended to defaultValue. + using DevCycleLocalClient api = DevCycleTestClient.getTestClient(); + var user = new DevCycleUser("test_user"); + + var variable = await api.Variable(user, "some_key", null); + + Assert.IsNotNull(variable); + } + + [TestMethod] + public async Task VariableAsync_NullJsonDefaultValue_IsAllowed() + { + using DevCycleLocalClient api = DevCycleTestClient.getTestClient(); + var user = new DevCycleUser("test_user"); + + var variable = await api.VariableAsync(user, "some_key", null); + + Assert.IsNotNull(variable); + } + [TestMethod] public void User_NullUserId_ThrowsException() { diff --git a/DevCycle.SDK.Server.Local/Api/DevCycleLocalClient.cs b/DevCycle.SDK.Server.Local/Api/DevCycleLocalClient.cs index 1c579fb..19d683f 100644 --- a/DevCycle.SDK.Server.Local/Api/DevCycleLocalClient.cs +++ b/DevCycle.SDK.Server.Local/Api/DevCycleLocalClient.cs @@ -330,6 +330,11 @@ public override Task> Variable(DevCycleUser user, string key, T d { var requestUser = new DevCyclePopulatedUser(user); + if (string.IsNullOrEmpty(key)) + { + throw new ArgumentException("key cannot be null or empty"); + } + if (!configManager.Initialized) { logger.LogWarning("Variable called before DevCycleClient has initialized, returning default value"); @@ -389,6 +394,11 @@ public async Task> VariableAsync(DevCycleUser user, string key, T { var requestUser = new DevCyclePopulatedUser(user); + if (string.IsNullOrEmpty(key)) + { + throw new ArgumentException("key cannot be null or empty"); + } + if (!configManager.Initialized) { logger.LogWarning("Variable called before DevCycleClient has initialized, returning default value");