diff --git a/README.md b/README.md index 3262bfb..d1318bb 100644 --- a/README.md +++ b/README.md @@ -112,23 +112,47 @@ Encoding not identified: 0 Refused, ambiguous encoding: 1 Refused, unreadable: 0 -Backups: enabled +Directory: C:\Source Target: utf-8 without BOM +Source encoding: detected per file +Backups: enabled +Guarantees: strict codecs, verified output, atomic install, ambiguity refusal No files modified. ``` +The two indented lines break down `Will convert`; the rest sum exactly to +`Selected`, so the totals can be checked rather than trusted. + `-Apply` carries that plan out. It does not detect anything a second time: the encodings, the target, and the backup setting all come from the plan, so `-BasePath`, `-Target`, `-From`, and `-Backup` are rejected rather than silently -ignored. - -The binding is the point of the feature, not the preview. Every scheduled file -carries the SHA-256 it had when the plan was made, and `-Apply` verifies each one -first. If any file has changed or been deleted in between, **nothing is -converted** — not even the files that still match. A plan reviewed as a whole -belongs to the directory it was reviewed against, and the files most likely to -have changed are the ones something else is actively writing. +ignored. `plan.json` is the whole approval. + +The binding is the point of the feature, not the preview: + +- **Bound to the files.** Every scheduled file carries the SHA-256 it had when + the plan was made, and `-Apply` verifies each one before writing anything. If + any file has changed or been deleted in between, **nothing is converted** — not + even the files that still match. A plan reviewed as a whole belongs to the + directory it was reviewed against, and the files most likely to have changed + are the ones something else is actively writing. Each file is checked once + more at the moment it is installed, which narrows the window between that + verification and the write. +- **Bound to the directory.** Paths are stored relative to a recorded root, so a + plan is a document about a directory rather than about one machine. Applying a + copy of a plan converts the tree it was approved for, not whichever tree it + happens to sit in, and an entry that resolves outside that root is refused. +- **Bound to the conversion.** The plan records the target encoding, BOM policy, + backup policy, whether the source encoding was detected or specified, and a + semantics version describing the conversion behaviour it was approved under. A + plan written under different behaviour is refused rather than carried out — + what was approved was a conversion, not a list of filenames. + +The semantics version is deliberately separate from EC's version number: it moves +only when conversion or classification behaviour changes, so a release that +changes nothing about conversion does not invalidate plans and teach people to +work around the check. ```bash EncodingChecker.exe -BasePath . -Include "*" -Target "utf-8" -Plan plan.json @@ -197,10 +221,15 @@ These are the guarantees the implementation actually provides. bytes is refused rather than guessed at, when the competing encodings would produce different text. `-From` overrides the detection, not the conversion safeguards. See [Ambiguous encodings](#ambiguous-encodings-and--from). -- A plan written by `-Plan` is bound to the SHA-256 of every file it schedules. - `-Apply` verifies all of them before writing anything and refuses the plan - whole if any file has changed, so a decision made about one set of bytes is - never applied to a different one. +- A plan written by `-Plan` is bound to the SHA-256 of every file it schedules, + to the directory those files are under, and to the conversion behaviour it was + approved under. `-Apply` verifies all of them before writing anything and + refuses the plan whole if any has changed, so a decision made about one set of + bytes is never applied to a different one. Under `-Apply`, each source is + re-hashed again immediately before installation. **This narrows the window + between verification and write; it does not close it** — a source rewritten + between that check and the replacement is still not detected, which would + require holding every source open against writers for the whole run. - `-BasePath` itself is rejected if it is a symbolic link, junction, or other reparse point. Reparse-point subdirectories are skipped during traversal, and a file that is (or becomes) a reparse point is rejected at diff --git a/sources/EncodingChecker.Tests/ConversionPlanTests.cs b/sources/EncodingChecker.Tests/ConversionPlanTests.cs index cb83447..6e77ad4 100644 --- a/sources/EncodingChecker.Tests/ConversionPlanTests.cs +++ b/sources/EncodingChecker.Tests/ConversionPlanTests.cs @@ -82,7 +82,20 @@ private ConversionPlan LoadPlan() private PlannedFile PlannedFor(string name) => Assert.Single( LoadPlan().Files, - f => string.Equals(Path.GetFileName(f.Path), name, StringComparison.Ordinal)); + f => string.Equals(f.RelativePath, name, StringComparison.Ordinal)); + + /// Edits the plan on disk, the way someone with a text editor would. + private void Rewrite(Action> edit) + { + using JsonDocument document = JsonDocument.Parse(File.ReadAllText(PlanPath)); + + Dictionary fields = document.RootElement + .EnumerateObject() + .ToDictionary(p => p.Name, p => p.Value.Clone()); + + edit(fields); + File.WriteAllText(PlanPath, JsonSerializer.Serialize(fields)); + } [Fact] public void PlanningWritesNothing() @@ -241,15 +254,7 @@ public void APlanFromAFutureVersionIsRefusedRatherThanGuessedAt() Write("jp.txt", "こんにちは世界。テキスト", "shift_jis"); Assert.Equal(0, Plan()); - using (JsonDocument document = JsonDocument.Parse(File.ReadAllText(PlanPath))) - { - Dictionary fields = document.RootElement - .EnumerateObject() - .ToDictionary(p => p.Name, p => p.Value.Clone()); - - fields["PlanVersion"] = JsonSerializer.SerializeToElement(99); - File.WriteAllText(PlanPath, JsonSerializer.Serialize(fields)); - } + Rewrite(fields => fields["PlanVersion"] = JsonSerializer.SerializeToElement(99)); Assert.Equal(1, Run("-Apply", PlanPath)); } @@ -295,6 +300,195 @@ public void PlanIsRejectedInModesThatConvertNothing(params string[] mode) ["-BasePath", _root, "-Target", "utf-8", "-Plan", PlanPath, .. mode])); } + [Fact] + public void ThePlanDescribesTheConversionAndNotOnlyTheFiles() + { + // "-Apply plan.json" must need no ambient option state to mean something exact, + // so everything that shapes the conversion has to be in the file. + Write("jp.txt", "こんにちは世界。テキスト", "shift_jis"); + + Assert.Equal(0, Plan("-Backup")); + + ConversionPlan plan = LoadPlan(); + + Assert.Equal(ConversionPlan.CurrentPlanVersion, plan.PlanVersion); + Assert.Equal(ConversionSemantics.Current, plan.SemanticsVersion); + Assert.Equal("utf-8", plan.TargetEncoding); + Assert.False(plan.TargetHasBom); + Assert.True(plan.BackupEnabled); + Assert.Equal("Detected", plan.DetectionMode); + Assert.Equal(Path.GetFullPath(_root), plan.BaseDirectory); + Assert.NotEmpty(plan.ECVersion); + + Assert.True(plan.Semantics.StrictDecoding); + Assert.True(plan.Semantics.StrictEncoding); + Assert.True(plan.Semantics.OutputVerification); + Assert.True(plan.Semantics.AtomicInstall); + Assert.True(plan.Semantics.AmbiguityRefusal); + } + + [Fact] + public void ExplicitSourceSelectionIsRecordedAsSuch() + { + Write("jp.txt", "こんにちは世界。テキスト", "shift_jis"); + + Assert.Equal(0, Plan("-From", "shift_jis")); + + ConversionPlan plan = LoadPlan(); + + Assert.Equal("Explicit", plan.DetectionMode); + Assert.Equal("shift_jis", plan.ExplicitSourceEncoding); + Assert.True(Assert.Single(plan.Files).SourceWasSpecified); + Assert.Contains("detection bypassed", plan.Summarize()); + } + + [Fact] + public void APlanMadeUnderDifferentConversionBehaviourIsRefused() + { + // The schema can be identical while the conversion it describes is not. What the + // user approved was a conversion, not a file listing. + string path = Write("jp.txt", "こんにちは世界。テキスト", "shift_jis"); + byte[] original = File.ReadAllBytes(path); + + Assert.Equal(0, Plan()); + Rewrite(fields => + fields["SemanticsVersion"] = + JsonSerializer.SerializeToElement(ConversionSemantics.Current + 1)); + + Assert.Equal(1, Run("-Apply", PlanPath)); + Assert.Equal(original, File.ReadAllBytes(path)); + + Assert.Null(ConversionPlan.Load(PlanPath, out string? error)); + Assert.Contains("different conversion behaviour", error); + } + + [Fact] + public void APlanAppliedFromACopyStillConvertsTheTreeItWasApprovedFor() + { + // A plan carrying absolute paths that is copied alongside its tree still names + // the original tree, and every hash matches, because those are the files it was + // made from. Resolving against the recorded root makes the intent explicit + // instead of incidental: the plan is about one directory, and says which. + const string text = "こんにちは世界。テキスト"; + string original = Write("jp.txt", text, "shift_jis"); + + Assert.Equal(0, Plan()); + + string copy = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(copy); + + try + { + string copiedPlan = Path.Combine(copy, "plan.json"); + File.Copy(original, Path.Combine(copy, "jp.txt")); + File.Copy(PlanPath, copiedPlan); + + Assert.Equal(0, Run("-Apply", copiedPlan)); + + // The tree the plan named was converted; the copy was not touched. + Assert.Equal(text, Encoding.UTF8.GetString(File.ReadAllBytes(original))); + Assert.Equal( + Encoding.GetEncoding("shift_jis").GetBytes(text), + File.ReadAllBytes(Path.Combine(copy, "jp.txt"))); + } + finally + { + Directory.Delete(copy, recursive: true); + } + } + + [Fact] + public void APlanWhoseDirectoryIsGoneIsRefusedRatherThanResolvedElsewhere() + { + Write("jp.txt", "こんにちは世界。テキスト", "shift_jis"); + Assert.Equal(0, Plan()); + + Rewrite(fields => fields["BaseDirectory"] = JsonSerializer.SerializeToElement( + Path.Combine(_root, "no-such-directory"))); + + Assert.Equal(3, Run("-Apply", PlanPath)); + } + + [Fact] + public void AnEntryReachingOutsideThePlansDirectoryIsRefused() + { + // A plan is an ordinary file that anyone can edit or receive from someone else. + // Its paths must stay inside the directory it claims to be about. + string outside = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + byte[] content = Encoding.GetEncoding("shift_jis").GetBytes("こんにちは世界。テキスト"); + File.WriteAllBytes(outside, content); + + try + { + Write("jp.txt", "こんにちは世界。テキスト", "shift_jis"); + Assert.Equal(0, Plan()); + + Rewrite(fields => + { + List files = [.. fields["Files"].EnumerateArray()]; + + Dictionary entry = files[0] + .EnumerateObject() + .ToDictionary(p => p.Name, p => p.Value.Clone()); + + entry["RelativePath"] = JsonSerializer.SerializeToElement( + Path.GetRelativePath(_root, outside)); + entry["Sha256"] = JsonSerializer.SerializeToElement( + ConversionMetadataStore.ComputeSha256(outside)); + + fields["Files"] = JsonSerializer.SerializeToElement(new[] { entry }); + }); + + Assert.Equal(3, Run("-Apply", PlanPath)); + + // Untouched, and still Shift_JIS rather than the UTF-8 it would have become. + Assert.Equal(content, File.ReadAllBytes(outside)); + } + finally + { + File.Delete(outside); + } + } + + [Fact] + public void TheDisplayedCategoriesSumToTheSelectedPopulation() + { + // A category total that does not add up is how a mechanism that is actually safe + // loses the confidence it earned. + Write("jp.txt", "こんにちは世界。日本語のテキストです。", "shift_jis"); + Write("ambiguous.txt", "Le café était déjà prêt", "windows-1252"); + Write("plain.txt", "just ascii here", "ascii"); + + Assert.Equal(0, Plan()); + + ConversionPlan plan = LoadPlan(); + string summary = plan.Summarize(); + + int Line(string label) + { + string line = Assert.Single( + summary.Split(Environment.NewLine), + l => l.StartsWith(label, StringComparison.Ordinal)); + + return int.Parse(line[label.Length..].Trim()); + } + + // The indented pair breaks down "Will convert" and is not part of the sum. + Assert.Equal( + Line("Selected:"), + Line("Will convert:") + + Line("Already in target encoding:") + + Line("Encoding not identified:") + + Line("Refused, ambiguous encoding:") + + Line("Refused, unreadable:")); + + Assert.Equal( + Line("Will convert:"), + Line(" encoding determined:") + Line(" same text either way:")); + + Assert.Equal(plan.Files.Count, Line("Selected:")); + } + [Fact] public void TheSummaryAccountsForEverySelectedFile() { diff --git a/sources/EncodingChecker.Tests/ConversionSafetyInvariantTests.cs b/sources/EncodingChecker.Tests/ConversionSafetyInvariantTests.cs index 8cba834..417d0de 100644 --- a/sources/EncodingChecker.Tests/ConversionSafetyInvariantTests.cs +++ b/sources/EncodingChecker.Tests/ConversionSafetyInvariantTests.cs @@ -1,4 +1,4 @@ -using System.Text; +using System.Text; namespace EncodingChecker.Tests; @@ -190,6 +190,83 @@ public void ASuccessfulConversionStillProvesPreservation() Assert.Equal(text, Encoding.UTF8.GetString(File.ReadAllBytes(path))); } + [Fact] + public void SourceThatNoLongerMatchesWhatWasApproved_RefusesAtInstallation() + { + // The preflight in -Apply proves every file matched when the run started. A + // large tree can take a while after that, and the length-and-timestamp recheck + // inside the converter only compares against what this run itself saw on + // opening the file - it cannot speak to a decision made before that. + string path = Path.Combine(_root, "approved.txt"); + byte[] original = Encoding.GetEncoding("shift_jis").GetBytes("こんにちは世界"); + File.WriteAllBytes(path, original); + + ConversionResult result = EncodingConverter.Convert( + path, + path, + Encoding.GetEncoding("shift_jis"), + Encoding.UTF8, + new ConversionOptions + { + // The hash of something else entirely: what an earlier plan would have + // recorded for a file that has since been rewritten. + ExpectedSourceSha256 = new string('0', 64), + }, + progress: null, + CancellationToken.None); + + Assert.False(result.Success); + Assert.Equal(ConversionErrorCode.SourceChangedDuringConversion, result.ErrorCode); + Assert.False(result.ReplacementCommitted); + Assert.Equal(original, File.ReadAllBytes(path)); + } + + [Fact] + public void SourceThatStillMatchesWhatWasApproved_Converts() + { + // The other direction, which matters just as much: a check that refuses + // everything is not a safety feature, it is a broken one. + const string text = "こんにちは世界"; + string path = Path.Combine(_root, "unchanged.txt"); + File.WriteAllBytes(path, Encoding.GetEncoding("shift_jis").GetBytes(text)); + + ConversionResult result = EncodingConverter.Convert( + path, + path, + Encoding.GetEncoding("shift_jis"), + Encoding.UTF8, + new ConversionOptions + { + ExpectedSourceSha256 = ConversionMetadataStore.ComputeSha256(path), + }, + progress: null, + CancellationToken.None); + + Assert.True(result.Success); + Assert.Equal(text, Encoding.UTF8.GetString(File.ReadAllBytes(path))); + } + + [Fact] + public void AnOrdinaryConversionDoesNotPayForTheApprovalCheck() + { + // The stronger check costs a second full read of every file. Conversions that + // nothing committed to in advance have nothing to compare against, so they keep + // the length-and-timestamp recheck and skip the extra pass. + const string text = "こんにちは世界"; + string path = Path.Combine(_root, "ordinary.txt"); + File.WriteAllBytes(path, Encoding.GetEncoding("shift_jis").GetBytes(text)); + + Assert.Null(ConversionOptions.Default.ExpectedSourceSha256); + + ConversionResult result = EncodingConverter.Convert( + path, path, + Encoding.GetEncoding("shift_jis"), Encoding.UTF8, + ConversionOptions.Default, progress: null, CancellationToken.None); + + Assert.True(result.Success); + Assert.Equal(text, Encoding.UTF8.GetString(File.ReadAllBytes(path))); + } + private void AssertRefusedAndUnchanged( string name, byte[] content, string source, string target) { diff --git a/sources/EncodingChecker/ConversionPlan.cs b/sources/EncodingChecker/ConversionPlan.cs index d5a02e3..5188f82 100644 --- a/sources/EncodingChecker/ConversionPlan.cs +++ b/sources/EncodingChecker/ConversionPlan.cs @@ -26,10 +26,76 @@ internal enum PlannedAction Refuse, } +/// +/// What a conversion carried out by this build of EC guarantees. +/// +/// +/// Recorded in every plan so that -Apply is not merely repeating a list of files +/// but re-asserting the conversion those files were approved for. None of these are +/// user-settable today; they are written down because a plan approved under them must +/// not be carried out by a build that no longer provides them. +/// +internal sealed record ConversionSemantics +{ + /// + /// Bumped whenever conversion or classification behaviour changes in a way that + /// makes an older plan's decisions no longer the ones this build would make. + /// + /// + /// Deliberately separate from the assembly version. Tying plan validity to the + /// version number would invalidate every plan on a release that changed nothing + /// about conversion, which teaches people to work around the check rather than read + /// it. This moves only when the meaning of a plan moves. + /// + internal const int Current = 1; + + /// + /// The guarantees of , in words, for a person reading a summary. + /// + /// + /// Comes from the build rather than from a loaded plan's booleans on purpose. A plan + /// is an editable file; describing what it claims about itself would let an edited + /// one state something untrue about the conversion that is actually going to happen. + /// A plan only reaches a summary once its semantics version has been accepted, so + /// describing this build is describing that plan. + /// + internal const string Describes = + "strict codecs, verified output, atomic install, ambiguity refusal"; + + /// Malformed input is rejected rather than replaced. + public bool StrictDecoding { get; init; } = true; + + /// Content the target cannot represent is rejected, not substituted. + public bool StrictEncoding { get; init; } = true; + + /// The output is re-decoded and compared before it is installed. + public bool OutputVerification { get; init; } = true; + + /// The source is never rewritten in place. + public bool AtomicInstall { get; init; } = true; + + /// + /// Files whose bytes do not identify their encoding, where the rival readings + /// disagree about the text, are refused rather than converted on a guess. + /// + public bool AmbiguityRefusal { get; init; } = true; +} + /// One file's entry in a conversion plan. internal sealed record PlannedFile { - public required string Path { get; init; } + /// + /// The file's path relative to the plan's . + /// + /// + /// The identity, in place of an absolute path. A plan carrying absolute paths that is + /// copied alongside its tree still names the original tree, so applying it from the + /// copy would convert the files somewhere else - and every hash would match, because + /// those are the files the plan was made from. Resolving against the recorded root + /// makes that impossible to do by accident, and makes the plan legible as a document + /// about a directory rather than about one machine. + /// + public required string RelativePath { get; init; } public required long Size { get; init; } @@ -76,15 +142,34 @@ internal sealed record PlannedFile /// had when the plan was made, and applying the plan verifies each one. A file that /// changed in between invalidates the plan rather than being converted on the strength of /// a decision made about different bytes. +/// +/// The plan also describes the conversion itself - root, target, BOM policy, source +/// encoding and how it was arrived at, backup policy, and the guarantees the converting +/// build provides - so that applying it needs no ambient option state at all. The file is +/// the whole approval. +/// /// internal sealed record ConversionPlan { - public int PlanVersion { get; init; } = 1; + /// The plan file's schema. Changed only when this shape changes. + internal const int CurrentPlanVersion = 2; + + public int PlanVersion { get; init; } = CurrentPlanVersion; + + /// The conversion behaviour this plan was made under. Checked on apply. + public int SemanticsVersion { get; init; } = ConversionSemantics.Current; public required string CreatedUtc { get; init; } public required string ECVersion { get; init; } + /// + /// What the conversion guaranteed when the plan was approved. Recorded for the + /// reader; is what the tool enforces. + /// + public ConversionSemantics Semantics { get; init; } = new(); + + /// The directory every is under. public required string BaseDirectory { get; init; } public required string TargetEncoding { get; init; } @@ -96,6 +181,12 @@ internal sealed record ConversionPlan /// The source encoding named by the caller, if any. public string? ExplicitSourceEncoding { get; init; } + /// + /// Whether the source encoding was chosen by the caller or worked out from the bytes. + /// + public string DetectionMode => + string.IsNullOrEmpty(ExplicitSourceEncoding) ? "Detected" : "Explicit"; + public required IReadOnlyList Files { get; init; } private static readonly JsonSerializerOptions Options = new() @@ -111,6 +202,7 @@ internal static ConversionPlan FromEntries( bool backupEnabled, string? explicitSource) { + string root = Path.TrimEndingDirectorySeparator(Path.GetFullPath(baseDirectory)); var files = new List(); foreach (ConversionReportEntry entry in entries) @@ -153,7 +245,7 @@ internal static ConversionPlan FromEntries( files.Add(new PlannedFile { - Path = entry.FilePath, + RelativePath = Path.GetRelativePath(root, entry.FilePath), Size = size, Sha256 = hash, Action = action, @@ -173,7 +265,7 @@ internal static ConversionPlan FromEntries( CreatedUtc = DateTime.UtcNow.ToString("O", CultureInfo.InvariantCulture), ECVersion = typeof(ConversionPlan).Assembly.GetName().Version?.ToString() ?? "unknown", - BaseDirectory = baseDirectory, + BaseDirectory = root, TargetEncoding = targetEncoding, TargetHasBom = targetHasBom, BackupEnabled = backupEnabled, @@ -210,9 +302,30 @@ internal static ConversionPlan FromEntries( return null; } - if (plan.PlanVersion != 1) + if (plan.PlanVersion != CurrentPlanVersion) + { + error = $"This plan uses schema version {plan.PlanVersion}; this build " + + $"writes and reads version {CurrentPlanVersion}. Re-run -Plan " + + "to produce one it can carry out."; + return null; + } + + // The schema can be identical while the conversion it describes is not. A + // plan approved under different behaviour is not this build's plan, whatever + // its file format says. + if (plan.SemanticsVersion != ConversionSemantics.Current) + { + error = "This plan was made under different conversion behaviour " + + $"(semantics version {plan.SemanticsVersion}; this build uses " + + $"{ConversionSemantics.Current}, and the plan was written by " + + $"EC {plan.ECVersion}). What it approved is not what this " + + "build would do. Re-run -Plan and review the result."; + return null; + } + + if (plan.Files is null) { - error = $"Plan version {plan.PlanVersion} is not supported."; + error = $"'{path}' does not list any files."; return null; } @@ -227,6 +340,36 @@ internal static ConversionPlan FromEntries( } } + /// + /// The absolute path of a planned file, resolved against the plan's own root. + /// + /// + /// The full path, or if the entry resolves outside that root - + /// a plan is an ordinary file that anyone can edit, and one naming + /// ..\..\Windows\System32 must not reach outside the directory it claims to + /// be about. + /// + internal string? ResolvePath(PlannedFile file) + { + string root = Path.TrimEndingDirectorySeparator(Path.GetFullPath(BaseDirectory)); + string full; + + try + { + full = Path.GetFullPath(Path.Combine(root, file.RelativePath)); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException) + { + return null; + } + + return full.StartsWith( + root + Path.DirectorySeparatorChar, + StringComparison.OrdinalIgnoreCase) + ? full + : null; + } + /// /// Confirms every file is still exactly what it was when the plan was made. /// @@ -248,20 +391,28 @@ internal IReadOnlyList FindStaleFiles() if (file.Action != PlannedAction.Convert) continue; + string? path = ResolvePath(file); + + if (path is null) + { + stale.Add($"{file.RelativePath} (resolves outside the plan's directory)"); + continue; + } + try { - if (!File.Exists(file.Path)) + if (!File.Exists(path)) { - stale.Add($"{file.Path} (no longer exists)"); + stale.Add($"{path} (no longer exists)"); continue; } - if (ConversionMetadataStore.ComputeSha256(file.Path) != file.Sha256) - stale.Add($"{file.Path} (contents changed since the plan was made)"); + if (ConversionMetadataStore.ComputeSha256(path) != file.Sha256) + stale.Add($"{path} (contents changed since the plan was made)"); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - stale.Add($"{file.Path} ({ex.Message})"); + stale.Add($"{path} ({ex.Message})"); } } @@ -271,8 +422,9 @@ internal IReadOnlyList FindStaleFiles() /// The summary a person reads before deciding. /// /// Written so the counts add up on the page. A reader who cannot see that the - /// sub-totals sum to the whole has to trust the numbers instead of checking them, - /// which is the opposite of what a preflight is for. + /// categories sum to the whole has to trust the numbers instead of checking them, + /// which is the opposite of what a preflight is for. The two indented lines break + /// down the one above them and are not part of that sum. /// internal string Summarize() { @@ -297,17 +449,19 @@ internal string Summarize() $"Refused, ambiguous encoding: {changing}", $"Refused, unreadable: {otherRefusals}", string.Empty, - $"Backups: {(BackupEnabled ? "enabled" : "DISABLED")}", + $"Directory: {BaseDirectory}", $"Target: {TargetEncoding}" + (TargetHasBom ? " with BOM" : " without BOM"), + "Source encoding: " + + (string.IsNullOrEmpty(ExplicitSourceEncoding) + ? "detected per file" + : $"{ExplicitSourceEncoding} (specified; detection bypassed)"), + $"Backups: {(BackupEnabled ? "enabled" : "DISABLED")}", + $"Guarantees: {ConversionSemantics.Describes}", + string.Empty, + "No files modified.", }; - if (!string.IsNullOrEmpty(ExplicitSourceEncoding)) - lines.Add($"Source encoding: {ExplicitSourceEncoding} (specified)"); - - lines.Add(string.Empty); - lines.Add("No files modified."); - return string.Join(Environment.NewLine, lines); } } diff --git a/sources/EncodingChecker/ConversionReport.cs b/sources/EncodingChecker/ConversionReport.cs index b35d3cb..47ac39b 100644 --- a/sources/EncodingChecker/ConversionReport.cs +++ b/sources/EncodingChecker/ConversionReport.cs @@ -79,6 +79,14 @@ internal sealed class ConversionReportEntry /// internal bool SourceEncodingWasSpecified { get; set; } + /// + /// The SHA-256 this file is required to still have when it is installed, or + /// when nothing earlier committed to its contents. Set when + /// the conversion was approved in advance by a plan. + /// Internal state; not included in CSV output. + /// + internal string? ExpectedSourceSha256 { get; set; } + /// Additional error detail; not included in CSV output. internal string? Diagnostic { get; set; } } diff --git a/sources/EncodingChecker/EncodingConverter.cs b/sources/EncodingChecker/EncodingConverter.cs index 9aa82dd..41b4fd6 100644 --- a/sources/EncodingChecker/EncodingConverter.cs +++ b/sources/EncodingChecker/EncodingConverter.cs @@ -73,6 +73,25 @@ internal sealed record ConversionOptions /// A failure here therefore aborts the conversion with the original intact. /// internal Func? RecordConversion { get; init; } + + /// + /// The SHA-256 the source is required to still have at the moment of installation, + /// or to skip the check. + /// + /// + /// Set when the decision to convert this file was made earlier, against bytes that + /// were read then - which is what a conversion plan is. The length-and-timestamp + /// recheck above catches a source rewritten during conversion, but it compares + /// against what this run itself observed on opening the file, so it cannot speak to + /// anything that happened before that. A plan can. + /// + /// This narrows the window rather than closing it: a source rewritten between this + /// check and File.Replace is still not detected. Eliminating that entirely + /// needs the source held open against writers for the whole conversion, which would + /// fail conversions of files legitimately open elsewhere. + /// + /// + internal string? ExpectedSourceSha256 { get; init; } } /// @@ -405,11 +424,13 @@ internal static ConversionResult Convert( // purpose: the backup already exists, the conversion is verified, and the // original is still in place, so a failure to record leaves nothing to // recover from and nothing needing recovery. - if (options.RecordConversion is not null) + // Hashed only when something asks for it, so the extra read is not paid for + // by conversions that need neither the check nor the record. + string sourceFileSha = string.Empty; + + if (options.ExpectedSourceSha256 is not null || + options.RecordConversion is not null) { - // Hashed only when a record is being written, so the extra pass is not - // paid for by conversions that do not need it. - string sourceFileSha; try { sourceFileSha = ComputeFileSha256(sourcePath); @@ -419,7 +440,33 @@ internal static ConversionResult Convert( { sourceFileSha = string.Empty; } + } + // The last point at which nothing has been installed. A caller that decided + // to convert these bytes earlier gets to insist they are still those bytes. + if (options.ExpectedSourceSha256 is not null && + !string.Equals( + sourceFileSha, + options.ExpectedSourceSha256, + StringComparison.OrdinalIgnoreCase)) + { + return Failure( + ConversionErrorCode.SourceChangedDuringConversion, + sourceFileSha.Length == 0 + ? "The source file could not be re-read to confirm it still " + + "matches the one this conversion was approved for." + : "The source file no longer matches the one this conversion " + + "was approved for; it changed before installation.", + sourceEncoding, + targetEncoding) with + { + SourceBytes = sourceBytesProcessed, + TargetBytes = targetBytesWritten, + }; + } + + if (options.RecordConversion is not null) + { string? recordError = options.RecordConversion(new ConversionRecord { SourcePath = sourcePath, diff --git a/sources/EncodingChecker/Program.cs b/sources/EncodingChecker/Program.cs index cb52d11..c319e5f 100644 --- a/sources/EncodingChecker/Program.cs +++ b/sources/EncodingChecker/Program.cs @@ -157,17 +157,27 @@ Write a conversion plan and change nothing. The plan whether the bytes identify it uniquely, and which files could come out with different text. + It also records the conversion itself - directory, + target encoding, BOM policy, backup policy, and the + guarantees this build provides - so the file is the + whole approval and needs no other options to mean + something exact. + [-Apply ] Carry out a plan written by -Plan. Every file is checked against the hash it had when the plan was - made; if any has changed, nothing is converted. A - plan approved for one set of files is not applied to - a different one. + made; if any has changed, nothing is converted at + all. A plan approved for one set of files is not + applied to a different one. Each file is checked + once more at the moment it is installed. Nothing is detected a second time: the encodings, the target, and the backup setting all come from the plan, so -BasePath, -Target, -From, and -Backup are - rejected here rather than silently ignored. + rejected here rather than silently ignored. A plan + written under different conversion behaviour, or by + an incompatible schema, is refused rather than + guessed at. Modes: Conversion is the default mode. @@ -260,6 +270,15 @@ private static int ApplyPlan(CliOptions options) return 1; } + // Every path in the plan is relative to this, so if it is gone there is nothing + // to resolve them against and no way to tell which tree was meant. + if (!Directory.Exists(plan.BaseDirectory)) + { + Console.Error.WriteLine( + $"The plan's directory no longer exists: {plan.BaseDirectory}"); + return 3; + } + // The whole reason a plan exists. Re-detecting here would make the preview a // demonstration rather than a promise: a second pass can reach different // conclusions, and it was the first that the user approved. @@ -288,7 +307,7 @@ .. plan.Files .Where(f => f.Action == PlannedAction.Convert) .Select(f => new ConversionReportEntry { - FilePath = f.Path, + FilePath = plan.ResolvePath(f)!, SourceEncoding = f.SourceEncoding, SourceHasBom = f.SourceHasBom, TargetEncoding = plan.TargetEncoding, @@ -299,6 +318,11 @@ .. plan.Files AmbiguityReason = f.AmbiguityReason, CompetingEncodings = f.CompetingEncodings, SourceEncodingWasSpecified = f.SourceWasSpecified, + + // Checked again at the moment of installation. FindStaleFiles above + // proved the file matched when this run started; a long conversion + // leaves room for that to stop being true. + ExpectedSourceSha256 = f.Sha256, }) ]; diff --git a/sources/EncodingChecker/ScanEngine.cs b/sources/EncodingChecker/ScanEngine.cs index 8f1468b..9fb188e 100644 --- a/sources/EncodingChecker/ScanEngine.cs +++ b/sources/EncodingChecker/ScanEngine.cs @@ -504,6 +504,10 @@ private static void ApplyConversion( RecordConversion = backup ? record => WriteConversionMetadata(path, record) : null, + + // Non-null only under -Apply, where an earlier run committed to these exact + // bytes. An ordinary conversion has nothing to compare against. + ExpectedSourceSha256 = entry.ExpectedSourceSha256, }; // Without the token, Parallel.ForEach could only observe cancellation between