diff --git a/design/mockups/per-target-launch-options/APPROVED b/design/mockups/per-target-launch-options/APPROVED index 71bad4f..31fa3e0 100644 --- a/design/mockups/per-target-launch-options/APPROVED +++ b/design/mockups/per-target-launch-options/APPROVED @@ -1 +1,2 @@ 2026-07-08 feat/launch-commands-editor approved 02-dark-pro.html +2026-07-08 fix/product-safety-review reused 02-dark-pro.html for non-visual drop safety changes diff --git a/src/Dropwheel/Services/FileOps.cs b/src/Dropwheel/Services/FileOps.cs index 8fba5bb..17f388b 100644 --- a/src/Dropwheel/Services/FileOps.cs +++ b/src/Dropwheel/Services/FileOps.cs @@ -1,3 +1,4 @@ +using System.IO; using System.Runtime.InteropServices; using Dropwheel.Models; @@ -9,7 +10,7 @@ public static class FileOps { private const uint FO_MOVE = 0x0001, FO_COPY = 0x0002, FO_DELETE = 0x0003; private const ushort FOF_ALLOWUNDO = 0x0040, FOF_NOCONFIRMMKDIR = 0x0200, FOF_NOCONFIRMATION = 0x0010, - FOF_SILENT = 0x0004, FOF_NOERRORUI = 0x0400; + FOF_SILENT = 0x0004, FOF_RENAMEONCOLLISION = 0x0008, FOF_NOERRORUI = 0x0400; [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] private struct SHFILEOPSTRUCT @@ -35,17 +36,29 @@ public static bool Execute(IEnumerable files, string destFolder, DropAct var list = files.ToArray(); if (list.Length == 0) return true; // nothing to do — don't call SHFileOperation with an empty list ushort flags = FOF_ALLOWUNDO | FOF_NOCONFIRMMKDIR; - if (silent) flags |= (ushort)(FOF_SILENT | FOF_NOERRORUI | FOF_NOCONFIRMATION); + if (silent) flags |= (ushort)(FOF_SILENT | FOF_NOERRORUI | FOF_NOCONFIRMATION | FOF_RENAMEONCOLLISION); var op = new SHFILEOPSTRUCT { - wFunc = action == DropAction.Move ? FO_MOVE : FO_COPY, - pFrom = string.Join("\0", list) + "\0\0", - pTo = destFolder + "\0\0", + wFunc = action == DropAction.Move ? FO_MOVE : FO_COPY, + pFrom = string.Join("\0", list) + "\0\0", + pTo = destFolder + "\0\0", fFlags = flags, }; return SHFileOperation(ref op) == 0 && !op.fAnyOperationsAborted; } + public static bool HasDestinationCollision(IEnumerable sources, string destFolder) + { + foreach (var source in sources) + { + var name = Path.GetFileName(source); + if (string.IsNullOrEmpty(name)) continue; + var dest = Path.Combine(destFolder, name); + if (File.Exists(dest) || Directory.Exists(dest)) return true; + } + return false; + } + /// Delete to Recycle Bin without confirmation (for Undo after a copy). public static bool Delete(IEnumerable paths) { @@ -53,9 +66,9 @@ public static bool Delete(IEnumerable paths) if (list.Length == 0) return true; var op = new SHFILEOPSTRUCT { - wFunc = FO_DELETE, - pFrom = string.Join("\0", list) + "\0\0", - pTo = "\0\0", + wFunc = FO_DELETE, + pFrom = string.Join("\0", list) + "\0\0", + pTo = "\0\0", fFlags = FOF_ALLOWUNDO | FOF_NOCONFIRMATION, }; return SHFileOperation(ref op) == 0 && !op.fAnyOperationsAborted; diff --git a/src/Dropwheel/Services/SortService.cs b/src/Dropwheel/Services/SortService.cs index de63eb2..9571cf2 100644 --- a/src/Dropwheel/Services/SortService.cs +++ b/src/Dropwheel/Services/SortService.cs @@ -9,6 +9,8 @@ namespace Dropwheel.Services; /// Distributes files according to a sorter target's rules. public static class SortService { + private static readonly TimeSpan RegexTimeout = TimeSpan.FromMilliseconds(250); + /// Returns a plan: destination folder → files. Uses the rich Rules engine when /// the target has Rules, otherwise the legacy SortRules. With no match and no catch-all /// a file goes to the target root (t.Path). @@ -111,7 +113,7 @@ private static Dictionary CollectGroups(SortRule rule, string fi foreach (var c in rule.All) { if (c.Field != ConditionField.NameRegex || Compiled(c.Value) is not { } rx) continue; - var m = rx.Match(fileName); + if (!TryMatch(rx, fileName, c.Value, out var m)) continue; if (!m.Success) continue; foreach (var name in rx.GetGroupNames()) { @@ -154,7 +156,7 @@ private static string SanitizeSegment(string value) { ConditionField.Extension => MatchExtension(c.Value, meta.Ext), ConditionField.NameContains => meta.Name.Contains(c.Value, StringComparison.OrdinalIgnoreCase), - ConditionField.NameRegex => Compiled(c.Value) is { } rx && rx.IsMatch(meta.Name), + ConditionField.NameRegex => Compiled(c.Value) is { } rx && IsMatch(rx, meta.Name, c.Value), ConditionField.SizeMb => MatchNumber(c.Op, meta.SizeMb, c.Value), ConditionField.AgeDays => MatchNumber(c.Op, meta.AgeDays, c.Value), _ => false, @@ -198,7 +200,8 @@ private static bool MatchNumber(CompareOp op, double actual, string value) try { rx = new Regex(pattern, - RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.CultureInvariant); + RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.CultureInvariant, + RegexTimeout); } catch (ArgumentException ex) // invalid pattern (RegexParseException derives from this) { @@ -208,4 +211,29 @@ private static bool MatchNumber(CompareOp op, double actual, string value) RegexCache[pattern] = rx; return rx; } + + private static bool IsMatch(Regex rx, string input, string pattern) + { + try { return rx.IsMatch(input); } + catch (RegexMatchTimeoutException ex) + { + ErrorLog.Write($"Regular expression timed out in rule: '{pattern}'", ex); + return false; + } + } + + private static bool TryMatch(Regex rx, string input, string pattern, out System.Text.RegularExpressions.Match match) + { + try + { + match = rx.Match(input); + return true; + } + catch (RegexMatchTimeoutException ex) + { + ErrorLog.Write($"Regular expression timed out in rule: '{pattern}'", ex); + match = System.Text.RegularExpressions.Match.Empty; + return false; + } + } } diff --git a/src/Dropwheel/Services/TargetStore.cs b/src/Dropwheel/Services/TargetStore.cs index 1f5e67e..47e8eb8 100644 --- a/src/Dropwheel/Services/TargetStore.cs +++ b/src/Dropwheel/Services/TargetStore.cs @@ -32,6 +32,7 @@ public static class TargetStore public static void Load() { + bool shouldBackup = false; if (File.Exists(FilePath)) { try @@ -40,14 +41,34 @@ public static void Load() if (Config.Presets == null) { Config.Presets = PresetService.Defaults(); Save(); } return; } - catch (JsonException) { /* corrupted config — recreate with defaults */ } - catch (IOException) { /* unreadable config — recreate with defaults */ } - catch (UnauthorizedAccessException) { /* unreadable config — recreate with defaults */ } + catch (JsonException ex) { ErrorLog.Write("Config is corrupted; backing it up and recreating defaults", ex); shouldBackup = true; } + catch (IOException ex) { ErrorLog.Write("Config is unreadable; backing it up and recreating defaults", ex); shouldBackup = true; } + catch (UnauthorizedAccessException ex) { ErrorLog.Write("Config is unreadable; backing it up and recreating defaults", ex); shouldBackup = true; } } + if (shouldBackup) BackupBadConfig(DateTime.Now); Config = Defaults(); Save(); } + internal static string BackupPath(DateTime now) + { + var stamp = now.ToString("yyyyMMdd_HHmmss"); + return Path.Combine(Dir, $"config.bad.{stamp}.json"); + } + + private static void BackupBadConfig(DateTime now) + { + try + { + if (!File.Exists(FilePath)) return; + var backup = BackupPath(now); + for (int i = 2; File.Exists(backup); i++) + backup = Path.Combine(Dir, $"config.bad.{now:yyyyMMdd_HHmmss}.{i}.json"); + File.Copy(FilePath, backup); + } + catch (Exception ex) { ErrorLog.Write("Failed to back up bad config", ex); } + } + /// Writes via a temp file then renames it: if the process is killed mid-write, the /// target config.json stays intact instead of becoming half-empty. public static void Save() @@ -100,5 +121,4 @@ private static AppConfig Defaults() }, }; } - } diff --git a/src/Dropwheel/Services/VirtualFileService.Streams.cs b/src/Dropwheel/Services/VirtualFileService.Streams.cs index 8d35b7e..7b702de 100644 --- a/src/Dropwheel/Services/VirtualFileService.Streams.cs +++ b/src/Dropwheel/Services/VirtualFileService.Streams.cs @@ -14,6 +14,7 @@ public static partial class VirtualFileService private static bool SaveContents(IComData com, int index, string path) { + var tmp = TempPathFor(path); var fmt = new FORMATETC { cfFormat = (short)System.Windows.DataFormats.GetDataFormat(ContentsFormat).Id, @@ -25,14 +26,24 @@ private static bool SaveContents(IComData com, int index, string path) try { if (med.unionmember == IntPtr.Zero) return false; // the source gave no medium for this index - if (med.tymed == TYMED.TYMED_ISTREAM) { SaveIStream(med.unionmember, path); return true; } - if (med.tymed == TYMED.TYMED_HGLOBAL) { SaveHGlobal(med.unionmember, path); return true; } - return false; + var saved = med.tymed switch + { + TYMED.TYMED_ISTREAM => SaveIStream(med.unionmember, tmp), + TYMED.TYMED_HGLOBAL => SaveHGlobal(med.unionmember, tmp), + _ => false, + }; + if (!saved) return false; + File.Move(tmp, path); + return true; + } + finally + { + try { if (File.Exists(tmp)) File.Delete(tmp); } catch { } + ReleaseStgMedium(ref med); } - finally { ReleaseStgMedium(ref med); } } - private static void SaveIStream(IntPtr punk, string path) + private static bool SaveIStream(IntPtr punk, string path) { var stream = (IStream)Marshal.GetObjectForIUnknown(punk); try @@ -51,6 +62,7 @@ private static void SaveIStream(IntPtr punk, string path) } } finally { Marshal.FreeHGlobal(pRead); } + return true; } finally { Marshal.ReleaseComObject(stream); } } @@ -60,22 +72,27 @@ private static void SaveIStream(IntPtr punk, string path) /// in the file. HGLOBAL for CFSTR_FILECONTENTS has no reliable "real length" field, and trimming /// trailing zeros is wrong (a legitimate binary also contains zeros). In practice sources deliver /// files via ISTREAM (the branch above); this is a rare fallback. - private static void SaveHGlobal(IntPtr h, string path) + private static bool SaveHGlobal(IntPtr h, string path) { var p = GlobalLock(h); - if (p == IntPtr.Zero) return; + if (p == IntPtr.Zero) return false; try { var buf = new byte[(long)GlobalSize(h)]; Marshal.Copy(p, buf, 0, buf.Length); File.WriteAllBytes(path, buf); + return true; } finally { GlobalUnlock(h); } } + internal static string TempPathFor(string path) => + Path.Combine(Path.GetDirectoryName(path) ?? "", $".{Path.GetFileName(path)}.{Guid.NewGuid():N}.tmp"); + private static string UniquePath(string folder, string name) { name = string.Join("_", name.Split(Path.GetInvalidFileNameChars())); + if (string.IsNullOrWhiteSpace(name)) name = "file"; var path = Path.Combine(folder, name); if (!File.Exists(path) && !Directory.Exists(path)) return path; string stem = Path.GetFileNameWithoutExtension(name), ext = Path.GetExtension(name); diff --git a/src/Dropwheel/UI/OverlayWindow.Dnd.cs b/src/Dropwheel/UI/OverlayWindow.Dnd.cs index aba9091..f9df5b6 100644 --- a/src/Dropwheel/UI/OverlayWindow.Dnd.cs +++ b/src/Dropwheel/UI/OverlayWindow.Dnd.cs @@ -77,8 +77,9 @@ private void OnBubbleDropCore(TargetItem t, DragEventArgs e) return; } var act = Resolve(t, e); + bool hadCollision = FileOps.HasDestinationCollision(files, dest); bool ok = FileOps.Execute(files, dest, act); - if (ok) RememberOp(act, files, dest); + if (ok) RememberOpIfUnambiguous(act, files, dest, hadCollision); ShowToast(ok ? $"{(act == DropAction.Move ? "➜ Moved" : "⧉ Copied")}: {files.Length} item(s) → {t.Name}" : "Operation was not completed", ok); @@ -89,7 +90,7 @@ private void OnBubbleDropCore(TargetItem t, DragEventArgs e) if (saved.Length > 0) { if (t.IsSorter) SortSavedVirtuals(t, saved); - else RememberOp(DropAction.Copy, saved, dest); + else RememberOpIfUnambiguous(DropAction.Copy, saved, dest, hadCollision: false); } ShowToast(saved.Length > 0 ? $"⧉ Saved: {saved.Length} item(s) → {t.Name}" @@ -101,7 +102,7 @@ private void OnBubbleDropCore(TargetItem t, DragEventArgs e) if (saved is { } path) { if (t.IsSorter) SortSavedVirtuals(t, new[] { path }); - else RememberOp(DropAction.Copy, new[] { path }, dest); + else RememberOpIfUnambiguous(DropAction.Copy, new[] { path }, dest, hadCollision: false); } ShowToast(saved != null ? $"≡ Saved text → {System.IO.Path.GetFileName(saved)}" diff --git a/src/Dropwheel/UI/OverlayWindow.Sort.cs b/src/Dropwheel/UI/OverlayWindow.Sort.cs index 2f87365..722e6d7 100644 --- a/src/Dropwheel/UI/OverlayWindow.Sort.cs +++ b/src/Dropwheel/UI/OverlayWindow.Sort.cs @@ -12,14 +12,15 @@ private void DropSorted(TargetItem t, string[] files, DropAction act) { var plan = SortService.Plan(t, files); bool ok = true; - var ops = new List<(DropAction, string[], string)>(); + var ops = new List<(DropAction, string[], string, bool)>(); foreach (var (folder, group) in plan) { Directory.CreateDirectory(folder); - if (FileOps.Execute(group, folder, act)) ops.Add((act, group.ToArray(), folder)); + bool hadCollision = FileOps.HasDestinationCollision(group, folder); + if (FileOps.Execute(group, folder, act)) ops.Add((act, group.ToArray(), folder, hadCollision)); else ok = false; } - if (ops.Count > 0) RememberOps(ops); + if (ops.Count > 0) RememberOpsIfUnambiguous(ops); ShowToast(ok ? $"⇅ Sorted: {files.Length} item(s) → {t.Name}" : "Sorting was not completed", ops.Count > 0); @@ -30,16 +31,17 @@ private void DropSorted(TargetItem t, string[] files, DropAction act) private void SortSavedVirtuals(TargetItem t, string[] saved) { var plan = SortService.Plan(t, saved); - var ops = new List<(DropAction, string[], string)>(); + var ops = new List<(DropAction, string[], string, bool)>(); string root = IOPath.GetFullPath(t.Path).TrimEnd('\\'); foreach (var (folder, group) in plan) { if (IOPath.GetFullPath(folder).TrimEnd('\\') == root) - { ops.Add((DropAction.Copy, group.ToArray(), folder)); continue; } + { ops.Add((DropAction.Copy, group.ToArray(), folder, false)); continue; } Directory.CreateDirectory(folder); + bool hadCollision = FileOps.HasDestinationCollision(group, folder); if (FileOps.Execute(group, folder, DropAction.Move)) - ops.Add((DropAction.Copy, group.ToArray(), folder)); + ops.Add((DropAction.Copy, group.ToArray(), folder, hadCollision)); } - if (ops.Count > 0) RememberOps(ops); + if (ops.Count > 0) RememberOpsIfUnambiguous(ops); } } diff --git a/src/Dropwheel/UI/OverlayWindow.Undo.cs b/src/Dropwheel/UI/OverlayWindow.Undo.cs index 652fd98..bc6fdd8 100644 --- a/src/Dropwheel/UI/OverlayWindow.Undo.cs +++ b/src/Dropwheel/UI/OverlayWindow.Undo.cs @@ -11,11 +11,17 @@ public partial class OverlayWindow // One drop operation may consist of several moves (sorter). private readonly List<(DropAction Act, string[] Sources, string Dest)> _lastOps = new(); - private void RememberOp(DropAction act, string[] sources, string dest) - { _lastOps.Clear(); _lastOps.Add((act, sources, dest)); } + private void RememberOpIfUnambiguous(DropAction act, string[] sources, string dest, bool hadCollision) + { + _lastOps.Clear(); + if (!hadCollision) _lastOps.Add((act, sources, dest)); + } - private void RememberOps(IEnumerable<(DropAction, string[], string)> ops) - { _lastOps.Clear(); _lastOps.AddRange(ops); } + private void RememberOpsIfUnambiguous(IEnumerable<(DropAction Act, string[] Sources, string Dest, bool HadCollision)> ops) + { + _lastOps.Clear(); + _lastOps.AddRange(ops.Where(op => !op.HadCollision).Select(op => (op.Act, op.Sources, op.Dest))); + } private void OnUndoClick(object sender, MouseButtonEventArgs e) { diff --git a/tests/Dropwheel.Tests/AppConfigTests.cs b/tests/Dropwheel.Tests/AppConfigTests.cs index 58ac6cc..da4af27 100644 --- a/tests/Dropwheel.Tests/AppConfigTests.cs +++ b/tests/Dropwheel.Tests/AppConfigTests.cs @@ -1,4 +1,6 @@ +using System.IO; using Dropwheel.Models; +using Dropwheel.Services; namespace Dropwheel.Tests; @@ -19,4 +21,12 @@ public void Default_open_animation_speed_is_normal() Assert.Equal(1.0, config.OpenAnimationSpeed); } + + [Fact] + public void Bad_config_backup_path_is_timestamped_json() + { + var path = TargetStore.BackupPath(new DateTime(2026, 7, 8, 18, 30, 5)); + + Assert.EndsWith(Path.Combine("Dropwheel", "config.bad.20260708_183005.json"), path); + } } diff --git a/tests/Dropwheel.Tests/FileOpsTests.cs b/tests/Dropwheel.Tests/FileOpsTests.cs index d8826e6..3f84bbb 100644 --- a/tests/Dropwheel.Tests/FileOpsTests.cs +++ b/tests/Dropwheel.Tests/FileOpsTests.cs @@ -17,4 +17,31 @@ public void Execute_with_no_files_is_a_noop_success(DropAction action) => [Fact] public void Delete_with_no_paths_is_a_noop_success() => Assert.True(FileOps.Delete(Array.Empty())); + + [Fact] + public void Destination_collision_detects_existing_file_with_same_name() + { + var root = Path.Combine(Path.GetTempPath(), "dw_collision_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(root); + try + { + var source = Path.Combine(Path.GetTempPath(), "report.txt"); + File.WriteAllText(Path.Combine(root, "report.txt"), "existing"); + + Assert.True(FileOps.HasDestinationCollision(new[] { source }, root)); + } + finally { Directory.Delete(root, true); } + } + + [Fact] + public void Destination_collision_ignores_non_colliding_names() + { + var root = Path.Combine(Path.GetTempPath(), "dw_collision_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(root); + try + { + Assert.False(FileOps.HasDestinationCollision(new[] { @"C:\drop\new.txt" }, root)); + } + finally { Directory.Delete(root, true); } + } } diff --git a/tests/Dropwheel.Tests/SortServiceTests.cs b/tests/Dropwheel.Tests/SortServiceTests.cs index e7eb6dd..d0502c2 100644 --- a/tests/Dropwheel.Tests/SortServiceTests.cs +++ b/tests/Dropwheel.Tests/SortServiceTests.cs @@ -1,4 +1,5 @@ using System.IO; +using System.Diagnostics; using Dropwheel.Models; using Dropwheel.Services; @@ -161,6 +162,23 @@ public void Invalid_regex_rule_does_not_throw_and_lets_file_fall_through() Assert.Contains(img, plan[Path.Combine(_root, "Camera")]); } + [Fact] + public void Catastrophic_regex_times_out_and_later_rules_can_match() + { + var file = new string('a', 5000) + "!_marker.txt"; + var rules = new List + { + Rule("Bad", ConditionField.NameRegex, CompareOp.Matches, "^(a+)+$"), + Rule("Fallback", ConditionField.NameContains, CompareOp.Contains, "marker"), + }; + + var sw = Stopwatch.StartNew(); + var idx = SortService.MatchedRuleIndex(rules, file); + + Assert.Equal(1, idx); + Assert.True(sw.Elapsed < TimeSpan.FromSeconds(5)); + } + [Fact] public void MatchedRuleIndex_distinguishes_rules_with_the_same_destination() { diff --git a/tests/Dropwheel.Tests/VirtualFileServiceTests.cs b/tests/Dropwheel.Tests/VirtualFileServiceTests.cs index 6c443ee..26e989b 100644 --- a/tests/Dropwheel.Tests/VirtualFileServiceTests.cs +++ b/tests/Dropwheel.Tests/VirtualFileServiceTests.cs @@ -1,3 +1,4 @@ +using System.IO; using System.Text; using Dropwheel.Services; @@ -48,4 +49,15 @@ public void Truncated_buffer_yields_empty() { Assert.Empty(VirtualFileService.ParseDescriptorNames(new byte[] { 1, 0 })); } + + [Fact] + public void Temp_path_for_virtual_file_stays_next_to_destination_and_is_hidden_tmp() + { + var dest = Path.Combine(Path.GetTempPath(), "invoice.pdf"); + var tmp = VirtualFileService.TempPathFor(dest); + + Assert.Equal(Path.GetDirectoryName(dest), Path.GetDirectoryName(tmp)); + Assert.StartsWith(".invoice.pdf.", Path.GetFileName(tmp)); + Assert.EndsWith(".tmp", tmp); + } }