diff --git a/Generation/Converters/Argumentum.AssetConverter.Tests/Utility/LoggerMarkupSafetyTests.cs b/Generation/Converters/Argumentum.AssetConverter.Tests/Utility/LoggerMarkupSafetyTests.cs new file mode 100644 index 00000000..e4cb1db5 --- /dev/null +++ b/Generation/Converters/Argumentum.AssetConverter.Tests/Utility/LoggerMarkupSafetyTests.cs @@ -0,0 +1,104 @@ +using System; +using System.IO; +using Argumentum.AssetConverter; +using FluentAssertions; +using Spectre.Console; +using Xunit; + +namespace Argumentum.AssetConverter.Tests.Utility +{ + /// + /// Regression tests for issue #630: a message containing square brackets (e.g. the + /// [HARVEST-FAILURE] marker emitted by the #614 per-set resilience path) was fed + /// unescaped to Spectre.Console markup rendering. The StyleParser then threw + /// InvalidOperationException("Could not find color or style 'HARVEST-FAILURE'") + /// from inside the resilience catch block, killing the whole run instead of degrading + /// gracefully. + /// + /// Two guarantees are pinned here: + /// (1) bracketed messages render literally (Markup.Escape on every console path), and + /// (2) Logger.Log never throws on any rendering failure (plain-text fallback), because + /// #614 calls it from catch paths where a throw is fatal to the run. + /// + public class LoggerMarkupSafetyTests + { + /// + /// Routes AnsiConsole to a plain StringWriter for the duration of a test so output can + /// be asserted, then restores the previous console. + /// + private static string CaptureConsole(Action action) + { + var writer = new StringWriter(); + var console = AnsiConsole.Create(new AnsiConsoleSettings + { + Ansi = AnsiSupport.No, + ColorSystem = ColorSystemSupport.NoColors, + Interactive = InteractionSupport.No, + Out = new AnsiConsoleOutput(writer), + }); + var previous = AnsiConsole.Console; + AnsiConsole.Console = console; + try + { + action(); + } + finally + { + AnsiConsole.Console = previous; + } + return writer.ToString(); + } + + [Fact] + public void Log_Problem_WithHarvestFailureMarker_DoesNotThrow_AndRendersLiterally() + { + // Exact shape emitted by HarvestManager's #614 resilience path. + var message = "[HARVEST-FAILURE] Card set 'FallaciesTarot' / 'fr' failed and was skipped: boom"; + + string output = null; + var act = () => { output = CaptureConsole(() => Logger.Log(message, MessageType.Problem)); }; + + act.Should().NotThrow("a bracketed failure marker must not be parsed as Spectre style markup (#630)"); + output.Should().Contain("[HARVEST-FAILURE]", "the marker must survive rendering literally for log greppability"); + } + + [Theory] + [InlineData(MessageType.Title)] + [InlineData(MessageType.Problem)] + [InlineData(MessageType.Instructions)] + [InlineData(MessageType.Explanations)] + [InlineData(MessageType.Warning)] + [InlineData(MessageType.Success)] + [InlineData(MessageType.Info)] + public void Log_AnyMessageType_WithBracketedContent_DoesNotThrow(MessageType messageType) + { + // Brackets show up in real messages: failure markers, file paths, exception text, + // Mustache/template fragments quoted in diagnostics. + var message = "path [C:\\x] marker [HARVEST-FAILURE] template {{field}} style [bold red]"; + + var act = () => CaptureConsole(() => Logger.Log(message, messageType)); + + act.Should().NotThrow($"no console rendering path may propagate for {messageType} (#630)"); + } + + [Fact] + public void Log_InvalidMessageType_StillThrowsArgumentOutOfRange() + { + // The render-failure fallback must not swallow the guard clause for invalid enums. + var act = () => CaptureConsole(() => Logger.Log("x", (MessageType)999)); + + act.Should().Throw(); + } + + [Fact] + public void LogException_WithBracketsInExceptionMessage_DoesNotThrow() + { + var ex = new InvalidOperationException("outer [HARVEST-FAILURE]", + new IOException("inner [bold red] not-a-style")); + + var act = () => CaptureConsole(() => Logger.LogException(ex)); + + act.Should().NotThrow("exception messages routinely contain brackets and are rendered on failure paths"); + } + } +} diff --git a/Generation/Converters/Argumentum.AssetConverter/Logger.cs b/Generation/Converters/Argumentum.AssetConverter/Logger.cs index 53f6cf7b..2698c160 100644 --- a/Generation/Converters/Argumentum.AssetConverter/Logger.cs +++ b/Generation/Converters/Argumentum.AssetConverter/Logger.cs @@ -61,7 +61,7 @@ static Logger() } catch (Exception ex) { - AnsiConsole.MarkupLine($"[bold red]Error during logger initialization: {ex.Message}[/]"); + AnsiConsole.MarkupLine($"[bold red]Error during logger initialization: {Markup.Escape(ex.Message)}[/]"); } } @@ -90,47 +90,57 @@ public static void Log(string message, MessageType messageType = MessageType.Inf { // Log initialization errors to the console to ensure they are visible // as the file logger itself may be the source of the problem. - AnsiConsole.MarkupLine($"[bold red]Failed to write to log file '{LogFile}'. Exception: {ex.Message}[/]"); + AnsiConsole.MarkupLine($"[bold red]Failed to write to log file '{Markup.Escape(LogFile)}'. Exception: {Markup.Escape(ex.Message)}[/]"); } - switch (messageType) + try + { + switch (messageType) + { + case MessageType.Info: + case MessageType.Success: + case MessageType.Warning: + var markup = messageType == MessageType.Info ? "dim" : messageType == MessageType.Warning ? "sandybrown" : "green3"; + if (LogInfo || messageType != MessageType.Info) + { + AnsiConsole.MarkupLine($"{Stopwatch.Elapsed}: [{markup}]{Markup.Escape(message)}[/]"); + } + break; + case MessageType.Title: + AnsiConsole.WriteLine(); + AnsiConsole.WriteLine(); + var rule = new Rule($"[bold]{Markup.Escape(message)}[/]"); + AnsiConsole.Write(rule); + AnsiConsole.WriteLine(); + break; + case MessageType.Problem: + AnsiConsole.WriteLine(); + AnsiConsole.MarkupLine($"[bold red]{Markup.Escape(message)}[/]"); + AnsiConsole.WriteLine(); + break; + case MessageType.Instructions: + case MessageType.Explanations: + AnsiConsole.WriteLine(); + var header = messageType.ToString(); + var color = messageType==MessageType.Instructions ? Color.Yellow : Color.PaleGreen1; + AnsiConsole.Write( + new Panel(Markup.Escape(message)) + .Header(header) + .Collapse() + .RoundedBorder() + .BorderColor(color)); + AnsiConsole.WriteLine(); + break; + default: + throw new ArgumentOutOfRangeException(nameof(messageType), messageType, null); + } + } + catch (Exception renderEx) when (renderEx is not ArgumentOutOfRangeException) { - case MessageType.Info: - case MessageType.Success: - case MessageType.Warning: - var markup = messageType == MessageType.Info ? "dim" : messageType == MessageType.Warning ? "sandybrown" : "green3"; - if (LogInfo || messageType != MessageType.Info) - { - AnsiConsole.MarkupLine($"{Stopwatch.Elapsed}: [{markup}]{Markup.Escape(message)}[/]"); - } - break; - case MessageType.Title: - AnsiConsole.WriteLine(); - AnsiConsole.WriteLine(); - var rule = new Rule($"[bold]{message}[/]"); - AnsiConsole.Write(rule); - AnsiConsole.WriteLine(); - break; - case MessageType.Problem: - AnsiConsole.WriteLine(); - AnsiConsole.MarkupLine($"[bold red]{message}[/]"); - AnsiConsole.WriteLine(); - break; - case MessageType.Instructions: - case MessageType.Explanations: - AnsiConsole.WriteLine(); - var header = messageType.ToString(); - var color = messageType==MessageType.Instructions ? Color.Yellow : Color.PaleGreen1; - AnsiConsole.Write( - new Panel(message) - .Header(header) - .Collapse() - .RoundedBorder() - .BorderColor(color)); - AnsiConsole.WriteLine(); - break; - default: - throw new ArgumentOutOfRangeException(nameof(messageType), messageType, null); + // Console rendering must never propagate (#630): the #614 per-set resilience calls + // Log() from its catch path, so a Spectre StyleParser failure here would kill the + // whole run instead of degrading gracefully. Fall back to plain (markup-free) output. + AnsiConsole.WriteLine($"{Stopwatch.Elapsed}: [{messageType}] {message} (console render failed: {renderEx.Message})"); } @@ -193,7 +203,7 @@ public static void LogException(Exception ex) } catch (Exception logEx) { - AnsiConsole.MarkupLine($"[bold red]Failed to write exception to log file '{LogFile}'. Exception: {logEx.Message}[/]"); + AnsiConsole.MarkupLine($"[bold red]Failed to write exception to log file '{Markup.Escape(LogFile)}'. Exception: {Markup.Escape(logEx.Message)}[/]"); } }