From a038876d8cb7926f47b4753ae4bcd623f9936546 Mon Sep 17 00:00:00 2001 From: Peter Csajtai Date: Tue, 7 Sep 2021 17:04:31 +0200 Subject: [PATCH 1/2] Ensure internal serializer doesn't use the global json serializer settings --- .../ConfigEvaluatorTestsBase.cs | 3 +- .../DataGovernanceTests.cs | 2 + .../DeserializerTests.cs | 29 ++++++ .../HttpConfigFetcherTests.cs | 9 +- src/ConfigCatClient/ConfigCatClient.cs | 12 ++- src/ConfigCatClient/ConfigDeserializer.cs | 17 +++- src/ConfigCatClient/HttpConfigFetcher.cs | 91 ++++++++++--------- 7 files changed, 106 insertions(+), 57 deletions(-) create mode 100644 src/ConfigCat.Client.Tests/DeserializerTests.cs diff --git a/src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs b/src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs index 45c724ce..15bbc4a7 100644 --- a/src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs +++ b/src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs @@ -1,5 +1,6 @@ using ConfigCat.Client.Evaluate; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Newtonsoft.Json; using System; using System.Collections.Generic; using System.Globalization; @@ -23,7 +24,7 @@ public abstract class ConfigEvaluatorTestsBase public ConfigEvaluatorTestsBase() { - this.configEvaluator = new RolloutEvaluator(logger, new ConfigDeserializer(logger)); + this.configEvaluator = new RolloutEvaluator(logger, new ConfigDeserializer(logger, JsonSerializer.Create())); this.config = new ProjectConfig(this.GetSampleJson(), DateTime.UtcNow, null); } diff --git a/src/ConfigCat.Client.Tests/DataGovernanceTests.cs b/src/ConfigCat.Client.Tests/DataGovernanceTests.cs index fb444e83..10d9c8ca 100644 --- a/src/ConfigCat.Client.Tests/DataGovernanceTests.cs +++ b/src/ConfigCat.Client.Tests/DataGovernanceTests.cs @@ -63,6 +63,7 @@ public async Task WithDataGovernanceSetting_ShouldUseProperCdnUrl(DataGovernance "DEMO", Mock.Of(), handlerMock.Object, + JsonSerializer.Create(), configuration.IsCustomBaseUrl); // Act @@ -378,6 +379,7 @@ internal async Task> Fetch( "DEMO", Mock.Of(), handlerMock.Object, + JsonSerializer.Create(), fetchConfig.IsCustomBaseUrl); // Act diff --git a/src/ConfigCat.Client.Tests/DeserializerTests.cs b/src/ConfigCat.Client.Tests/DeserializerTests.cs new file mode 100644 index 00000000..9679382a --- /dev/null +++ b/src/ConfigCat.Client.Tests/DeserializerTests.cs @@ -0,0 +1,29 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Newtonsoft.Json; +using Newtonsoft.Json.Converters; +using System; +using System.IO; + +namespace ConfigCat.Client.Tests +{ + [TestClass] + public class DeserializerTests + { + [TestMethod] + public void Ensure_Global_Settings_Doesnt_Interfere() + { + JsonConvert.DefaultSettings = () => + { + var settings = new JsonSerializerSettings(); + settings.Converters.Add(new StringEnumConverter { AllowIntegerValues = false }); + return settings; + }; + + var deserializer = new ConfigDeserializer(new LoggerWrapper(new ConsoleLogger(LogLevel.Debug)), JsonSerializer.Create()); + + deserializer.TryDeserialize(new ProjectConfig("{\"p\": {\"u\": \"http://example.com\", \"r\": 0}}", DateTime.Now, ""), out var configs); + + JsonConvert.DefaultSettings = null; + } + } +} diff --git a/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs b/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs index f249e949..5afaaa28 100644 --- a/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs +++ b/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs @@ -1,4 +1,5 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; +using Newtonsoft.Json; using System; using System.Net; using System.Threading.Tasks; @@ -15,7 +16,7 @@ public async Task HttpConfigFetcher_WithCustomHttpClientHandler_ShouldUsePassedH var myHandler = new MyFakeHttpClientHandler(); - var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, false); + var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, JsonSerializer.Create(), false); // Act @@ -33,7 +34,7 @@ public void HttpConfigFetcher_WithCustomHttpClientHandler_HandlersDisposeShouldN var myHandler = new MyFakeHttpClientHandler(); - var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, false); + var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, JsonSerializer.Create(), false); // Act @@ -51,7 +52,7 @@ public async Task HttpConfigFetcher_ResponseHttpCodeIsUnexpected_ShouldReturnsPa var myHandler = new MyFakeHttpClientHandler(HttpStatusCode.Forbidden); - var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, false); + var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, JsonSerializer.Create(), false); var lastConfig = new ProjectConfig("{ }", DateTime.UtcNow, "\"ETAG\""); @@ -71,7 +72,7 @@ public async Task HttpConfigFetcher_ThrowAnException_ShouldReturnPassedConfig() var myHandler = new ExceptionThrowerHttpClientHandler(new WebException()); - var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, false); + var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, JsonSerializer.Create(), false); var lastConfig = new ProjectConfig("{ }", DateTime.UtcNow, "\"ETAG\""); diff --git a/src/ConfigCatClient/ConfigCatClient.cs b/src/ConfigCatClient/ConfigCatClient.cs index f8f17424..88a6c5ad 100644 --- a/src/ConfigCatClient/ConfigCatClient.cs +++ b/src/ConfigCatClient/ConfigCatClient.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using ConfigCat.Client.Cache; using ConfigCat.Client.Security; +using Newtonsoft.Json; namespace ConfigCat.Client { @@ -16,6 +17,8 @@ namespace ConfigCat.Client /// public class ConfigCatClient : IConfigCatClient { + private readonly JsonSerializer serializer; + private readonly ILogger log; private readonly IRolloutEvaluator configEvaluator; @@ -61,7 +64,7 @@ public ConfigCatClient(AutoPollConfiguration configuration) : this((ConfigurationBase)configuration) { var autoPollService = new AutoPollConfigService( - new HttpConfigFetcher(configuration.CreateUri(), "a-" + version, configuration.Logger, configuration.HttpClientHandler, configuration.IsCustomBaseUrl), + new HttpConfigFetcher(configuration.CreateUri(), "a-" + version, configuration.Logger, configuration.HttpClientHandler, this.serializer, configuration.IsCustomBaseUrl), this.cacheParameters, TimeSpan.FromSeconds(configuration.PollIntervalSeconds), TimeSpan.FromSeconds(configuration.MaxInitWaitTimeSeconds), @@ -83,7 +86,7 @@ public ConfigCatClient(LazyLoadConfiguration configuration) : this((ConfigurationBase)configuration) { var lazyLoadService = new LazyLoadConfigService( - new HttpConfigFetcher(configuration.CreateUri(), "l-" + version, configuration.Logger, configuration.HttpClientHandler, configuration.IsCustomBaseUrl), + new HttpConfigFetcher(configuration.CreateUri(), "l-" + version, configuration.Logger, configuration.HttpClientHandler, this.serializer, configuration.IsCustomBaseUrl), this.cacheParameters, configuration.Logger, TimeSpan.FromSeconds(configuration.CacheTimeToLiveSeconds)); @@ -101,7 +104,7 @@ public ConfigCatClient(ManualPollConfiguration configuration) : this((ConfigurationBase)configuration) { var configService = new ManualPollConfigService( - new HttpConfigFetcher(configuration.CreateUri(), "m-" + version, configuration.Logger, configuration.HttpClientHandler, configuration.IsCustomBaseUrl), + new HttpConfigFetcher(configuration.CreateUri(), "m-" + version, configuration.Logger, configuration.HttpClientHandler, this.serializer, configuration.IsCustomBaseUrl), this.cacheParameters, configuration.Logger); @@ -117,8 +120,9 @@ private ConfigCatClient(ConfigurationBase configuration) configuration.Validate(); + this.serializer = JsonSerializer.Create(); this.log = configuration.Logger; - this.configDeserializer = new ConfigDeserializer(this.log); + this.configDeserializer = new ConfigDeserializer(this.log, this.serializer); this.configEvaluator = new RolloutEvaluator(this.log, this.configDeserializer); this.cacheParameters = new CacheParameters { diff --git a/src/ConfigCatClient/ConfigDeserializer.cs b/src/ConfigCatClient/ConfigDeserializer.cs index 7ad35170..34fcd385 100644 --- a/src/ConfigCatClient/ConfigDeserializer.cs +++ b/src/ConfigCatClient/ConfigDeserializer.cs @@ -1,16 +1,19 @@ using ConfigCat.Client.Evaluate; using Newtonsoft.Json; using System.Collections.Generic; +using System.IO; namespace ConfigCat.Client { internal class ConfigDeserializer : IConfigDeserializer { private readonly ILogger logger; + private readonly JsonSerializer serializer; - public ConfigDeserializer(ILogger logger) + public ConfigDeserializer(ILogger logger, JsonSerializer serializer) { this.logger = logger; + this.serializer = serializer; } public bool TryDeserialize(ProjectConfig projectConfig, out IDictionary settings) @@ -18,17 +21,21 @@ public bool TryDeserialize(ProjectConfig projectConfig, out IDictionary(projectConfig.JsonString); + using (var stringReader = new StringReader(projectConfig.JsonString)) + using (var reader = new JsonTextReader(stringReader)) + { + var settingsWithPreferences = this.serializer.Deserialize(reader); - settings = settingsWithPreferences.Settings; + settings = settingsWithPreferences.Settings; - return true; + return true; + } } } } diff --git a/src/ConfigCatClient/HttpConfigFetcher.cs b/src/ConfigCatClient/HttpConfigFetcher.cs index 4c9551f0..08d475b8 100644 --- a/src/ConfigCatClient/HttpConfigFetcher.cs +++ b/src/ConfigCatClient/HttpConfigFetcher.cs @@ -1,4 +1,5 @@ using System; +using System.IO; using System.Net; using System.Net.Http; using System.Net.Http.Headers; @@ -17,14 +18,14 @@ internal sealed class HttpConfigFetcher : IConfigFetcher, IDisposable private readonly ILogger log; private readonly HttpClientHandler httpClientHandler; - + private readonly JsonSerializer serializer; private readonly bool isCustomUri; private HttpClient httpClient; private Uri requestUri; - public HttpConfigFetcher(Uri requestUri, string productVersion, ILogger logger, HttpClientHandler httpClientHandler, bool isCustomUri) + public HttpConfigFetcher(Uri requestUri, string productVersion, ILogger logger, HttpClientHandler httpClientHandler, JsonSerializer serializer, bool isCustomUri) { this.requestUri = requestUri; @@ -33,7 +34,7 @@ public HttpConfigFetcher(Uri requestUri, string productVersion, ILogger logger, this.log = logger; this.httpClientHandler = httpClientHandler; - + this.serializer = serializer; this.isCustomUri = isCustomUri; ReInitializeHttpClient(); @@ -98,53 +99,57 @@ private async Task> FetchRequest(ProjectConfi { var responseBody = await response.Content.ReadAsStringAsync().ConfigureAwait(false); - var body = JsonConvert.DeserializeObject(responseBody); - - if (body?.Preferences != null) + using (var stringReader = new StringReader(responseBody)) + using (var reader = new JsonTextReader(stringReader)) { - var newBaseUrl = body.Preferences.Url; - - if (newBaseUrl == null || requestUri.Host == new Uri(newBaseUrl).Host) - { - return Tuple.Create(response, responseBody); - } - - Evaluate.RedirectMode redirect = body.Preferences.RedirectMode; - - if (isCustomUri && redirect != RedirectMode.Force) - { - return Tuple.Create(response, responseBody); - } - - UpdateRequestUri(new Uri(newBaseUrl)); + var body = this.serializer.Deserialize(reader); - if (redirect == RedirectMode.No) + if (body?.Preferences != null) { - return Tuple.Create(response, responseBody); + var newBaseUrl = body.Preferences.Url; + + if (newBaseUrl == null || requestUri.Host == new Uri(newBaseUrl).Host) + { + return Tuple.Create(response, responseBody); + } + + Evaluate.RedirectMode redirect = body.Preferences.RedirectMode; + + if (isCustomUri && redirect != RedirectMode.Force) + { + return Tuple.Create(response, responseBody); + } + + UpdateRequestUri(new Uri(newBaseUrl)); + + if (redirect == RedirectMode.No) + { + return Tuple.Create(response, responseBody); + } + + if (redirect == RedirectMode.Should) + { + this.log.Warning("Your dataGovernance parameter at ConfigCatClient initialization is not in sync " + + "with your preferences on the ConfigCat Dashboard: " + + "https://app.configcat.com/organization/data-governance. " + + "Only Organization Admins can access this preference."); + } + + if (maxExecutionCount <= 1) + { + log.Error("Redirect loop during config.json fetch. Please contact support@configcat.com."); + return Tuple.Create(response, responseBody); + } + + return await this.FetchRequest( + lastConfig, + ReplaceUri(request.RequestUri, new Uri(newBaseUrl)), + --maxExecutionCount); } - - if (redirect == RedirectMode.Should) + else { - this.log.Warning("Your dataGovernance parameter at ConfigCatClient initialization is not in sync " + - "with your preferences on the ConfigCat Dashboard: " + - "https://app.configcat.com/organization/data-governance. " + - "Only Organization Admins can access this preference."); - } - - if (maxExecutionCount <= 1) - { - log.Error("Redirect loop during config.json fetch. Please contact support@configcat.com."); return Tuple.Create(response, responseBody); } - - return await this.FetchRequest( - lastConfig, - ReplaceUri(request.RequestUri, new Uri(newBaseUrl)), - --maxExecutionCount); - } - else - { - return Tuple.Create(response, responseBody); } } From a323e6948776577fbc9792b7f933525f5a2c8ad8 Mon Sep 17 00:00:00 2001 From: Peter Csajtai Date: Tue, 7 Sep 2021 17:15:17 +0200 Subject: [PATCH 2/2] Update DeserializerTests.cs --- src/ConfigCat.Client.Tests/DeserializerTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ConfigCat.Client.Tests/DeserializerTests.cs b/src/ConfigCat.Client.Tests/DeserializerTests.cs index 9679382a..f6ded2a4 100644 --- a/src/ConfigCat.Client.Tests/DeserializerTests.cs +++ b/src/ConfigCat.Client.Tests/DeserializerTests.cs @@ -22,8 +22,6 @@ public void Ensure_Global_Settings_Doesnt_Interfere() var deserializer = new ConfigDeserializer(new LoggerWrapper(new ConsoleLogger(LogLevel.Debug)), JsonSerializer.Create()); deserializer.TryDeserialize(new ProjectConfig("{\"p\": {\"u\": \"http://example.com\", \"r\": 0}}", DateTime.Now, ""), out var configs); - - JsonConvert.DefaultSettings = null; } } }