Skip to content

Commit 3ca406c

Browse files
committed
fix: report dropped segments when sanitizing folder paths
ClipRepository.SanitizeFolderPath now logs a warning naming the dropped segments and what was kept, and raises FolderPathSanitized so the host can surface a non-blocking notice. The save flow in MainWindowViewModel captures these notices and appends a sanitized suffix to the final status, so programmatic placements that landed somewhere other than requested are visible to the user.
1 parent f90944e commit 3ca406c

5 files changed

Lines changed: 200 additions & 11 deletions

File tree

src/SharpFM/Models/ClipRepository.cs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -234,19 +234,41 @@ private static IReadOnlyList<string> GetRelativeFolderSegments(string root, stri
234234
return decoded;
235235
}
236236

237-
// Reject traversal segments (security) and percent-encode any character
238-
// the filesystem rejects so FileMaker names containing '/' or ':' survive
239-
// the round-trip instead of being silently dropped.
240-
private static IReadOnlyList<string> SanitizeFolderPath(IReadOnlyList<string> segments)
237+
/// <summary>
238+
/// Raised when <see cref="SanitizeFolderPath"/> drops one or more segments
239+
/// (empty, whitespace, or traversal). Hosts can subscribe to surface a
240+
/// non-blocking notification so plugin-driven placements that landed
241+
/// somewhere other than requested are visible to the user.
242+
/// </summary>
243+
public event EventHandler<FolderPathSanitizedEventArgs>? FolderPathSanitized;
244+
245+
// Empty / "." / ".." segments are dropped (not encoded) — the call is
246+
// best-effort, not a hard error, but the dropped segments are reported via
247+
// Log.Warn and FolderPathSanitized so the caller can surface them.
248+
private IReadOnlyList<string> SanitizeFolderPath(IReadOnlyList<string> segments)
241249
{
242250
if (segments is null || segments.Count == 0) return [];
243251
var safe = new List<string>(segments.Count);
252+
var dropped = new List<string>();
244253
foreach (var raw in segments)
245254
{
246-
if (string.IsNullOrWhiteSpace(raw)) continue;
247-
if (raw == "." || raw == "..") continue;
255+
if (string.IsNullOrWhiteSpace(raw) || raw == "." || raw == "..")
256+
{
257+
dropped.Add(raw ?? "");
258+
continue;
259+
}
248260
safe.Add(EncodeName(raw));
249261
}
262+
263+
if (dropped.Count > 0)
264+
{
265+
Log.Warn(
266+
"Sanitized folder path: dropped [{Dropped}]; kept [{Kept}].",
267+
string.Join(", ", dropped),
268+
string.Join("/", safe));
269+
FolderPathSanitized?.Invoke(this, new FolderPathSanitizedEventArgs(segments, safe, dropped));
270+
}
271+
250272
return safe;
251273
}
252274

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
using System;
2+
using System.Collections.Generic;
3+
4+
namespace SharpFM.Models;
5+
6+
/// <summary>
7+
/// Payload for <see cref="ClipRepository.FolderPathSanitized"/>. Reports the
8+
/// original folder path, the path that was actually used, and the segments
9+
/// that were dropped during sanitization.
10+
/// </summary>
11+
public sealed class FolderPathSanitizedEventArgs : EventArgs
12+
{
13+
public IReadOnlyList<string> Original { get; }
14+
public IReadOnlyList<string> Sanitized { get; }
15+
public IReadOnlyList<string> DroppedSegments { get; }
16+
17+
public FolderPathSanitizedEventArgs(
18+
IReadOnlyList<string> original,
19+
IReadOnlyList<string> sanitized,
20+
IReadOnlyList<string> droppedSegments)
21+
{
22+
Original = original;
23+
Sanitized = sanitized;
24+
DroppedSegments = droppedSegments;
25+
}
26+
}

src/SharpFM/ViewModels/MainWindowViewModel.cs

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -230,14 +230,32 @@ public async Task SaveClipsStorageAsync()
230230
})
231231
.ToList();
232232

233-
// Folders persist first so the orphan sweep in SaveClipsAsync
234-
// sees up-to-date marker files when reclaiming empty directories.
235-
await _repository.SaveFoldersAsync(Folders.ToList());
236-
await _repository.SaveClipsAsync(clipData);
233+
var sanitized = 0;
234+
void OnSanitized(object? _, FolderPathSanitizedEventArgs __) => sanitized++;
235+
var fileRepo = _repository as ClipRepository;
236+
if (fileRepo is not null) fileRepo.FolderPathSanitized += OnSanitized;
237+
238+
try
239+
{
240+
// Folders persist first so the orphan sweep in SaveClipsAsync
241+
// sees up-to-date marker files when reclaiming empty directories.
242+
await _repository.SaveFoldersAsync(Folders.ToList());
243+
await _repository.SaveClipsAsync(clipData);
244+
}
245+
finally
246+
{
247+
if (fileRepo is not null) fileRepo.FolderPathSanitized -= OnSanitized;
248+
}
237249

238250
foreach (var c in FileMakerClips) c.MarkSaved();
239251

