diff --git a/CHANGELOG.md b/CHANGELOG.md index 29a2c81..57ec9ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,20 +13,33 @@ - Drop the advertised "Enter to scan, Del to delete selected" keyboard shortcuts from the feature list: the default modern WPF shell is click-only by design, matching the project's no-keyboard-shortcuts convention. ### Modern UI +- Colour-code the review list: each result's Status is shown as a coloured word (eligible = red, kept = muted, deleted = green, protected = blue, failed = amber), so the destructive list now reflects the legend instead of leaving every row the same colour. The colour reinforces the status word and is never the only signal. +- Give the path box, deletion-mode dropdown, filter lists, and result list a visible keyboard-focus ring (they previously showed only WPF's near-invisible dotted default on the dark surface) — WCAG 2.4.7. +- Report a dry run honestly: it now says "… would be removed. Nothing was changed." instead of claiming directories and files were "changed", and the completion line matches the chosen mode (deleted / recycled / moved). Distinguish a user **Canceled** run from one **Stopped after an error**. +- Use a legible red for the "eligible" status text, give result rows a comfortable minimum height, raise the idle status indicator to green / working to amber, and expose the plain state word ("Ready"/"Working") to screen readers instead of the decorative bullet. +- Add a positive "no empty directories found" state for a completed scan with nothing to remove, and normalize one-off button colours and corner radii onto the shared palette. - Add **Restore deletion** to the modern WPF shell's Extras menu: pick any kept undo manifest (newest first, with timestamp, mode, and item count) and restore it on a background thread with live status. Recovery no longer requires `-classic`. - Add **Import saved dry-run results...** to the modern WPF shell's Extras menu: load a saved `.json`/`.ndjson`/`.csv`/`.txt` dry-run and review the records in the results list (review/export only; re-scan to delete, and the engine re-checks every directory before acting). Review of saved runs no longer requires `-classic`. ### CLI +- Validate `-saveprofile` options before writing: an unknown `-mode`, or `-mode move` without an absolute `-moveto`, is now rejected with exit code 1 instead of silently saving a profile that fails every later `-profile` run. Profile names are trimmed and rejected if they exceed 128 characters or contain control characters (which would corrupt the `-listprofiles` output). - Add saved profiles: `-saveprofile ` stores the current options (paths, mode, empty-files, age/depth, gitignore/MFT/hidden/system toggles) as a named profile; `-profile ` runs it (command-line options still override); `-listprofiles` lists them. A scheduled task can now reference `-profile nightly` instead of a long argument list. Profiles live in a dedicated `RED+.profiles.json`, separate from the XML config. - Headless runs now print a "Run complete" summary line with total empty directories, empty files, deleted, failed, and wall-clock duration. The `-json` `meta` record (now schema 3) carries the same totals plus `elapsedMs`. (The modern GUI already shows the current directory being scanned in its status strip.) ### Reliability - Derive batch-recycle outcomes (success/fail and the undo manifest) authoritatively from `Directory.Exists` rather than the shell sink's submission-order fallback. `IFileOperation` does not guarantee one `PostDeleteItem` per `DeleteItem` in order, so a coalesced/skipped callback could previously record an undo entry against the wrong path; the filesystem is now the ground truth. +- Apply the same ground-truth rule to the empty-files recycle batch: each file's success and undo entry now come from `!File.Exists` rather than the sink's positional result list, so a coalesced callback can no longer log or record the wrong file as deleted. +- Scope a per-directory `.gitignore`'s bare-name rule (e.g. `dist`) to that directory's own subtree, matching Git semantics. Previously such a rule was applied tree-wide, so directories of that name elsewhere were wrongly reported as ignored (and skipped) in the dry-run the user reviews before deleting. ### Security & data safety +- Never delete a directory that contains a user-protected descendant. Protecting a folder only guarded that exact path, so an empty-eligible **ancestor** would still be deleted recursively and take the protected child with it (in both the direct and recycle-batch delete paths). A directory is now treated as protected when it, or anything beneath it, is on the protected list, and the protected-folder list is matched case-insensitively to follow Windows path semantics. +- Harden the cross-volume Move-to-folder fallback: after copying the directory to the other volume, re-verify it is file-free and remove the source bottom-up by handle (which refuses a non-empty directory) instead of a blind recursive `Directory.Delete`, so content that appears after the copy can never be destroyed. +- Bound user-supplied filter regexes (`RegExName`/`RegExPath`) with a one-second match timeout, so a catastrophic-backtracking pattern (e.g. `(a+)+$`) can no longer hang the scan thread; a timed-out match is treated as no-match. - Clamp the C-style (`SpecialFormatters`) printf width and precision to a bounded ceiling so a crafted format such as `%9999999d` can no longer drive a multi-megabyte allocation (memory-amplification DoS for callers that opt into the format callback). - Guard `Translator.RegisterTranslationsByCulture` against a malformed caller-supplied search pattern: a bad `string.Format` pattern is now skipped instead of throwing an uncaught `FormatException`. - Stop truncating the live run log when rotation cannot move it (e.g. the log is held open). The existing log is now preserved and appended to, keeping it intact as a forensic/undo aid. +- Restore a default sub-object after loading a config whose child element was explicitly nil'd (e.g. a hand-edited ``), which would otherwise NullReference inside the dirty-state check — including in the load's `finally`, crashing a headless run with a non-deterministic exit code. +- Run the scanned paths through the same bidi/zero-width/control-character sanitizer before writing the `-eventlog` summary, so a crafted folder name cannot reorder or corrupt the Windows Event Viewer entry. - Make the MFT/USN turbo scanner fail closed on an incomplete enumeration. The volume walk now treats any termination other than the EOF sentinel as truncated, and rejects a result set in which a directory referenced as a parent is missing its own record (the signature of a dropped USN record). In either case the scan falls back to the standard recursive walker instead of risking a non-empty directory being reported empty because its children were dropped. ### Developer / CI diff --git a/RED.Tests/DeletionWorkerTests.cs b/RED.Tests/DeletionWorkerTests.cs new file mode 100644 index 0000000..1e01a5e --- /dev/null +++ b/RED.Tests/DeletionWorkerTests.cs @@ -0,0 +1,67 @@ +using System; +using System.Collections.Generic; +using Xunit; + +namespace RED +{ + // Locks in the protected-folder invariant: a directory must not be deleted when + // it (or any ancestor) is the parent of a user-protected folder. Regression guard + // for the bug where a protected child was destroyed by an empty-eligible ancestor's + // recursive delete because only the child's own path was checked. + public class DeletionWorkerTests + { + private static HashSet Set(params string[] paths) + { + return new HashSet(paths, StringComparer.OrdinalIgnoreCase); + } + + [Fact] + public void Protect_Self_IsProtected() + { + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\b\keep", Set(@"C:\a\b\keep"))); + } + + [Fact] + public void Protect_Ancestor_OfProtected_IsProtected() + { + var p = Set(@"C:\a\b\keep"); + // Every ancestor of a protected folder must itself be treated as protected, + // or a recursive delete of the ancestor would destroy the protected child. + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\b", p)); + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a", p)); + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\b\", p)); // trailing separator + } + + [Fact] + public void Protect_IsCaseInsensitive() + { + var p = Set(@"C:\Data\Keep"); + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"c:\data", p)); + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\DATA\KEEP", p)); + } + + [Fact] + public void Sibling_And_Unrelated_AreNotProtected() + { + var p = Set(@"C:\a\b\keep"); + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\b\other", p)); + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\x", p)); + } + + [Fact] + public void PrefixSibling_IsNotMistakenForAncestor() + { + // "C:\a\bc" shares a string prefix with "C:\a\b\keep" but is NOT an ancestor; + // the path-separator boundary must prevent a false protection match. + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\bc", Set(@"C:\a\b\keep"))); + } + + [Fact] + public void EmptyOrNull_AreNotProtected() + { + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a", new HashSet())); + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(null, Set(@"C:\a"))); + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected("", Set(@"C:\a"))); + } + } +} diff --git a/RED.Tests/GitIgnoreTests.cs b/RED.Tests/GitIgnoreTests.cs index 72393ef..1ca748d 100644 --- a/RED.Tests/GitIgnoreTests.cs +++ b/RED.Tests/GitIgnoreTests.cs @@ -71,5 +71,23 @@ public void CommentsAndBlankLines_Ignored() Assert.True(p.IsIgnored("cache", "cache")); Assert.False(p.IsIgnored("a", "a")); } + + [Fact] + public void NamePattern_InSubdirGitignore_IsScopedToThatSubtree() + { + // A bare-name rule in a per-directory .gitignore must only affect that + // directory's subtree, not every directory of that name elsewhere (Git + // per-directory scope). Regression guard: previously it applied tree-wide. + string sub = Path.Combine(_root, "sub"); + Directory.CreateDirectory(sub); + File.WriteAllText(Path.Combine(sub, ".gitignore"), "dist\n"); + + var atSub = RED.GitIgnoreParser.LoadFromAncestors(_root).ExtendForDirectory(sub, _root); + + Assert.True(atSub.IsIgnored("dist", "sub/dist")); // in scope -> ignored + Assert.True(atSub.IsIgnored("dist", "sub/a/dist")); // deeper in scope -> ignored + Assert.False(atSub.IsIgnored("dist", "other/dist")); // different subtree -> NOT ignored + Assert.False(atSub.IsIgnored("dist", "dist")); // at root -> NOT ignored + } } } diff --git a/RED.Tests/RedMatchTests.cs b/RED.Tests/RedMatchTests.cs index e5e0d06..76adf1f 100644 --- a/RED.Tests/RedMatchTests.cs +++ b/RED.Tests/RedMatchTests.cs @@ -1,5 +1,7 @@ using System.Collections.Generic; +using System.Diagnostics; using System.IO; +using System.Text.RegularExpressions; using RED.Match; using Xunit; @@ -39,6 +41,21 @@ public void RegexNameRule_MatchesCaseInsensitive() Assert.False(list.IsOnList(new DirectoryInfo(@"C:\x\temp"))); } + [Fact] + public void RegexRule_PathologicalPattern_DoesNotHangAndYieldsNoMatch() + { + // A catastrophic-backtracking pattern against a long name must hit the + // bounded match timeout and resolve to "no match" instead of wedging the + // scan thread. Regression guard for the missing regex matchTimeout. + var list = BuildDirList("+|RN|/(a+)+$/"); + var dir = new DirectoryInfo(@"C:\x\" + new string('a', 44) + "!"); + var sw = Stopwatch.StartNew(); + bool hit = list.IsOnList(dir); + sw.Stop(); + Assert.False(hit); + Assert.True(sw.ElapsedMilliseconds < 5000, "regex match should time out fast, took " + sw.ElapsedMilliseconds + "ms"); + } + [Fact] public void WildcardCodelessRule_BecomesNameRegex() { diff --git a/RED/Config/Config.cs b/RED/Config/Config.cs index a94555b..722ad9f 100644 --- a/RED/Config/Config.cs +++ b/RED/Config/Config.cs @@ -57,6 +57,22 @@ internal void SetToDefaults() UI.SetToDefaults(); } + /// + /// Replaces any null sub-object with a fresh default. A hand-crafted config that + /// nils a child (e.g. <Options xsi:nil="true"/>) would otherwise + /// deserialize with a null member and NullReference inside the DataIsDirty + /// getter/setter — including in ConfigLoad's finally, where it could crash a + /// headless run with a non-deterministic exit code. + /// + internal void EnsureSubObjects() + { + if (Options == null) Options = new ConfigOptions(); + if (Filters == null) Filters = new ConfigFilters(); + if (UI == null) UI = new ConfigUI(); + if (Volatile == null) Volatile = new ConfigVolatile(); + if (Runtime == null) Runtime = new ConfigRuntime(); + } + public void PopulateRuntime(string configFilename, string executableName, string productName, string productVersion) { Runtime.CreatedBy = string.Format("{0} {1}", productName, productVersion); diff --git a/RED/Config/ConfigAssist.cs b/RED/Config/ConfigAssist.cs index 0a7e7d7..32fcefd 100644 --- a/RED/Config/ConfigAssist.cs +++ b/RED/Config/ConfigAssist.cs @@ -39,6 +39,10 @@ internal static void ConfigLoad(ref RedConfiguration config, string appName) { config = ConfigAssist.Load(filename); + // A crafted/corrupt file can deserialize with a nil child object; + // restore any missing sub-object before anything dereferences it. + config?.EnsureSubObjects(); + // Does the config file redirect to another location? if (!string.IsNullOrWhiteSpace(config.RedirectTo)) { diff --git a/RED/Program.cs b/RED/Program.cs index 14a281b..b015ee5 100644 --- a/RED/Program.cs +++ b/RED/Program.cs @@ -343,6 +343,25 @@ private static void Main() if (saveProfileName != null) { + // Validate the options now so the saved profile is guaranteed runnable, + // instead of silently saving a profile that fails every later -profile run. + if (modeOverride != null && !ModeAliases.ContainsKey(modeOverride)) + { + if (!quiet) Console.Error.WriteLine("Error: unknown -mode '" + modeOverride + "' (use recycle|direct|move|simulate)"); + Environment.ExitCode = 1; + return; + } + if (modeOverride != null && ModeAliases[modeOverride] == DeleteModes.MoveToFolder) + { + string expandedTarget = string.IsNullOrWhiteSpace(moveTarget) ? null : Environment.ExpandEnvironmentVariables(moveTarget); + if (string.IsNullOrWhiteSpace(expandedTarget) || !System.IO.Path.IsPathRooted(expandedTarget)) + { + if (!quiet) Console.Error.WriteLine("Error: -mode move requires an absolute -moveto "); + Environment.ExitCode = 1; + return; + } + } + var prof = new RedProfile { Name = saveProfileName, diff --git a/RED/RedLib/Core.cs b/RED/RedLib/Core.cs index e3bb6dd..10c74a1 100644 --- a/RED/RedLib/Core.cs +++ b/RED/RedLib/Core.cs @@ -48,8 +48,9 @@ public void SearchingForEmptyDirectories() { CurrentProcessStep = WorkflowSteps.StartSearchingForEmptyDirs; - // Rest folder list - RunData.ProtectedFolderList = new Dictionary(); + // Reset protected-folder list. Case-insensitive to match Windows path + // semantics (a protect entry must match the scan result regardless of casing). + RunData.ProtectedFolderList = new Dictionary(StringComparer.OrdinalIgnoreCase); // Start async empty directory search worker SearchEmptyFoldersWorker = new FindEmptyDirectoryWorker(); diff --git a/RED/RedLib/DeletionWorker.cs b/RED/RedLib/DeletionWorker.cs index d9319f5..7fe68cb 100644 --- a/RED/RedLib/DeletionWorker.cs +++ b/RED/RedLib/DeletionWorker.cs @@ -34,6 +34,29 @@ public class DeletionWorker : BackgroundWorker private readonly System.Collections.Generic.HashSet deletedParents = new System.Collections.Generic.HashSet(StringComparer.OrdinalIgnoreCase); + /// + /// True when is itself user-protected OR is an + /// ancestor of any protected path. A directory that merely contains a protected + /// descendant must never be deleted: a recursive delete of the ancestor destroys + /// the protected child, and the per-item protection check on the child's own + /// path is then bypassed (the child is reported as already-deleted-by-parent). + /// Comparison is case-insensitive to match Windows path semantics and the + /// OrdinalIgnoreCase ProtectedFolderList. + /// + internal static bool IsProtectedOrAncestorOfProtected(string fullPath, System.Collections.Generic.ICollection protectedPaths) + { + if (string.IsNullOrEmpty(fullPath) || protectedPaths == null || protectedPaths.Count == 0) + return false; + string prefix = fullPath.TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; + foreach (string p in protectedPaths) + { + if (string.IsNullOrEmpty(p)) continue; + if (string.Equals(p, fullPath, StringComparison.OrdinalIgnoreCase)) return true; + if (p.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) return true; + } + return false; + } + public DeletionWorker() { WorkerReportsProgress = true; @@ -114,8 +137,9 @@ protected override void OnDoWork(DoWorkEventArgs e) MovedTo = null }); } - // Do not delete one time protected folders - else if (!this.Data.ProtectedFolderList.ContainsKey(scanResult.FullPath)) + // Never delete a protected folder, nor any directory that contains a + // protected descendant (a recursive delete would take the child with it). + else if (!IsProtectedOrAncestorOfProtected(scanResult.FullPath, this.Data.ProtectedFolderList.Keys)) { try { @@ -155,6 +179,7 @@ protected override void OnDoWork(DoWorkEventArgs e) else { status = DirectoryDeletionStatusTypes.Protected; + this.ProtectedCount++; } this.ReportProgress(1, new DeleteProcessUpdateEventArgs(this.ListPos, scanResult, status, this.Data.ScanResults.Count)); @@ -218,8 +243,9 @@ private void BatchRecycleRun(DoWorkEventArgs e) Match.RedScanResultItem scanResult = this.Data.ScanResults[pos]; - if (this.Data.ProtectedFolderList.ContainsKey(scanResult.FullPath)) + if (IsProtectedOrAncestorOfProtected(scanResult.FullPath, this.Data.ProtectedFolderList.Keys)) { + this.ProtectedCount++; this.ReportProgress(1, new DeleteProcessUpdateEventArgs(pos, scanResult, DirectoryDeletionStatusTypes.Protected, total)); continue; } @@ -409,20 +435,26 @@ private void DeleteEmptyFilesBatch(List files) bool showErrors = this.Data.DeleteMode == DeleteModes.RecycleBinShowErrors && !NotBob.Config.ConfigAssist.SilentMode; bool showConfirm = this.Data.DeleteMode == DeleteModes.RecycleBinWithQuestion && !NotBob.Config.ConfigAssist.SilentMode; - List results = RecycleBinOperation.RecycleBatch( + RecycleBinOperation.RecycleBatch( paths, showConfirm, showErrors, () => CancellationPending, null); - for (int i = 0; i < results.Count; i++) + // Derive each file's outcome from the filesystem, not the sink's positional + // result list: IFileOperation does not guarantee one PostDeleteItem per + // DeleteItem in submission order, so trusting results[i] can attribute a + // success/failure (and the undo entry) to the wrong file. A recycled file is + // gone from its origin, so !File.Exists is the ground truth (matches the + // directory batch path). + foreach (string filePath in paths) { - RecycleBinOperation.ItemResult r = results[i]; - if (r.Succeeded) + bool deleted = !File.Exists(filePath); + if (deleted) { - this.Data.AddLogMessage(TXT.Translate("Successfully deleted empty file: {0}", RedAssist.DQuote(r.Path))); + this.Data.AddLogMessage(TXT.Translate("Successfully deleted empty file: {0}", RedAssist.DQuote(filePath))); this.DeletedFileCount++; undoEntries.Add(new UndoManager.ManifestEntry { - Path = r.Path, + Path = filePath, Mode = this.Data.DeleteMode.ToString(), MovedTo = null, IsFile = true @@ -430,7 +462,7 @@ private void DeleteEmptyFilesBatch(List files) } else { - this.Data.AddLogMessage(TXT.Translate("Failed to delete empty file: {0} - HRESULT 0x{1:X8}", RedAssist.DQuote(r.Path), r.HResult)); + this.Data.AddLogMessage(TXT.Translate("Failed to delete empty file: {0}", RedAssist.DQuote(filePath))); this.FailedFileCount++; } } diff --git a/RED/RedLib/EventLogWriter.cs b/RED/RedLib/EventLogWriter.cs index 7c39536..21a8be0 100644 --- a/RED/RedLib/EventLogWriter.cs +++ b/RED/RedLib/EventLogWriter.cs @@ -18,6 +18,10 @@ internal static void WriteRunSummary(string scanPaths, string deleteMode, EventLog.CreateEventSource(SourceName, LogName); } + // Folder names reach the Event Viewer here; neutralize bidi/zero-width/ + // control characters as the console and log lines already do. + scanPaths = RED.Helper.RedAssist.SanitizeDisplay(scanPaths); + string message = string.Format( "RED++ scan completed.\r\n" + "Paths: {0}\r\n" + diff --git a/RED/RedLib/GitIgnoreParser.cs b/RED/RedLib/GitIgnoreParser.cs index 1ea1ecf..4405384 100644 --- a/RED/RedLib/GitIgnoreParser.cs +++ b/RED/RedLib/GitIgnoreParser.cs @@ -173,6 +173,15 @@ public bool IsIgnored(string name, string relativePath) } else { + // A per-directory .gitignore's bare-name rule is scoped to that + // directory's own subtree, exactly like a path pattern. Without this + // guard a deep ".gitignore" line such as `dist` would wrongly ignore + // every `dist` folder tree-wide (siblings and ancestors included). + if (!string.IsNullOrEmpty(rule.ScopeDir) + && !pathToCheck.StartsWith(rule.ScopeDir + "/", StringComparison.OrdinalIgnoreCase)) + { + continue; + } text = name; } diff --git a/RED/RedLib/ProfileStore.cs b/RED/RedLib/ProfileStore.cs index 5e1e2b3..3c1e4f4 100644 --- a/RED/RedLib/ProfileStore.cs +++ b/RED/RedLib/ProfileStore.cs @@ -78,6 +78,22 @@ public static bool Save(RedProfile profile, out string error) error = "Profile name is required."; return false; } + profile.Name = profile.Name.Trim(); + // A control character (tab/newline) would corrupt the tab-delimited + // -listprofiles output; an unbounded name bloats the store. + if (profile.Name.Length > 128) + { + error = "Profile name must be 128 characters or fewer."; + return false; + } + foreach (char c in profile.Name) + { + if (char.IsControl(c)) + { + error = "Profile name must not contain control characters."; + return false; + } + } try { List all = LoadAll(); diff --git a/RED/RedLib/RuntimeData.cs b/RED/RedLib/RuntimeData.cs index 3cba2fa..e06e89f 100644 --- a/RED/RedLib/RuntimeData.cs +++ b/RED/RedLib/RuntimeData.cs @@ -15,7 +15,7 @@ public class RuntimeData : IDisposable public RuntimeData() { this.LogMessages = new StringBuilder(); - this.ProtectedFolderList = new Dictionary(); + this.ProtectedFolderList = new Dictionary(StringComparer.OrdinalIgnoreCase); this.ScanResults = new RedScanResultItemList(); try @@ -108,7 +108,7 @@ private static bool RotateLogIfNeeded(string logPath, out string rotatedPath) public int InfiniteLoopDetectionCount { get; set; } public StringBuilder LogMessages = null; - public Dictionary ProtectedFolderList = new Dictionary(); + public Dictionary ProtectedFolderList = new Dictionary(StringComparer.OrdinalIgnoreCase); public bool HideIgnoredDirectories { get; set; } public bool RespectGitIgnore { get; set; } diff --git a/RED/RedLib/SystemFunctions.cs b/RED/RedLib/SystemFunctions.cs index 1e8c130..f57ad0b 100644 --- a/RED/RedLib/SystemFunctions.cs +++ b/RED/RedLib/SystemFunctions.cs @@ -509,9 +509,14 @@ public static void SecureDeleteDirectory(string path, DeleteModes deleteMode, ou { VerifySubtreeHasNoFiles(path); } - // Directory.Move cannot cross volumes — replicate, then remove the source + // Directory.Move cannot cross volumes — replicate, then remove the + // source. Re-verify file-free one last time and delete bottom-up by + // handle (which refuses a non-empty directory) instead of a blind + // recursive Directory.Delete that would destroy anything that appeared + // after the copy. CopyDirectoryRecursive(path, destPath); - Directory.Delete(path, true); + VerifySubtreeHasNoFiles(path); + DeleteEmptySubtreeByHandle(path); } movedToDestination = destPath; return; diff --git a/RED/RedLib/TreeManager.cs b/RED/RedLib/TreeManager.cs index c51203d..6f27896 100644 --- a/RED/RedLib/TreeManager.cs +++ b/RED/RedLib/TreeManager.cs @@ -555,10 +555,10 @@ internal void UnprotectSelected() private void unprotectNode(TreeNode node) { - if (node != null) + // A node whose Tag is missing or not a DirectoryInfo (e.g. a placeholder + // child) must be skipped rather than throwing while walking the tree. + if (node?.Tag is DirectoryInfo directory) { - DirectoryInfo directory = ((DirectoryInfo)node.Tag); - if (!this.nodePropsBackup.ContainsKey(directory.FullName)) { // TODO: What to do when this info is missing, show error? @@ -589,10 +589,8 @@ private void unprotectNode(TreeNode node) private void ProtectNode(TreeNode node) { - if (node != null) + if (node?.Tag is DirectoryInfo directory) { - DirectoryInfo directory = (DirectoryInfo)node.Tag; - if (nodePropsBackup.ContainsKey(directory.FullName)) { return; diff --git a/RED/RedMatch/RedMatchItem.cs b/RED/RedMatch/RedMatchItem.cs index 41d02ef..9e96bc2 100644 --- a/RED/RedMatch/RedMatchItem.cs +++ b/RED/RedMatch/RedMatchItem.cs @@ -32,8 +32,10 @@ public void Populate(RedMatchMethodType matchMethod, string matchText, bool enab if (matchMethod == RedMatchMethodType.RegExName || matchMethod == RedMatchMethodType.RegExPath) { // All other match methods compare case-insensitively (names are - // lowercased before matching) — regex rules must behave the same - RegEx = new Regex(ExpandRegEx(matchText), RegexOptions.IgnoreCase); + // lowercased before matching) — regex rules must behave the same. + // Bound backtracking so a pathological user pattern (e.g. "(a+)+$") + // cannot wedge the scan thread; a timeout is treated as no-match. + RegEx = new Regex(ExpandRegEx(matchText), RegexOptions.IgnoreCase, System.TimeSpan.FromSeconds(1)); } MatchText = matchText.Trim(); MatchTextToCompare = MatchText.ToLowerInvariant(); @@ -66,6 +68,17 @@ private string ExpandRegEx(string text) return respx; } + /// + /// Bounded regex match: a user pattern that exhausts its match timeout is + /// treated as no-match rather than throwing out of the hot scan loop. + /// + internal static bool SafeIsMatch(Regex regex, string input) + { + if (regex == null) return false; + try { return regex.IsMatch(input); } + catch (RegexMatchTimeoutException) { return false; } + } + internal static string FormatToString(RedMatchItem item) { return FormatToString(item.Enabled, item.MatchMethod, item.MatchText); diff --git a/RED/RedMatch/RedMatchItemList.cs b/RED/RedMatch/RedMatchItemList.cs index 93cecd9..82d0c56 100644 --- a/RED/RedMatch/RedMatchItemList.cs +++ b/RED/RedMatch/RedMatchItemList.cs @@ -195,11 +195,11 @@ private bool IsOnList(string nameToCheck, string pathToCheck, long filesize, boo hit = textToCheck.StartsWith(matchItem.MatchTextToCompare); break; case RedMatchMethodType.RegExName: - hit = matchItem.RegEx.IsMatch(nameToCheck); + hit = RedMatchItem.SafeIsMatch(matchItem.RegEx, nameToCheck); break; case RedMatchMethodType.RegExPath: if (FilterType == RedMatchFilterType.Directory) - hit = matchItem.RegEx.IsMatch(pathToCheck); + hit = RedMatchItem.SafeIsMatch(matchItem.RegEx, pathToCheck); break; case RedMatchMethodType.NameExactWithPath: if (FilterType == RedMatchFilterType.Directory) diff --git a/RED/UI/MainWindow.cs b/RED/UI/MainWindow.cs index bf961c2..0c41ce8 100644 --- a/RED/UI/MainWindow.cs +++ b/RED/UI/MainWindow.cs @@ -1787,7 +1787,7 @@ private void mnuItemImportDryRunResults_Click(object sender, EventArgs e) } } - RunData.ProtectedFolderList = new Dictionary(); + RunData.ProtectedFolderList = new Dictionary(StringComparer.OrdinalIgnoreCase); RunData.StartFolder = imported.Roots.Count == 1 ? imported.Roots[0].RootDirectory : null; var directoryRoots = new List(); diff --git a/RED/UI/Wpf/ModernMainWindow.cs b/RED/UI/Wpf/ModernMainWindow.cs index 8cf82c6..d7ba75a 100644 --- a/RED/UI/Wpf/ModernMainWindow.cs +++ b/RED/UI/Wpf/ModernMainWindow.cs @@ -45,6 +45,11 @@ internal sealed class ModernMainWindow : Window private Grid contentHost; private Border resultSurface; private Grid emptyState; + private System.Windows.Shapes.Path emptyIcon; + private TextBlock emptyTitle; + private TextBlock emptySubtitle; + private StackPanel emptyTrust; + private bool hasScanned; private ListView resultsList; private WpfTextBox pathBox; private WpfButton scanButton; @@ -91,8 +96,17 @@ internal sealed class ModernMainWindow : Window private static readonly Brush Blue = BrushFrom("#2f6df2"); private static readonly Brush BlueLight = BrushFrom("#7aa8ff"); private static readonly Brush Red = BrushFrom("#dc3548"); + // A brighter red than the legend swatch, for status text that must stay legible + // on the dark result surface (the #dc3548 swatch dips below AA at body size). + private static readonly Brush RedText = BrushFrom("#ff7a86"); private static readonly Brush Green = BrushFrom("#67d16f"); + private static readonly Brush Amber = BrushFrom("#ffca55"); private static readonly Brush Pink = BrushFrom("#f17aa5"); + + // Keyboard-focus ring for the non-button controls (text box, combo, lists), + // which otherwise show only WPF's near-invisible dotted default on this dark + // surface. Matches the BlueLight ring the buttons already use (WCAG 2.4.7). + private static readonly Style FocusVisual = CreateFocusVisual(); private static readonly Geometry IconSearch = Glyph("M17,8 A9,9 0 1 1 17,26 A9,9 0 1 1 17,8 M24,24 L32,32"); private static readonly Geometry IconSettings = Glyph("M18,5 L20.6,9.7 L26,10.8 L22.4,15 L24.1,20.3 L18.8,19 L15.2,23.2 L12.6,18.5 L7.2,17.4 L10.8,13.2 L9.1,7.9 L14.4,9.2 Z M18,13 A5,5 0 1 1 17.9,13"); private static readonly Geometry IconFilter = Glyph("M7,7 L31,7 L22,18 L22,29 L16,32 L16,18 Z"); @@ -219,7 +233,7 @@ private static ControlTemplate CreateButtonTemplate() var border = new FrameworkElementFactory(typeof(System.Windows.Controls.Border)); border.Name = "ButtonChrome"; border.SetValue(System.Windows.Controls.Border.SnapsToDevicePixelsProperty, true); - border.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(2)); + border.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(4)); border.SetBinding(System.Windows.Controls.Border.BackgroundProperty, new Binding("Background") { RelativeSource = RelativeSource.TemplatedParent }); border.SetBinding(System.Windows.Controls.Border.BorderBrushProperty, new Binding("BorderBrush") { RelativeSource = RelativeSource.TemplatedParent }); border.SetBinding(System.Windows.Controls.Border.BorderThicknessProperty, new Binding("BorderThickness") { RelativeSource = RelativeSource.TemplatedParent }); @@ -238,7 +252,7 @@ private static ControlTemplate CreateButtonTemplate() // hover/press feedback regardless of its base color. var overlay = new FrameworkElementFactory(typeof(System.Windows.Controls.Border)); overlay.Name = "HoverOverlay"; - overlay.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(2)); + overlay.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(4)); overlay.SetValue(System.Windows.Controls.Border.BackgroundProperty, Brushes.Transparent); overlay.SetValue(UIElement.IsHitTestVisibleProperty, false); layers.AppendChild(overlay); @@ -247,7 +261,7 @@ private static ControlTemplate CreateButtonTemplate() // button takes keyboard focus, so it is visible on any background. var focusRing = new FrameworkElementFactory(typeof(System.Windows.Controls.Border)); focusRing.Name = "FocusRing"; - focusRing.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(2)); + focusRing.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(4)); focusRing.SetValue(System.Windows.Controls.Border.BorderBrushProperty, Brushes.Transparent); focusRing.SetValue(System.Windows.Controls.Border.BorderThicknessProperty, new Thickness(2)); focusRing.SetValue(UIElement.IsHitTestVisibleProperty, false); @@ -272,6 +286,40 @@ private static ControlTemplate CreateButtonTemplate() return template; } + // A focus-visual adorner (drawn over the focused control on keyboard focus): + // a 2px BlueLight rounded border so text box / combo / list focus is visible. + private static Style CreateFocusVisual() + { + var template = new ControlTemplate(); + var border = new FrameworkElementFactory(typeof(System.Windows.Controls.Border)); + border.SetValue(System.Windows.Controls.Border.BorderBrushProperty, BlueLight); + border.SetValue(System.Windows.Controls.Border.BorderThicknessProperty, new Thickness(2)); + border.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(4)); + border.SetValue(System.Windows.Controls.Border.SnapsToDevicePixelsProperty, true); + border.SetValue(FrameworkElement.MarginProperty, new Thickness(-2)); + template.VisualTree = border; + var style = new Style(); + style.Setters.Add(new Setter(Control.TemplateProperty, template)); + return style; + } + + // Maps a result's status word to its legend color so the review list is + // colour-coded (eligible = red, kept = muted, deleted = green, protected = + // blue, failed = amber). The colour is paired with the status word, never the + // only signal, so meaning never depends on colour alone. + private static Brush StatusToBrush(string statusLabel) + { + if (string.IsNullOrEmpty(statusLabel)) return Muted; + switch (statusLabel) + { + case "Eligible": return RedText; + case "Deleted": return Green; + case "Protected": return BlueLight; + case "Warning": return Amber; + default: return Muted; // Kept / Ignored / NeverEmpty / etc. + } + } + private void ApplyInitialWindowBounds() { double availableWidth = Math.Max(640d, SystemParameters.WorkArea.Width - 40d); @@ -575,6 +623,7 @@ private UIElement BuildSearchTab() VerticalAlignment = VerticalAlignment.Top, HorizontalAlignment = HorizontalAlignment.Stretch }; + pathBox.FocusVisualStyle = FocusVisual; SetAutomation(pathBox, "Folder to scan", "Enter or paste the root folder RED++ should scan."); pathRow.Children.Add(pathBox); @@ -689,16 +738,18 @@ private Grid BuildEmptyState() VerticalAlignment = VerticalAlignment.Center }; grid.Children.Add(center); - center.Children.Add(IconPath(IconFolder, Muted2, 46, 2.5, new DoubleCollection(new[] { 5d, 4d }), new Thickness(0, 0, 0, 8))); - center.Children.Add(new TextBlock + emptyIcon = IconPath(IconFolder, Muted2, 46, 2.5, new DoubleCollection(new[] { 5d, 4d }), new Thickness(0, 0, 0, 8)); + center.Children.Add(emptyIcon); + emptyTitle = new TextBlock { Text = "Choose a folder to scan.", Foreground = Text, FontSize = 18, TextAlignment = TextAlignment.Center, Margin = new Thickness(0, 0, 0, 8) - }); - center.Children.Add(new TextBlock + }; + center.Children.Add(emptyTitle); + emptySubtitle = new TextBlock { Text = "Review results before anything changes.", Foreground = Muted, @@ -706,13 +757,42 @@ private Grid BuildEmptyState() LineHeight = 18, TextAlignment = TextAlignment.Center, Margin = new Thickness(0, 0, 0, 10) - }); - center.Children.Add(TrustRow(IconSearch, BlueLight, "Pick a root folder, then scan.")); - center.Children.Add(TrustRow(IconShield, Green, "Review eligible results.")); - center.Children.Add(TrustRow(IconTrash, Pink, "Confirm before changes.")); + }; + center.Children.Add(emptySubtitle); + emptyTrust = new StackPanel(); + emptyTrust.Children.Add(TrustRow(IconSearch, BlueLight, "Pick a root folder, then scan.")); + emptyTrust.Children.Add(TrustRow(IconShield, Green, "Review eligible results.")); + emptyTrust.Children.Add(TrustRow(IconTrash, Pink, "Confirm before changes.")); + center.Children.Add(emptyTrust); return grid; } + // Swaps the centre panel between the pre-scan prompt and a positive "all clean" + // state, so a completed scan with no results no longer tells the user to pick a + // folder they just scanned. + private void SetEmptyStateMode(bool clean) + { + if (emptyIcon == null) return; + if (clean) + { + emptyIcon.Data = IconCheck; + emptyIcon.Stroke = Green; + emptyIcon.StrokeDashArray = null; + emptyTitle.Text = "No empty directories found."; + emptySubtitle.Text = "RED++ scanned this folder and found nothing to remove."; + emptyTrust.Visibility = Visibility.Collapsed; + } + else + { + emptyIcon.Data = IconFolder; + emptyIcon.Stroke = Muted2; + emptyIcon.StrokeDashArray = new DoubleCollection(new[] { 5d, 4d }); + emptyTitle.Text = "Choose a folder to scan."; + emptySubtitle.Text = "Review results before anything changes."; + emptyTrust.Visibility = Visibility.Visible; + } + } + private UIElement TrustRow(Geometry icon, Brush brush, string text) { var row = new StackPanel @@ -739,9 +819,30 @@ private ListView BuildResultsList() ScrollViewer.SetHorizontalScrollBarVisibility(list, ScrollBarVisibility.Auto); ScrollViewer.SetVerticalScrollBarVisibility(list, ScrollBarVisibility.Auto); SetAutomation(list, "Review results", "Empty directories and empty files found during the last scan."); + list.FocusVisualStyle = FocusVisual; + + // Comfortable, tappable rows for a destructive-review list. + var itemStyle = new Style(typeof(System.Windows.Controls.ListViewItem)); + itemStyle.Setters.Add(new Setter(Control.MinHeightProperty, 28d)); + itemStyle.Setters.Add(new Setter(Control.PaddingProperty, new Thickness(4, 2, 4, 2))); + itemStyle.Setters.Add(new Setter(Control.VerticalContentAlignmentProperty, VerticalAlignment.Center)); + list.ItemContainerStyle = itemStyle; + var gridView = new GridView(); list.View = gridView; - gridView.Columns.Add(new GridViewColumn { Header = "Status", Width = 96, DisplayMemberBinding = new Binding("StatusLabel") }); + + // Status as a coloured word (eligible = red, kept = muted, deleted = + // green, …) so the review list reflects the legend instead of leaving the + // computed status colour unused. The word carries the meaning; the colour + // reinforces it (never colour alone). + var statusTemplate = new DataTemplate(); + var statusText = new FrameworkElementFactory(typeof(TextBlock)); + statusText.SetBinding(TextBlock.TextProperty, new Binding("StatusLabel")); + statusText.SetBinding(TextBlock.ForegroundProperty, new Binding("StatusBrush")); + statusText.SetValue(TextBlock.FontWeightProperty, FontWeights.SemiBold); + statusTemplate.VisualTree = statusText; + + gridView.Columns.Add(new GridViewColumn { Header = "Status", Width = 96, CellTemplate = statusTemplate }); gridView.Columns.Add(new GridViewColumn { Header = "Name", Width = 170, DisplayMemberBinding = new Binding("Name") }); gridView.Columns.Add(new GridViewColumn { Header = "Reason", Width = 220, DisplayMemberBinding = new Binding("Reason") }); gridView.Columns.Add(new GridViewColumn { Header = "Path", Width = 460, DisplayMemberBinding = new Binding("FullPath") }); @@ -790,6 +891,7 @@ private UIElement BuildSettingsTab() BorderBrush = BorderStrong, BorderThickness = new Thickness(1) }; + deleteMode.FocusVisualStyle = FocusVisual; SetAutomation(deleteMode, "Deletion mode", "Choose whether RED++ simulates, recycles, deletes directly, or moves eligible results."); foreach (DeleteModes mode in DeleteModeItem.GetList()) { @@ -859,6 +961,7 @@ private void AddFilterList(Grid grid, int column, string title, List rul MinHeight = 420, FontSize = 15 }; + list.FocusVisualStyle = FocusVisual; panel.Children.Add(list); } @@ -905,7 +1008,7 @@ private void BuildCommandBar() deleteButton.Click += (s, e) => StartDelete(); primaryActions.Children.Add(deleteButton); - cancelButton = ActionButton("Cancel", BrushFrom("#27334a"), IconCancel, 140); + cancelButton = ActionButton("Cancel", Panel2, IconCancel, 140); SetAutomation(cancelButton, "Cancel current operation", "Cancel the scan or deletion currently in progress."); cancelButton.Margin = new Thickness(12, 0, 0, 0); cancelButton.Click += (s, e) => core?.CancelCurrentProcess(); @@ -935,7 +1038,7 @@ private WpfButton ActionButton(string text, Brush background, Geometry icon, dou Width = width, Height = 52, Background = background, - BorderBrush = BrushFrom("#6f86ad"), + BorderBrush = BorderStrong, BorderThickness = new Thickness(1), Cursor = Cursors.Hand }; @@ -1189,9 +1292,10 @@ private void AttachCoreEvents(REDCore activeCore) AddEmptyFileResults(); int total = e.EmptyFolderCount + e.EmptyFileCount; detailStatusText.Text = total == 0 - ? string.Format("Checked {0} directories. Nothing to delete yet.", e.FolderCount) + ? string.Format("Checked {0} {1} — no empty directories found.", e.FolderCount, e.FolderCount == 1 ? "directory" : "directories") : string.Format("{0} empty directories and {1} empty files eligible.", e.EmptyFolderCount, e.EmptyFileCount); itemCountText.Text = total + " items"; + hasScanned = true; UpdateUiState(false); deleteButton.IsEnabled = total > 0; progressBar.IsIndeterminate = false; @@ -1207,12 +1311,12 @@ private void AttachCoreEvents(REDCore activeCore) activeCore.OnCancelled += (s, e) => Dispatcher.Invoke(() => { UpdateUiState(false); - detailStatusText.Text = "Operation canceled."; + detailStatusText.Text = "Canceled."; }); activeCore.OnAborted += (s, e) => Dispatcher.Invoke(() => { UpdateUiState(false); - detailStatusText.Text = "Operation stopped."; + detailStatusText.Text = "Stopped after an error."; }); activeCore.OnDeleteProcessChanged += (s, e) => Dispatcher.Invoke(() => { @@ -1220,7 +1324,9 @@ private void AttachCoreEvents(REDCore activeCore) AddOrUpdateResult(e.ScanResult); if (rowsByPath.ContainsKey(e.ScanResult.FullPath)) { - rowsByPath[e.ScanResult.FullPath].StatusLabel = e.Status.ToString(); + ResultRow updatedRow = rowsByPath[e.ScanResult.FullPath]; + updatedRow.StatusLabel = e.Status.ToString(); + updatedRow.StatusBrush = StatusToBrush(updatedRow.StatusLabel); } progressBar.IsIndeterminate = false; progressBar.Maximum = Math.Max(1, e.FolderCount); @@ -1247,7 +1353,7 @@ private void AttachCoreEvents(REDCore activeCore) { UpdateUiState(false); deleteButton.IsEnabled = false; - detailStatusText.Text = string.Format("Deletion complete: {0} directories and {1} files changed.", e.DeletedFolderCount, e.DeletedFileCount); + detailStatusText.Text = BuildCompletionMessage(e.DeletedFolderCount, e.DeletedFileCount); progressBar.IsIndeterminate = false; progressBar.Value = 0; progressText.Text = "0%"; @@ -1272,7 +1378,7 @@ private void AddOrUpdateResult(RedScanResultItem item) row.FullPath = item.FullPath; row.Reason = item.StatusReason; row.StatusLabel = item.SearchStatus == DirectorySearchStatusTypes.Empty ? "Eligible" : "Kept"; - row.StatusBrush = item.SearchStatus == DirectorySearchStatusTypes.Empty ? Red : Muted; + row.StatusBrush = StatusToBrush(row.StatusLabel); itemCountText.Text = results.Count + " items"; RefreshResultsVisibility(); } @@ -1387,13 +1493,35 @@ private string BuildDeleteConfirmationMessage(int deleteCount, int fileDeleteCou } } + // Completion copy that matches the chosen mode. Critically, a dry run reports + // "would be removed / nothing was changed" instead of claiming files changed. + private string BuildCompletionMessage(int dirs, int files) + { + string d = dirs == 1 ? "directory" : "directories"; + string f = files == 1 ? "file" : "files"; + DeleteModes mode = runData != null ? runData.DeleteMode : DeleteModes.RecycleBin; + switch (mode) + { + case DeleteModes.Simulate: + return string.Format("Dry run complete — {0} {1} and {2} {3} would be removed. Nothing was changed.", dirs, d, files, f); + case DeleteModes.MoveToFolder: + return string.Format("Moved {0} {1} and {2} {3}.", dirs, d, files, f); + case DeleteModes.Direct: + return string.Format("Deleted {0} {1} and {2} {3}.", dirs, d, files, f); + default: + return string.Format("Recycled {0} {1} and {2} {3}.", dirs, d, files, f); + } + } + private void UpdateUiState(bool busy) { scanButton.IsEnabled = !busy; cancelButton.IsEnabled = busy; extrasButton.IsEnabled = !busy; - readyText.Text = busy ? "● Busy" : "● Ready"; - readyText.Foreground = busy ? BrushFrom("#e8c35b") : Text; + readyText.Text = busy ? "● Working…" : "● Ready"; + readyText.Foreground = busy ? Amber : Green; + // Screen readers should hear the state word, not the decorative bullet. + System.Windows.Automation.AutomationProperties.SetName(readyText, busy ? "Working" : "Ready"); if (!busy && deleteButton != null) { // Enable delete only when something is actually eligible — rows that @@ -1424,6 +1552,7 @@ private void RefreshResultsVisibility() bool empty = results.Count == 0; emptyState.Visibility = empty ? Visibility.Visible : Visibility.Collapsed; resultsList.Visibility = empty ? Visibility.Collapsed : Visibility.Visible; + if (empty) SetEmptyStateMode(hasScanned); } private void ApplyConfigToUi()