From fec66fa90bc0bec856ad03853f8e4b4c2bd6c1a3 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Fri, 20 Mar 2026 09:36:32 +1100 Subject: [PATCH] Fix crash when reading boolean config values from aspire.config.json When 'aspire config set features.X true' writes a JSON string "true" instead of a JSON boolean, AspireConfigFile.Load() fails to deserialize the Dictionary Features property. Fix by adding a FlexibleBooleanConverter that tolerates both JSON booleans and JSON string representations of booleans. The converter is registered globally via JsonSourceGenerationOptions.Converters so all bool properties benefit. Also fix TryNormalizeSettingsFile to use DeepClone() instead of ToString() when normalizing colon-separated keys, which was corrupting non-string types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Configuration/FlexibleBooleanConverter.cs | 42 +++++++++++++ src/Aspire.Cli/JsonSourceGenerationContext.cs | 3 +- src/Aspire.Cli/Utils/ConfigurationHelper.cs | 6 +- .../Configuration/AspireConfigFileTests.cs | 63 +++++++++++++++++++ .../Configuration/ConfigurationHelperTests.cs | 33 ++++++++++ .../ConfigurationServiceTests.cs | 62 ++++++++++++++++++ 6 files changed, 206 insertions(+), 3 deletions(-) create mode 100644 src/Aspire.Cli/Configuration/FlexibleBooleanConverter.cs diff --git a/src/Aspire.Cli/Configuration/FlexibleBooleanConverter.cs b/src/Aspire.Cli/Configuration/FlexibleBooleanConverter.cs new file mode 100644 index 00000000000..5e75f0d04d3 --- /dev/null +++ b/src/Aspire.Cli/Configuration/FlexibleBooleanConverter.cs @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace Aspire.Cli.Configuration; + +/// +/// A JSON converter for that accepts both actual boolean values +/// (true/false) and string representations ("true"/"false"). +/// This provides backward compatibility for settings files that may have string values +/// written by older CLI versions. +/// +internal sealed class FlexibleBooleanConverter : JsonConverter +{ + public override bool Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + return reader.TokenType switch + { + JsonTokenType.True => true, + JsonTokenType.False => false, + JsonTokenType.String => ParseString(reader.GetString()), + _ => throw new JsonException($"Unexpected token parsing boolean. Token: {reader.TokenType}") + }; + } + + private static bool ParseString(string? value) + { + if (bool.TryParse(value, out var result)) + { + return result; + } + + throw new JsonException($"Invalid boolean value: '{value}'. Expected 'true' or 'false'."); + } + + public override void Write(Utf8JsonWriter writer, bool value, JsonSerializerOptions options) + { + writer.WriteBooleanValue(value); + } +} diff --git a/src/Aspire.Cli/JsonSourceGenerationContext.cs b/src/Aspire.Cli/JsonSourceGenerationContext.cs index ad23050e888..b0fadbcf717 100644 --- a/src/Aspire.Cli/JsonSourceGenerationContext.cs +++ b/src/Aspire.Cli/JsonSourceGenerationContext.cs @@ -18,7 +18,8 @@ namespace Aspire.Cli; WriteIndented = true, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, AllowTrailingCommas = true, - ReadCommentHandling = JsonCommentHandling.Skip)] + ReadCommentHandling = JsonCommentHandling.Skip, + Converters = [typeof(FlexibleBooleanConverter)])] [JsonSerializable(typeof(CliSettings))] [JsonSerializable(typeof(JsonObject))] [JsonSerializable(typeof(ListIntegrationsResponse))] diff --git a/src/Aspire.Cli/Utils/ConfigurationHelper.cs b/src/Aspire.Cli/Utils/ConfigurationHelper.cs index 8d4e4b2235f..354a0bb0696 100644 --- a/src/Aspire.Cli/Utils/ConfigurationHelper.cs +++ b/src/Aspire.Cli/Utils/ConfigurationHelper.cs @@ -154,13 +154,15 @@ internal static bool TryNormalizeSettingsFile(string filePath) } // Find all colon-separated keys at root level - var colonKeys = new List<(string key, string? value)>(); + var colonKeys = new List<(string key, JsonNode? value)>(); foreach (var kvp in settings) { if (kvp.Key.Contains(':')) { - colonKeys.Add((kvp.Key, kvp.Value?.ToString())); + // DeepClone preserves the original JSON type (boolean, number, etc.) + // instead of converting to string via ToString(). + colonKeys.Add((kvp.Key, kvp.Value?.DeepClone())); } } diff --git a/tests/Aspire.Cli.Tests/Configuration/AspireConfigFileTests.cs b/tests/Aspire.Cli.Tests/Configuration/AspireConfigFileTests.cs index ec3d9c4d6f6..f7556f11b89 100644 --- a/tests/Aspire.Cli.Tests/Configuration/AspireConfigFileTests.cs +++ b/tests/Aspire.Cli.Tests/Configuration/AspireConfigFileTests.cs @@ -306,6 +306,69 @@ public void GetIntegrationReferences_IncludesPackagesAndBasePackage() Assert.Contains(refs, r => r.Name == "Aspire.Hosting.Redis"); } + [Fact] + public void Load_ReturnsConfig_WhenFeaturesAreBooleans() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var configPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName); + File.WriteAllText(configPath, """ + { + "features": { "polyglotSupportEnabled": true, "showAllTemplates": false } + } + """); + + var result = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName); + + Assert.NotNull(result); + Assert.NotNull(result.Features); + Assert.True(result.Features["polyglotSupportEnabled"]); + Assert.False(result.Features["showAllTemplates"]); + } + + [Fact] + public void Load_ReturnsConfig_WhenFeaturesAreStringBooleans() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + // Simulates what happens when ConfigurationService.SetNestedValue wrote "true"/"false" as strings + var configPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName); + File.WriteAllText(configPath, """ + { + "features": { "polyglotSupportEnabled": "true", "showAllTemplates": "false" } + } + """); + + var result = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName); + + Assert.NotNull(result); + Assert.NotNull(result.Features); + Assert.True(result.Features["polyglotSupportEnabled"]); + Assert.False(result.Features["showAllTemplates"]); + } + + [Fact] + public void Save_Load_RoundTrips_WithFeatures() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var config = new AspireConfigFile + { + Features = new Dictionary + { + ["polyglotSupportEnabled"] = true, + ["showAllTemplates"] = false + } + }; + + config.Save(workspace.WorkspaceRoot.FullName); + var loaded = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName); + + Assert.NotNull(loaded?.Features); + Assert.True(loaded.Features["polyglotSupportEnabled"]); + Assert.False(loaded.Features["showAllTemplates"]); + } + [Fact] public void Load_RoundTrips_WithProfiles() { diff --git a/tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs b/tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs index cdea30079d2..f65125ca022 100644 --- a/tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs +++ b/tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Text.Json; +using System.Text.Json.Nodes; using Aspire.Cli.Configuration; using Aspire.Cli.Tests.Utils; using Aspire.Cli.Utils; @@ -108,4 +110,35 @@ public void RegisterSettingsFiles_HandlesCommentsAndTrailingCommas() Assert.Equal("MyApp.csproj", config["appHost:path"]); Assert.Equal("daily", config["channel"]); } + + [Fact] + public void TryNormalizeSettingsFile_PreservesBooleanTypes() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var settingsPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName); + // File has a colon-separated key with a boolean value + File.WriteAllText(settingsPath, """ + { + "features:polyglotSupportEnabled": true, + "features:showAllTemplates": false + } + """); + + var normalized = ConfigurationHelper.TryNormalizeSettingsFile(settingsPath); + + Assert.True(normalized); + + var json = JsonNode.Parse(File.ReadAllText(settingsPath)); + var polyglotNode = json!["features"]!["polyglotSupportEnabled"]; + var templatesNode = json!["features"]!["showAllTemplates"]; + Assert.Equal(JsonValueKind.True, polyglotNode!.GetValueKind()); + Assert.Equal(JsonValueKind.False, templatesNode!.GetValueKind()); + + // Verify the file can be loaded by AspireConfigFile without error + var config = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName); + Assert.NotNull(config?.Features); + Assert.True(config.Features["polyglotSupportEnabled"]); + Assert.False(config.Features["showAllTemplates"]); + } } diff --git a/tests/Aspire.Cli.Tests/Configuration/ConfigurationServiceTests.cs b/tests/Aspire.Cli.Tests/Configuration/ConfigurationServiceTests.cs index 8b10267db2b..f0ff9161d10 100644 --- a/tests/Aspire.Cli.Tests/Configuration/ConfigurationServiceTests.cs +++ b/tests/Aspire.Cli.Tests/Configuration/ConfigurationServiceTests.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Text.Json; +using System.Text.Json.Nodes; using Aspire.Cli.Configuration; using Aspire.Cli.Tests.Utils; using Microsoft.Extensions.Configuration; @@ -225,4 +227,64 @@ public async Task SetConfigurationAsync_SetsNestedValues() Assert.Contains("appHost", result); Assert.Contains("MyApp/MyApp.csproj", result); } + + [Fact] + public async Task SetConfigurationAsync_WritesBooleanStringAsJsonString() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var (service, settingsFilePath) = CreateService(workspace, "{}"); + + await service.SetConfigurationAsync("features.polyglotSupportEnabled", "true", isGlobal: false); + + // Value is written as a JSON string "true", not a JSON boolean true. + // The FlexibleBooleanConverter handles parsing "true" -> bool on read. + var json = JsonNode.Parse(File.ReadAllText(settingsFilePath)); + var node = json!["features"]!["polyglotSupportEnabled"]; + Assert.Equal(JsonValueKind.String, node!.GetValueKind()); + Assert.Equal("true", node.GetValue()); + + // Verify round-trip through AspireConfigFile.Load still works + var config = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName); + Assert.NotNull(config?.Features); + Assert.True(config.Features["polyglotSupportEnabled"]); + } + + [Fact] + public async Task SetConfigurationAsync_ChannelWithBooleanLikeValue_StaysAsString() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var (service, settingsFilePath) = CreateService(workspace, "{}"); + + // "true" is a valid channel value and must remain a string in JSON + // to avoid corrupting the string-typed Channel property. + await service.SetConfigurationAsync("channel", "true", isGlobal: false); + + // Must be a JSON string "true", not a JSON boolean true + var json = JsonNode.Parse(File.ReadAllText(settingsFilePath)); + var node = json!["channel"]; + Assert.Equal(JsonValueKind.String, node!.GetValueKind()); + Assert.Equal("true", node.GetValue()); + + // Verify it round-trips correctly through AspireConfigFile.Load + var config = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName); + Assert.NotNull(config); + Assert.Equal("true", config.Channel); + } + + [Fact] + public async Task SetConfigurationAsync_WritesStringValueAsString() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var (service, settingsFilePath) = CreateService(workspace, "{}"); + + await service.SetConfigurationAsync("channel", "daily", isGlobal: false); + + var json = JsonNode.Parse(File.ReadAllText(settingsFilePath)); + var node = json!["channel"]; + Assert.Equal(JsonValueKind.String, node!.GetValueKind()); + Assert.Equal("daily", node.GetValue()); + } }