240-
ShowStatus($"Saved {clipData.Count} clip(s) to {_repository.CurrentLocation}");
252+
var hadSanitization = sanitized > 0;
253+
var suffix = hadSanitization
254+
? $"; sanitized {sanitized} folder path(s) — see log for dropped segments"
255+
: "";
256+
ShowStatus(
257+
$"Saved {clipData.Count} clip(s) to {_repository.CurrentLocation}{suffix}",
258+
isError: hadSanitization);
241259
}
242260
catch (Exception e)
243261
{

tests/SharpFM.Tests/Models/ClipRepositoryTests.cs

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,4 +502,99 @@ await repo.SaveClipsAsync([new("Evil", "Mac-XMSS", "<x/>")
502502
if (Directory.Exists(sibling)) Directory.Delete(sibling, true);
503503
}
504504
}
505+
506+
[Fact]
507+
public async Task SaveClipsAsync_TraversalSegment_RaisesFolderPathSanitizedEvent()
508+
{
509+
var dir = CreateTempDir();
510+
try
511+
{
512+
var repo = new ClipRepository(dir);
513+
FolderPathSanitizedEventArgs? captured = null;
514+
repo.FolderPathSanitized += (_, e) => captured = e;
515+
516+
await repo.SaveClipsAsync([new("X", "Mac-XMSS", "<x/>")
517+
{ FolderPath = new[] { "..", "Real" } }]);
518+
519+
Assert.NotNull(captured);
520+
Assert.Equal(new[] { "..", "Real" }, captured!.Original);
521+
Assert.Equal(new[] { "Real" }, captured.Sanitized);
522+
Assert.Equal(new[] { ".." }, captured.DroppedSegments);
523+
}
524+
finally { Directory.Delete(dir, true); }
525+
}
526+
527+
[Fact]
528+
public async Task SaveClipsAsync_EmptySegment_RaisesEventNamingDropped()
529+
{
530+
var dir = CreateTempDir();
531+
try
532+
{
533+
var repo = new ClipRepository(dir);
534+
FolderPathSanitizedEventArgs? captured = null;
535+
repo.FolderPathSanitized += (_, e) => captured = e;
536+
537+
await repo.SaveClipsAsync([new("X", "Mac-XMSS", "<x/>")
538+
{ FolderPath = new[] { "Real", " " } }]);
539+
540+
Assert.NotNull(captured);
541+
Assert.Equal(new[] { "Real" }, captured!.Sanitized);
542+
Assert.Single(captured.DroppedSegments);
543+
}
544+
finally { Directory.Delete(dir, true); }
545+
}
546+
547+
[Fact]
548+
public async Task SaveClipsAsync_AllValidSegments_DoesNotRaiseEvent()
549+
{
550+
var dir = CreateTempDir();
551+
try
552+
{
553+
var repo = new ClipRepository(dir);
554+
var fired = 0;
555+
repo.FolderPathSanitized += (_, _) => fired++;
556+
557+
await repo.SaveClipsAsync([new("X", "Mac-XMSS", "<x/>")
558+
{ FolderPath = new[] { "Group", "Sub" } }]);
559+
560+
Assert.Equal(0, fired);
561+
}
562+
finally { Directory.Delete(dir, true); }
563+
}
564+
565+
[Fact]
566+
public async Task SaveClipsAsync_PercentEncodedOnly_DoesNotRaiseEvent()
567+
{
568+
var dir = CreateTempDir();
569+
try
570+
{
571+
var repo = new ClipRepository(dir);
572+
var fired = 0;
573+
repo.FolderPathSanitized += (_, _) => fired++;
574+
575+
await repo.SaveClipsAsync([new("X", "Mac-XMSS", "<x/>")
576+
{ FolderPath = new[] { "Date/Time" } }]);
577+
578+
Assert.Equal(0, fired);
579+
}
580+
finally { Directory.Delete(dir, true); }
581+
}
582+
583+
[Fact]
584+
public async Task SaveFoldersAsync_TraversalSegment_RaisesFolderPathSanitizedEvent()
585+
{
586+
var dir = CreateTempDir();
587+
try
588+
{
589+
var repo = new ClipRepository(dir);
590+
FolderPathSanitizedEventArgs? captured = null;
591+
repo.FolderPathSanitized += (_, e) => captured = e;
592+
593+
await repo.SaveFoldersAsync([new(new[] { "..", "Real" })]);
594+
595+
Assert.NotNull(captured);
596+
Assert.Equal(new[] { ".." }, captured!.DroppedSegments);
597+
}
598+
finally { Directory.Delete(dir, true); }
599+
}
505600
}

tests/SharpFM.Tests/ViewModels/MainWindowViewModelTests.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,34 @@ public async Task CopySelectedToClip_NoSelection_ShowsStatus()
421421
Assert.Equal("No clip selected", vm.StatusMessage);
422422
}
423423

424+
[Fact]
425+
public async Task SaveClips_SanitizationDropsSegment_StatusIncludesSanitizedSuffix()
426+
{
427+
var vm = CreateVm();
428+
ResetRepoState(vm);
429+
var clip = Clip.FromXml("Test", "Mac-XMSC", "<x/>");
430+
vm.FileMakerClips.Add(new ClipViewModel(clip) { FolderPath = new[] { "..", "Real" } });
431+
432+
await vm.SaveClipsStorageAsync();
433+
434+
Assert.Contains("Saved", vm.StatusMessage);
435+
Assert.Contains("sanitized 1 folder path", vm.StatusMessage);
436+
}
437+
438+
[Fact]
439+
public async Task SaveClips_NoSanitization_StatusOmitsSuffix()
440+
{
441+
var vm = CreateVm();
442+
ResetRepoState(vm);
443+
var clip = Clip.FromXml("Test", "Mac-XMSC", "<x/>");
444+
vm.FileMakerClips.Add(new ClipViewModel(clip));
445+
446+
await vm.SaveClipsStorageAsync();
447+
448+
Assert.Contains("Saved", vm.StatusMessage);
449+
Assert.DoesNotContain("sanitized", vm.StatusMessage, StringComparison.OrdinalIgnoreCase);
450+
}
451+
424452
[Fact]
425453
public async Task PasteFileMakerClipData_NoFormats_ShowsStatus()
426454
{

0 commit comments

Comments
 (0)