diff --git a/README.md b/README.md index 6508cbe..d9c042a 100644 --- a/README.md +++ b/README.md @@ -428,6 +428,15 @@ first dupe size /srv/a/big.iso /srv/c/big-copy2.iso 4294967296 ``` +Paths are raw bytes and may hold any byte except NUL, so the path +columns (`first` and `dupe`) are escaped to keep every row one line of +tab-separated fields: a backslash is written as `\\`, a tab as `\t`, a +newline as `\n`, and a carriage return as `\r`. Every other byte is +written unchanged, including bytes that are not valid UTF-8. Undoing +those four escapes gives back the stored path. Grouping and ordering +use the stored path, not the escaped one. The warnings `scan` prints on +stderr are escaped the same way, so each warning is one line. + Summary to stderr: records read, number of duplicate groups, number of dupe files, and total reclaimable bytes (sum of `size` over all dupe rows) in human units. @@ -499,6 +508,9 @@ first dupe files size /srv/a/project /srv/backup/project 3417 104857600 ``` +The `first` and `dupe` paths are escaped as described under "Report +output format". The root directory's path is `/`. + Summary to stderr: records read, number of duplicate-tree groups, number of dupe trees, and total reclaimable bytes (sum of `size` over all dupe rows) in human units. diff --git a/TODO.md b/TODO.md index b540d3a..7724ba8 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,10 @@ # Completed Steps +- escape tabs, newlines, carriage returns and backslashes in report, + trees and warning paths; the root directory's path is `/` + (2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/7) + - stamp the git tag or short commit in a plain `docker build .` instead of `dev` (2026-10-02, branch `next`, closes https://git.eeqj.de/sneak/sfdupes/issues/67): `.dockerignore` now diff --git a/progress.go b/progress.go index ecb0585..f351d47 100644 --- a/progress.go +++ b/progress.go @@ -106,6 +106,8 @@ func (p *progress) increment() { } // warnf prints a one-line warning to stderr without corrupting the bar. +// The whole message is escaped like a report's path columns, so a path +// holding a newline cannot split the warning. func (p *progress) warnf(format string, args ...any) { if p == nil { return @@ -115,7 +117,7 @@ func (p *progress) warnf(format string, args ...any) { _ = p.bar.Clear() } - fmt.Fprintf(os.Stderr, format+"\n", args...) + fmt.Fprintln(os.Stderr, escapePath(fmt.Sprintf(format, args...))) } // finish terminates the pass's display. diff --git a/report.go b/report.go index c5761b5..f02ce2b 100644 --- a/report.go +++ b/report.go @@ -87,7 +87,7 @@ func runReport(ctx context.Context) error { for _, g := range dupes { for _, p := range g.paths[1:] { _, err = fmt.Fprintf(out, "%s\t%s\t%d\n", - g.paths[0], p, g.size) + escapePath(g.paths[0]), escapePath(p), g.size) if err != nil { return fmt.Errorf("write stdout: %w", err) } @@ -156,6 +156,21 @@ func collectDupeGroups(recs []scanRec) []dupeGroup { return dupes } +// escapePath returns a path as it is written in a report column (README +// "Report output format"): a backslash, tab, newline or carriage return +// becomes \\, \t, \n or \r, and every other byte is kept as it is. +// Grouping and sorting use the raw path, never this form. +func escapePath(p string) string { + // Most paths need no escaping; skip building a replacer for them. + if !strings.ContainsAny(p, "\\\t\n\r") { + return p + } + + return strings.NewReplacer( + `\`, `\\`, "\t", `\t`, "\n", `\n`, "\r", `\r`, + ).Replace(p) +} + // humanBytes formats a byte count in human units (binary prefixes). func humanBytes(n int64) string { const unit = 1024 diff --git a/report_test.go b/report_test.go index 1aa7cc5..d1aa290 100644 --- a/report_test.go +++ b/report_test.go @@ -1,10 +1,128 @@ package main import ( + "bytes" + "io" + "os" + "path/filepath" "slices" "testing" ) +// awkwardDir is a directory name holding every byte the reports escape. +const awkwardDir = "/d/\tone\ntwo\rthree\\four" + +// awkwardPairRecs is a duplicate pair in sibling directories /d/A and +// awkwardDir. A raw tab sorts before "A" but its escaped form `\t` +// sorts after it, so awkwardDir coming first shows that sorting uses +// the raw path. +func awkwardPairRecs() []scanRec { + return []scanRec{ + {size: 5, head: "h", tail: "t", content: "c", path: "/d/A/f"}, + {size: 5, head: "h", tail: "t", content: "c", path: awkwardDir + "/f"}, + } +} + +// seedDatabase writes recs into a fresh database and returns its path. +func seedDatabase(t *testing.T, recs []scanRec) string { + t.Helper() + + path := testDBPath(t) + + db, err := openScanDatabase(t.Context(), path) + if err != nil { + t.Fatal(err) + } + + err = applyChanges(t.Context(), db, recs, nil, nil) + if err != nil { + t.Fatal(err) + } + + err = db.Close() + if err != nil { + t.Fatal(err) + } + + return path +} + +func TestRunReportEscapesPaths(t *testing.T) { + t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs())) + + var stderr bytes.Buffer + + stdout := captureStdout(t) + + code := run([]string{cmdReport}, &stderr) + if code != exitOK { + t.Fatalf("run(report) = %d, want %d; stderr: %s", + code, exitOK, stderr.String()) + } + + want := "first\tdupe\tsize\n" + + `/d/\tone\ntwo\rthree\\four/f` + "\t/d/A/f\t5\n" + if got := stdout(); got != want { + t.Errorf("stdout = %q, want %q", got, want) + } +} + +func TestEscapePath(t *testing.T) { + t.Parallel() + + cases := map[string]string{ + "/srv/plain": "/srv/plain", + "/a\tb": `/a\tb`, + "/a\nb": `/a\nb`, + "/a\rb": `/a\rb`, + `/a\b`: `/a\\b`, + `/a\tb`: `/a\\tb`, + "/not-utf8\xff": "/not-utf8\xff", + } + for in, want := range cases { + if got := escapePath(in); got != want { + t.Errorf("escapePath(%q) = %q, want %q", in, got, want) + } + } +} + +// TestWarnfEscapes checks that a warning naming a path that holds a +// newline is still one line. +// +//nolint:paralleltest // replaces the process-wide os.Stderr +func TestWarnfEscapes(t *testing.T) { + f, err := os.Create(filepath.Join(t.TempDir(), "stderr")) + if err != nil { + t.Fatal(err) + } + + saved := os.Stderr + os.Stderr = f + + t.Cleanup(func() { + os.Stderr = saved + + _ = f.Close() + }) + + (&progress{}).warnf("stat %s: %s", "/d/a\nb", "gone") + + _, err = f.Seek(0, io.SeekStart) + if err != nil { + t.Fatal(err) + } + + got, err := io.ReadAll(f) + if err != nil { + t.Fatal(err) + } + + want := `stat /d/a\nb: gone` + "\n" + if string(got) != want { + t.Errorf("warning = %q, want %q", got, want) + } +} + func TestCollectDupeGroups(t *testing.T) { t.Parallel() diff --git a/trees.go b/trees.go index f5337fc..f43374c 100644 --- a/trees.go +++ b/trees.go @@ -62,7 +62,8 @@ func runTrees(ctx context.Context) error { first := g[0] for _, n := range g[1:] { _, err = fmt.Fprintf(out, "%s\t%s\t%d\t%d\n", - first.path, n.path, first.fileCount, first.totalSize) + escapePath(first.path), escapePath(n.path), + first.fileCount, first.totalSize) if err != nil { return fmt.Errorf("write stdout: %w", err) } @@ -87,8 +88,8 @@ func runTrees(ctx context.Context) error { // buildHierarchy reconstructs the directory hierarchy from the record // paths under a synthetic super-root. Paths are split on "/"; for -// absolute paths the first component is empty, which simply becomes a -// top-level node representing "/". It returns the super-root and every +// absolute paths the first component is empty, which becomes the +// top-level node with path "/". It returns the super-root and every // directory node created. func buildHierarchy(recs []scanRec) (*treeNode, []*treeNode) { super := &treeNode{} @@ -102,9 +103,17 @@ func buildHierarchy(recs []scanRec) (*treeNode, []*treeNode) { for _, c := range comps[:len(comps)-1] { child := node.dirs[c] if child == nil { - childPath := c - if node != super { - childPath = node.path + "/" + c + childPath := node.path + "/" + c + + // The root directory's path is "/", not empty, and its + // children's paths start with one slash, not two. + switch { + case node == super && c == "": + childPath = "/" + case node == super: + childPath = c + case node.path == "/": + childPath = "/" + c } child = &treeNode{path: childPath, parent: node} diff --git a/trees_test.go b/trees_test.go index d0995ae..abf61f1 100644 --- a/trees_test.go +++ b/trees_test.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "slices" "testing" ) @@ -84,6 +85,46 @@ func TestBuildHierarchyCounts(t *testing.T) { } } +func TestBuildHierarchyRootPath(t *testing.T) { + t.Parallel() + + // The root directory's path is "/", never empty, and its + // children's paths start with a single slash. + _, dirs := buildHierarchy([]scanRec{{path: "/f"}, {path: "/srv/g"}}) + + got := make([]string, 0, len(dirs)) + for _, d := range dirs { + got = append(got, d.path) + } + + slices.Sort(got) + + want := []string{"/", "/srv"} + if !slices.Equal(got, want) { + t.Fatalf("directory paths = %q, want %q", got, want) + } +} + +func TestRunTreesEscapesPaths(t *testing.T) { + t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs())) + + var stderr bytes.Buffer + + stdout := captureStdout(t) + + code := run([]string{cmdTrees}, &stderr) + if code != exitOK { + t.Fatalf("run(trees) = %d, want %d; stderr: %s", + code, exitOK, stderr.String()) + } + + want := "first\tdupe\tfiles\tsize\n" + + `/d/\tone\ntwo\rthree\\four` + "\t/d/A\t1\t5\n" + if got := stdout(); got != want { + t.Errorf("stdout = %q, want %q", got, want) + } +} + func TestTreeDigests(t *testing.T) { t.Parallel()