diff --git a/fs/list/sorter.go b/fs/list/sorter.go index b2674b294..cfe821238 100644 --- a/fs/list/sorter.go +++ b/fs/list/sorter.go @@ -50,6 +50,7 @@ type Sorter struct { errChan <-chan error // for getting errors from the ext sort sorter *extsort.StringSorter // external string sort errs *errcount.ErrCount // accumulate errors + tempDir string // directory for temp files (empty for default) } // KeyFn turns an entry into a sort key @@ -144,6 +145,7 @@ func (ls *Sorter) startExtSort() (err error) { ChanBuffSize: 1024, // small effect SortedChanBuffSize: 1024, // makes a lot of difference ChunkSize: 32 * 1024, // tuned for 50 char records (UUID sized) + TempFilesDir: ls.tempDir, // Defaults // ChunkSize: int(1e6), // amount of records to store in each chunk which will be written to disk // NumWorkers: 2, // maximum number of workers to use for parallel sorting @@ -152,6 +154,17 @@ func (ls *Sorter) startExtSort() (err error) { // TempFilesDir: "", // empty for use OS default ex: /tmp } ls.sorter, ls.outputChan, ls.errChan = extsort.Strings(ls.inputChan, &opt) + if ls.sorter == nil { + // extsort.Strings returns nil when it can't create temp + // files (e.g. due to permissions or apparmor restrictions). + // The error will be on errChan. + select { + case err = <-ls.errChan: + default: + err = errors.New("failed to initialise on-disk sort") + } + return fmt.Errorf("sorter: %w", err) + } go ls.sorter.Sort(ls.ctx) // Show we are extsorting now diff --git a/fs/list/sorter_test.go b/fs/list/sorter_test.go index b8db3725e..b1941c481 100644 --- a/fs/list/sorter_test.go +++ b/fs/list/sorter_test.go @@ -4,6 +4,9 @@ import ( "cmp" "context" "fmt" + "os" + "path/filepath" + "runtime" "slices" "strings" "testing" @@ -231,6 +234,51 @@ func TestSorterExt(t *testing.T) { } } +// Test that startExtSort returns an error instead of panicking when +// temp file creation fails (e.g. due to permissions or apparmor). +// See: https://github.com/rclone/rclone/issues/9244 +func TestSorterExtTempFileError(t *testing.T) { + ctx := context.Background() + ctx, ci := fs.AddConfig(ctx) + ci.ListCutoff = 1 // force ext sort on first Add + + callback := func(entries fs.DirEntries) error { + return nil + } + ls, err := NewSorter(ctx, nil, callback, nil) + require.NoError(t, err) + defer ls.CleanUp() + + // Override tempDir to a path where os.MkdirAll will fail, + // simulating apparmor or permission denial. This makes + // extsort.Strings return a nil sorter. + if runtime.GOOS == "windows" { + // On Windows, create a regular file and use a child path. + // os.MkdirAll fails because an intermediate component is a file. + // Windows stat returns ERROR_PATH_NOT_FOUND (treated as + // IsNotExist) so the extsort library won't fall back to a + // default directory. + blocker := filepath.Join(t.TempDir(), "notadir") + require.NoError(t, os.WriteFile(blocker, []byte("x"), 0644)) + ls.tempDir = filepath.Join(blocker, "sub") + } else { + // On Unix, create a read-only parent directory. stat on the + // child returns ENOENT so the extsort library won't fall + // back, but os.MkdirAll fails with EACCES. + parent := filepath.Join(t.TempDir(), "roparent") + require.NoError(t, os.Mkdir(parent, 0o755)) + require.NoError(t, os.Chmod(parent, 0o555)) + t.Cleanup(func() { _ = os.Chmod(parent, 0o755) }) + ls.tempDir = filepath.Join(parent, "child") + } + + // Add enough entries to trigger ext sort. Before the fix this + // would panic with nil pointer dereference. + err = ls.Add(fs.DirEntries{mockobject.Object("a"), mockobject.Object("b")}) + require.Error(t, err) + assert.Contains(t, err.Error(), "sorter:") +} + // benchFs implements enough of the fs.Fs interface for Sorter type benchFs struct{}