From 7c33d978c49be945ec22c46c5f670c329cf2d307 Mon Sep 17 00:00:00 2001 From: Levi Date: Thu, 17 Sep 2026 23:35:40 +0800 Subject: [PATCH 1/3] fix(bt): clean partial files and empty parent directories --- internal/protocol/bt/fetcher.go | 34 ++++++++++++-- internal/protocol/bt/selection_test.go | 62 ++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/internal/protocol/bt/fetcher.go b/internal/protocol/bt/fetcher.go index 759b8fc1..bc32f741 100644 --- a/internal/protocol/bt/fetcher.go +++ b/internal/protocol/bt/fetcher.go @@ -399,10 +399,8 @@ func (f *Fetcher) Wait() (err error) { } } if !selected { - // Path already includes the torrent's root directory. - name := filepath.Join(f.meta.Opts.Path, file.Path()) - if err := util.SafeRemove(name); err != nil { - return fmt.Errorf("remove unselected torrent file %q: %w", name, err) + if err := removeUnselectedFile(f.meta.Opts.Path, file.Path()); err != nil { + return err } } } @@ -413,6 +411,34 @@ func (f *Fetcher) Wait() (err error) { } } +// Path already includes the torrent's root directory. Never prune the download +// directory itself, and only remove parent directories that are empty. +func removeUnselectedFile(downloadDir, torrentPath string) error { + root := filepath.Clean(downloadDir) + name := filepath.Join(root, torrentPath) + for _, candidate := range []string{name, name + ".part"} { + if err := util.SafeRemove(candidate); err != nil { + return fmt.Errorf("remove unselected torrent file %q: %w", candidate, err) + } + } + for dir := filepath.Dir(name); dir != root && dir != filepath.Dir(dir); dir = filepath.Dir(dir) { + entries, err := os.ReadDir(dir) + if os.IsNotExist(err) { + continue + } + if err != nil { + return fmt.Errorf("read torrent directory %q: %w", dir, err) + } + if len(entries) != 0 { + break + } + if err := util.SafeRemove(dir); err != nil { + return fmt.Errorf("remove empty torrent directory %q: %w", dir, err) + } + } + return nil +} + func (f *Fetcher) isDone() bool { if f.meta.Opts == nil { return false diff --git a/internal/protocol/bt/selection_test.go b/internal/protocol/bt/selection_test.go index 6b99a2a2..f147cfc5 100644 --- a/internal/protocol/bt/selection_test.go +++ b/internal/protocol/bt/selection_test.go @@ -3,6 +3,7 @@ package bt import ( "context" "crypto/sha1" + "fmt" "os" "path/filepath" "testing" @@ -62,7 +63,7 @@ func TestStartUsesSelectionSubmittedAfterResolve(t *testing.T) { } func TestWaitCleansUnselectedFilesFromSharedPiece(t *testing.T) { - for _, mode := range []string{"partial", "all", "already removed", "remove error"} { + for _, mode := range []string{"partial", "part only", "both paths", "all", "already removed", "remove error"} { t.Run(mode, func(t *testing.T) { dir := t.TempDir() keepPath := filepath.Join(dir, "bundle", "keep.txt") @@ -118,7 +119,12 @@ func TestWaitCleansUnselectedFilesFromSharedPiece(t *testing.T) { if !f.isDone() { t.Fatal("selected file is not complete") } - if mode == "already removed" || mode == "remove error" { + if mode == "part only" || mode == "both paths" || mode == "all" { + if err := os.WriteFile(skipPath+".part", []byte("skip"), 0644); err != nil { + t.Fatal(err) + } + } + if mode == "part only" || mode == "already removed" || mode == "remove error" { if err := os.Remove(skipPath); err != nil { t.Fatal(err) } @@ -147,15 +153,63 @@ func TestWaitCleansUnselectedFilesFromSharedPiece(t *testing.T) { t.Fatalf("selected file changed: %q, %v", data, err) } if mode == "all" { + if _, err := os.Stat(skipPath + ".part"); err != nil { + t.Fatalf("selected part file was removed: %v", err) + } data, err := os.ReadFile(skipPath) if err != nil || string(data) != "skip" { t.Fatalf("selected file changed: %q, %v", data, err) } } else if mode != "remove error" { - if _, err := os.Stat(skipPath); !os.IsNotExist(err) { - t.Fatalf("unselected file remains: %v", err) + for _, removed := range []string{skipPath, skipPath + ".part", filepath.Dir(skipPath)} { + if _, err := os.Stat(removed); !os.IsNotExist(err) { + t.Fatalf("unselected file or empty directory remains: %s: %v", removed, err) + } } } }) } } + +func TestRemoveUnselectedFilePrunesOnlyEmptyParents(t *testing.T) { + for _, preserveSibling := range []bool{false, true} { + t.Run(fmt.Sprint(preserveSibling), func(t *testing.T) { + root := t.TempDir() + relative := filepath.Join("bundle", "nested", "deep", "skip.txt") + name := filepath.Join(root, relative) + if err := os.MkdirAll(filepath.Dir(name), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(name+".part", []byte("partial"), 0644); err != nil { + t.Fatal(err) + } + sibling := filepath.Join(root, "bundle", "keep.txt") + if preserveSibling { + if err := os.WriteFile(sibling, []byte("keep"), 0644); err != nil { + t.Fatal(err) + } + } + if err := removeUnselectedFile(root, relative); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(root, "bundle", "nested")); !os.IsNotExist(err) { + t.Fatalf("empty ancestors remain: %v", err) + } + if preserveSibling { + data, err := os.ReadFile(sibling) + if err != nil || string(data) != "keep" { + t.Fatalf("sibling changed: %q, %v", data, err) + } + } else if _, err := os.Stat(filepath.Join(root, "bundle")); !os.IsNotExist(err) { + t.Fatalf("empty torrent directory remains: %v", err) + } + if _, err := os.Stat(root); err != nil { + t.Fatalf("download directory was removed: %v", err) + } + // Repeating cleanup after all files and directories are gone is harmless. + if err := removeUnselectedFile(root, relative); err != nil { + t.Fatal(err) + } + }) + } +} From 69b3e223adbbce28d31765b0fab8f267999136df Mon Sep 17 00:00:00 2001 From: Levi Date: Fri, 18 Sep 2026 09:23:46 +0800 Subject: [PATCH 2/3] fix(bt): always preserve the torrent root directory --- internal/protocol/bt/fetcher.go | 11 +++++++++-- internal/protocol/bt/selection_test.go | 18 +++++++++++++++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/internal/protocol/bt/fetcher.go b/internal/protocol/bt/fetcher.go index bc32f741..71aa80ff 100644 --- a/internal/protocol/bt/fetcher.go +++ b/internal/protocol/bt/fetcher.go @@ -411,11 +411,18 @@ func (f *Fetcher) Wait() (err error) { } } -// Path already includes the torrent's root directory. Never prune the download -// directory itself, and only remove parent directories that are empty. +// Path already includes the torrent's root directory. Preserve that directory +// even when empty, and only prune empty directories below it. func removeUnselectedFile(downloadDir, torrentPath string) error { + if !filepath.IsLocal(torrentPath) { + return fmt.Errorf("invalid torrent file path %q", torrentPath) + } root := filepath.Clean(downloadDir) + torrentPath = filepath.Clean(torrentPath) name := filepath.Join(root, torrentPath) + if torrentDir, _, hasDir := strings.Cut(torrentPath, string(filepath.Separator)); hasDir { + root = filepath.Join(root, torrentDir) + } for _, candidate := range []string{name, name + ".part"} { if err := util.SafeRemove(candidate); err != nil { return fmt.Errorf("remove unselected torrent file %q: %w", candidate, err) diff --git a/internal/protocol/bt/selection_test.go b/internal/protocol/bt/selection_test.go index f147cfc5..677cda54 100644 --- a/internal/protocol/bt/selection_test.go +++ b/internal/protocol/bt/selection_test.go @@ -200,13 +200,25 @@ func TestRemoveUnselectedFilePrunesOnlyEmptyParents(t *testing.T) { if err != nil || string(data) != "keep" { t.Fatalf("sibling changed: %q, %v", data, err) } - } else if _, err := os.Stat(filepath.Join(root, "bundle")); !os.IsNotExist(err) { - t.Fatalf("empty torrent directory remains: %v", err) + } + if info, err := os.Stat(filepath.Join(root, "bundle")); err != nil || !info.IsDir() { + t.Fatalf("torrent root directory was removed: %v", err) } if _, err := os.Stat(root); err != nil { t.Fatalf("download directory was removed: %v", err) } - // Repeating cleanup after all files and directories are gone is harmless. + // A file directly inside the torrent root must not cause that root to be pruned. + rootFile := filepath.Join("bundle", "root.txt") + if err := os.WriteFile(filepath.Join(root, rootFile)+".part", []byte("partial"), 0644); err != nil { + t.Fatal(err) + } + if err := removeUnselectedFile(root, rootFile); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(root, "bundle")); err != nil { + t.Fatalf("torrent root directory was removed after root-level cleanup: %v", err) + } + // Repeating cleanup after the file and its subdirectories are gone is harmless. if err := removeUnselectedFile(root, relative); err != nil { t.Fatal(err) } From 3c0c019bc956e0041ee0ceedc9565bba4c3a8bf5 Mon Sep 17 00:00:00 2001 From: Levi Date: Fri, 18 Sep 2026 09:33:42 +0800 Subject: [PATCH 3/3] fix(bt): derive cleanup boundaries from torrent storage layout --- internal/protocol/bt/fetcher.go | 46 ++++++++++---- internal/protocol/bt/selection_test.go | 83 ++++++++++++++++++++++++-- 2 files changed, 115 insertions(+), 14 deletions(-) diff --git a/internal/protocol/bt/fetcher.go b/internal/protocol/bt/fetcher.go index 71aa80ff..73b5175b 100644 --- a/internal/protocol/bt/fetcher.go +++ b/internal/protocol/bt/fetcher.go @@ -399,7 +399,8 @@ func (f *Fetcher) Wait() (err error) { } } if !selected { - if err := removeUnselectedFile(f.meta.Opts.Path, file.Path()); err != nil { + root, name := torrentFileLayout(f.torrent.Info(), file.FileInfo()) + if err := removeUnselectedFile(f.meta.Opts.Path, root, name); err != nil { return err } } @@ -411,17 +412,38 @@ func (f *Fetcher) Wait() (err error) { } } -// Path already includes the torrent's root directory. Preserve that directory -// even when empty, and only prune empty directories below it. -func removeUnselectedFile(downloadDir, torrentPath string) error { - if !filepath.IsLocal(torrentPath) { +// Match the file storage layout, including its NoName convention for rootless +// torrents. File.Path() always prepends BestName, even when storage omits it. +func torrentFileLayout(info *metainfo.Info, file metainfo.FileInfo) (root, name string) { + parts := file.BestPath() + if info.BestName() != metainfo.NoName { + if len(parts) > 0 { + root = info.BestName() + } + parts = append([]string{info.BestName()}, parts...) + } + // A v2 file tree can itself have a named root without an extra name wrapper. + if root == "" && info.HasV2() && len(info.FileTree.Dir) == 1 { + for dir, tree := range info.FileTree.Dir { + if tree.IsDir() { + root = dir + } + } + } + return root, filepath.Join(parts...) +} + +// Preserve the actual torrent root, or the download directory for rootless +// torrents. Only prune empty directories strictly below that boundary. +func removeUnselectedFile(downloadDir, torrentRoot, torrentPath string) error { + if !filepath.IsLocal(torrentPath) || (torrentRoot != "" && !filepath.IsLocal(torrentRoot)) { return fmt.Errorf("invalid torrent file path %q", torrentPath) } - root := filepath.Clean(downloadDir) - torrentPath = filepath.Clean(torrentPath) - name := filepath.Join(root, torrentPath) - if torrentDir, _, hasDir := strings.Cut(torrentPath, string(filepath.Separator)); hasDir { - root = filepath.Join(root, torrentDir) + root := filepath.Join(downloadDir, torrentRoot) + name := filepath.Join(downloadDir, torrentPath) + rel, err := filepath.Rel(root, name) + if err != nil || rel == "." || !filepath.IsLocal(rel) { + return fmt.Errorf("torrent file %q is outside cleanup root %q", name, root) } for _, candidate := range []string{name, name + ".part"} { if err := util.SafeRemove(candidate); err != nil { @@ -675,6 +697,10 @@ func (f *Fetcher) addTorrent(req *base.Request, fromUpload bool) (err error) { } spec.Storage = storage.NewFileOpts(storage.NewFileClientOpts{ ClientBaseDir: cfg.DataDir, + FilePathMaker: func(opts storage.FilePathMakerOpts) string { + _, name := torrentFileLayout(opts.Info, *opts.File) + return name + }, TorrentDirMaker: func(baseDir string, info *metainfo.Info, infoHash metainfo.Hash) string { return f.meta.Opts.Path }, diff --git a/internal/protocol/bt/selection_test.go b/internal/protocol/bt/selection_test.go index 677cda54..8c2f9773 100644 --- a/internal/protocol/bt/selection_test.go +++ b/internal/protocol/bt/selection_test.go @@ -96,7 +96,7 @@ func TestWaitCleansUnselectedFilesFromSharedPiece(t *testing.T) { t.Fatal(err) } defer c.Close() - spec, err := torrent.TorrentSpecFromMetaInfoErr(&metainfo.MetaInfo{InfoBytes: bencode.MustMarshal(info)}) + spec, err := torrent.TorrentSpecFromMetaInfoErr(&metainfo.MetaInfo{InfoBytes: bencode.MustMarshal(&info)}) if err != nil { t.Fatal(err) } @@ -189,7 +189,7 @@ func TestRemoveUnselectedFilePrunesOnlyEmptyParents(t *testing.T) { t.Fatal(err) } } - if err := removeUnselectedFile(root, relative); err != nil { + if err := removeUnselectedFile(root, "bundle", relative); err != nil { t.Fatal(err) } if _, err := os.Stat(filepath.Join(root, "bundle", "nested")); !os.IsNotExist(err) { @@ -212,16 +212,91 @@ func TestRemoveUnselectedFilePrunesOnlyEmptyParents(t *testing.T) { if err := os.WriteFile(filepath.Join(root, rootFile)+".part", []byte("partial"), 0644); err != nil { t.Fatal(err) } - if err := removeUnselectedFile(root, rootFile); err != nil { + if err := removeUnselectedFile(root, "bundle", rootFile); err != nil { t.Fatal(err) } if _, err := os.Stat(filepath.Join(root, "bundle")); err != nil { t.Fatalf("torrent root directory was removed after root-level cleanup: %v", err) } // Repeating cleanup after the file and its subdirectories are gone is harmless. - if err := removeUnselectedFile(root, relative); err != nil { + if err := removeUnselectedFile(root, "bundle", relative); err != nil { t.Fatal(err) } }) } } + +func TestCleanupMatchesNamedAndRootlessStorage(t *testing.T) { + for _, version := range []int64{1, 2} { + for _, layout := range []string{"rootless", "wrapped", "file-tree-root"} { + if version == 1 && layout == "file-tree-root" { + continue + } + t.Run(fmt.Sprintf("v%d/%s", version, layout), func(t *testing.T) { + dir := t.TempDir() + info := metainfo.Info{Name: metainfo.NoName, PieceLength: 16384} + root := dir + if layout == "wrapped" { + info.Name = "bundle" + root = filepath.Join(dir, "bundle") + } + if version == 1 { + info.Files = []metainfo.FileInfo{{Path: []string{"keep.txt"}}, {Path: []string{"nested", "deep", "skip.txt"}}} + } else { + info.MetaVersion = 2 + info.FileTree = metainfo.FileTree{Dir: map[string]metainfo.FileTree{ + "keep.txt": {}, + "nested": {Dir: map[string]metainfo.FileTree{"deep": {Dir: map[string]metainfo.FileTree{"skip.txt": {}}}}}, + }} + } + if layout == "file-tree-root" { + info.FileTree = metainfo.FileTree{Dir: map[string]metainfo.FileTree{"tree-root": info.FileTree}} + root = filepath.Join(dir, "tree-root") + } + // Keep the library's default FilePathMaker here to independently verify + // that cleanup agrees with its real disk layout, including NoName. + store := storage.NewFileOpts(storage.NewFileClientOpts{ + ClientBaseDir: dir, + TorrentDirMaker: func(baseDir string, _ *metainfo.Info, _ metainfo.Hash) string { return baseDir }, + }) + defer store.Close() + torrentStorage, err := store.OpenTorrent(context.Background(), &info, metainfo.Hash{1}) + if err != nil { + t.Fatal(err) + } + defer torrentStorage.Close() + skip := filepath.Join(root, "nested", "deep", "skip.txt") + if _, err := os.Stat(skip); err != nil { + t.Fatalf("unexpected storage layout: %v", err) + } + if err := os.WriteFile(skip+".part", []byte("partial"), 0644); err != nil { + t.Fatal(err) + } + files := info.UpvertedFiles() + boundary, name := torrentFileLayout(&info, files[1]) + if err := removeUnselectedFile(dir, boundary, name); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(root, "nested")); !os.IsNotExist(err) { + t.Fatalf("empty subtree remains: %v", err) + } + if _, err := os.Stat(filepath.Join(root, "keep.txt")); err != nil { + t.Fatalf("selected file removed: %v", err) + } + // Also verify the boundary when the protected directory becomes empty. + if err := os.Remove(filepath.Join(root, "keep.txt")); err != nil { + t.Fatal(err) + } + for _, file := range files { + boundary, name := torrentFileLayout(&info, file) + if err := removeUnselectedFile(dir, boundary, name); err != nil { + t.Fatal(err) + } + } + if stat, err := os.Stat(root); err != nil || !stat.IsDir() { + t.Fatalf("cleanup boundary was removed: %v", err) + } + }) + } + } +}