From b1ebb4f0febc69750b37de5725b6c99eac1d801c Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:46:12 +0300 Subject: [PATCH] Prove the conversion safety invariant per failure mode The engine already enforces it: verification is unconditional, there is no flag to skip it, and AtomicReplace runs only after it succeeds. What was missing is evidence that it holds for every way a conversion can fail. That gap matters here more than usual. An audit across four corpora found EC reporting success on files whose text had silently changed, and the lesson was not "fix that decoder" but "a safety property nothing tests is a safety property nobody knows they still have". Eight cases, each driving a different failure mode and asserting the same pair: the conversion is refused, and the source file is byte-for-byte what it was. invalid byte sequence refused, unchanged truncated multi-byte sequence refused, unchanged target cannot represent content refused, unchanged post-write verification failure refused, unchanged impossible BOM request refused, unchanged backup creation failure aborted before converting -WhatIf nothing modified at all a provably preserving conversion succeeds The pairing is the point. A refusal that still damaged the file would be worse than no refusal, and counting safe refusals means nothing unless each is verified to have left the source alone. The last case exists so the invariant cannot be satisfied by refusing everything. This is the measurable half of the reviewer's proposed fifth metric: safe refusal is a property with tests rather than a number without them. 320 tests passing. Co-Authored-By: Claude Opus 5 --- README.md | 9 +- .../ConversionSafetyInvariantTests.cs | 259 ++++++++++++++++++ 2 files changed, 266 insertions(+), 2 deletions(-) create mode 100644 sources/EncodingChecker.Tests/ConversionSafetyInvariantTests.cs diff --git a/README.md b/README.md index 15ee0d3..c81202d 100644 --- a/README.md +++ b/README.md @@ -180,11 +180,16 @@ merely happened to be ASCII: **Unicode and ASCII input is safe on this evidence** — not one of the 1,832 files converted from a Unicode or ASCII source came out with different text. -**Legacy input carries the residual risk**, and it is a detection problem rather -than a conversion one: single-byte code pages are mutually decodable, so +**Legacy input carries the residual risk.** The dominant part of it is +source-encoding identification: single-byte code pages are mutually decodable, so `windows-1252` text is perfectly valid `iso-8859-1` text and nothing in the bytes distinguishes them. Forced to the correct codec, those files convert exactly. +Codec implementation differences and decoder strictness are separate conversion +risks rather than the same one, and the two interact — the detector can name the +right encoding and the conversion still alter text, because the implementation +behind that name differs from the reference. + The 89 codec divergences are known Microsoft-vs-Unicode mapping differences in the Japanese and Chinese code pages (U+301C wave dash versus U+FF5E fullwidth tilde, and similar) — properties of .NET's code-page tables, not of this tool. diff --git a/sources/EncodingChecker.Tests/ConversionSafetyInvariantTests.cs b/sources/EncodingChecker.Tests/ConversionSafetyInvariantTests.cs new file mode 100644 index 0000000..8cba834 --- /dev/null +++ b/sources/EncodingChecker.Tests/ConversionSafetyInvariantTests.cs @@ -0,0 +1,259 @@ +using System.Text; + +namespace EncodingChecker.Tests; + +/// +/// The invariant the whole tool rests on: if preservation cannot be proved, the +/// source is not replaced. +/// +/// Every case here drives a different failure mode and asserts the same two +/// things — the conversion is refused, and the original file is byte-for-byte +/// what it was. That pairing is the point. A refusal that still damaged the file +/// would be worse than no refusal at all, and the counting of "safe refusals" +/// means nothing unless each one is verified to have left the source alone. +/// +/// This exists because the invariant was previously enforced but never +/// demonstrated. An audit across four corpora found a defect where conversion +/// reported success on files whose text had silently changed; the lesson taken +/// from it is that a safety property nothing tests is a safety property nobody +/// knows they still have. +/// +public sealed class ConversionSafetyInvariantTests : IDisposable +{ + private readonly string _root = + Directory.CreateTempSubdirectory("ec_safety_").FullName; + + public void Dispose() + { + try + { + Directory.Delete(_root, recursive: true); + } + catch (IOException) + { + // Best-effort cleanup. + } + } + + // EUC-JP bytes carrying a JIS X 0212 sequence introduced by SS3 (0x8F). + // Code page 51932 has no mapping for it. + private static readonly byte[] Unrepresentable = + [0x8F, 0xB0, 0xDF, 0xB9, 0xA5, 0xA1, 0xA4, 0xC0, 0xA4, 0xB3, 0xA6, 0xA1, 0xAA]; + + [Fact] + public void InvalidByteSequence_RefusesAndLeavesTheSourceUnchanged() + { + AssertRefusedAndUnchanged( + "invalid.txt", + Unrepresentable, + source: "euc-jp", + target: "utf-8"); + } + + [Fact] + public void TruncatedMultiByteSequence_RefusesAndLeavesTheSourceUnchanged() + { + // A file that ends mid-character. Strict decoding must reject it rather + // than dropping or substituting the incomplete tail. + byte[] truncated = Encoding.GetEncoding("shift_jis").GetBytes("日本語"); + AssertRefusedAndUnchanged( + "truncated.txt", + truncated[..^1], + source: "shift_jis", + target: "utf-8"); + } + + [Fact] + public void ContentTheTargetCannotRepresent_RefusesAndLeavesTheSourceUnchanged() + { + // The encoder side: CJK has no representation in Windows-1252. + AssertRefusedAndUnchanged( + "unencodable.txt", + Encoding.UTF8.GetBytes("世界 مرحبا Привет"), + source: "utf-8", + target: "windows-1252"); + } + + [Fact] + public void PostWriteVerificationFailure_RefusesAndLeavesTheSourceUnchanged() + { + // An encoding whose code page cannot be rebuilt keeps its own codecs, so + // a substituting encoder reaches the SHA-256 backstop. Whatever the + // route, the file must survive. + string path = Path.Combine(_root, "verify.txt"); + byte[] original = Encoding.UTF8.GetBytes("Привет мир"); + File.WriteAllBytes(path, original); + + ConversionResult result = EncodingConverter.Convert( + path, path, Encoding.UTF8, new SubstitutingEncoding(), new ConversionOptions()); + + Assert.False(result.Success); + Assert.Equal(ConversionErrorCode.UnicodeMismatch, result.ErrorCode); + Assert.False(result.VerificationPassed); + Assert.Equal(original, File.ReadAllBytes(path)); + } + + [Fact] + public void ImpossibleBomRequest_RefusesBeforeTouchingTheFile() + { + // A target with no preamble cannot satisfy WriteBom. This is rejected + // before any I/O, and the file must be untouched either way. + string path = Path.Combine(_root, "nobom.txt"); + byte[] original = Encoding.UTF8.GetBytes("plain ascii text"); + File.WriteAllBytes(path, original); + + ConversionResult result = EncodingConverter.Convert( + path, path, Encoding.UTF8, Encoding.GetEncoding("windows-1252"), + new ConversionOptions { WriteBom = true }); + + Assert.False(result.Success); + Assert.Equal(ConversionErrorCode.BomMismatch, result.ErrorCode); + Assert.Equal(original, File.ReadAllBytes(path)); + } + + [Fact] + public void BackupFailure_AbortsBeforeConvertingAnything() + { + // If the backup cannot be written, the conversion must not proceed: + // a converted file with no recoverable original is the outcome backups + // exist to prevent. A directory occupying the ".bak" path makes the + // copy fail without needing permissions to be manipulated. + string path = Path.Combine(_root, "backupfail.txt"); + byte[] original = Encoding.GetEncoding("windows-1252").GetBytes("café"); + File.WriteAllBytes(path, original); + Directory.CreateDirectory(path + ".bak"); + + var entry = new ConversionReportEntry + { + FilePath = path, + SourceEncoding = "windows-1252", + SourceHasBom = false, + TargetEncoding = "windows-1252", + TargetHasBom = false, + }; + + var completed = new List(); + ScanEngine.ConvertFiles( + [entry], "utf-8", targetWriteBom: false, + ScanEngine.DefaultMaxParallelism, + whatIf: false, backup: true, completed.Add, CancellationToken.None); + + ConversionReportEntry result = Assert.Single(completed); + + Assert.Equal(ConversionRowResult.Error, result.Result); + Assert.Equal(original, File.ReadAllBytes(path)); + } + + [Fact] + public void WhatIf_NeverModifiesAnything() + { + // The dry run must be exactly that, including for a file that would + // convert successfully. + string path = Path.Combine(_root, "whatif.txt"); + byte[] original = Encoding.GetEncoding("windows-1252").GetBytes("café"); + File.WriteAllBytes(path, original); + + var entry = new ConversionReportEntry + { + FilePath = path, + SourceEncoding = "windows-1252", + SourceHasBom = false, + TargetEncoding = "windows-1252", + TargetHasBom = false, + }; + + var completed = new List(); + ScanEngine.ConvertFiles( + [entry], "utf-8", targetWriteBom: false, + ScanEngine.DefaultMaxParallelism, + whatIf: true, backup: false, completed.Add, CancellationToken.None); + + Assert.Equal(original, File.ReadAllBytes(path)); + Assert.False(File.Exists(path + ".bak")); + } + + [Fact] + public void ASuccessfulConversionStillProvesPreservation() + { + // The invariant must not be satisfied by refusing everything. A file + // that can be proved preserved has to convert, and the result has to be + // the same text. + const string text = "café — naïve — 日本語\r\n"; + string path = Path.Combine(_root, "good.txt"); + File.WriteAllBytes(path, Encoding.UTF8.GetBytes(text)); + + ConversionResult result = EncodingConverter.Convert( + path, path, Encoding.UTF8, new UTF8Encoding(false), new ConversionOptions()); + + Assert.True(result.Success, result.ErrorMessage); + Assert.True(result.VerificationPassed); + Assert.Equal(text, Encoding.UTF8.GetString(File.ReadAllBytes(path))); + } + + private void AssertRefusedAndUnchanged( + string name, byte[] content, string source, string target) + { + string path = Path.Combine(_root, name); + File.WriteAllBytes(path, content); + + var entry = new ConversionReportEntry + { + FilePath = path, + SourceEncoding = source, + SourceHasBom = false, + TargetEncoding = source, + TargetHasBom = false, + }; + + var completed = new List(); + ScanEngine.ConvertFiles( + [entry], target, targetWriteBom: false, + ScanEngine.DefaultMaxParallelism, + whatIf: false, backup: false, completed.Add, CancellationToken.None); + + ConversionReportEntry result = Assert.Single(completed); + + Assert.Equal(ConversionRowResult.Error, result.Result); + Assert.Equal(content, File.ReadAllBytes(path)); + } + + /// + /// Substitutes rather than throwing, and reports a code page that cannot be + /// rebuilt, so strict reconstruction cannot rescue it. + /// + private sealed class SubstitutingEncoding : Encoding + { + public override int CodePage => 65_000_003; + + public override int GetByteCount(char[] chars, int index, int count) => count; + + public override int GetBytes( + char[] chars, int charIndex, int charCount, byte[] bytes, int byteIndex) + { + for (int i = 0; i < charCount; i++) + { + char c = chars[charIndex + i]; + bytes[byteIndex + i] = c < 0x80 ? (byte)c : (byte)'?'; + } + + return charCount; + } + + public override int GetCharCount(byte[] bytes, int index, int count) => count; + + public override int GetChars( + byte[] bytes, int byteIndex, int byteCount, char[] chars, int charIndex) + { + for (int i = 0; i < byteCount; i++) + { + chars[charIndex + i] = (char)bytes[byteIndex + i]; + } + + return byteCount; + } + + public override int GetMaxByteCount(int charCount) => charCount; + + public override int GetMaxCharCount(int byteCount) => byteCount; + } +}