mirror of
https://github.com/rclone/rclone.git
synced 2026-03-26 03:12:36 -04:00
list: fix nil pointer panic in Sorter when temp file creation fails
When extsort.Strings() cannot create temporary files (e.g. due to apparmor restrictions or permission denied), it returns a nil sorter with the error on errChan. The code then called Sort() on the nil sorter, causing a panic. Check for nil sorter and return the error instead of panicking. Fixes #9244
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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{}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user