diff --git a/TODO.md b/TODO.md index 9f83b8a..04bb908 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,14 @@ # Completed Steps +- `scan` creates the database's directory where the database path leads: the + path with its last element cut off as text, never cleaned, so a `..` after a + symlinked directory is followed through the symlink, as when the file is + opened. With `SFDUPES_DATABASE=/srv/link/../new/db.sqlite` and `/srv/link` a + symlink to `/data/x`, it creates `/data/new` and the database in it, where it + used to create `/srv/new` and then fail (2026-10-17, + https://git.eeqj.de/sneak/sfdupes/issues/119) + - `scan` never records its own database: once the database and its `.lock` file are open, it notes the device and inode of the database file, its `-wal` and `-shm` files and its `.lock` file, and the walk passes over a file with one of diff --git a/db.go b/db.go index 4f15621..de61db5 100644 --- a/db.go +++ b/db.go @@ -109,10 +109,8 @@ func databasePath() (string, string, error) { } // The working directory as the kernel reports it, with no symlink - // in it. os.Getwd may return PWD instead, which can name it through - // a symlink; a ".." after that symlink then leads somewhere else - // once the path is cleaned, as scan cleans it to create the - // database's directory. + // in it, as README "Database" says. os.Getwd may return PWD + // instead, which can name it through a symlink. wd, err := unix.Getwd() if err != nil { return "", "", fmt.Errorf("resolve database path %s: %w", path, err) @@ -125,6 +123,24 @@ func databasePath() (string, string, error) { return strings.TrimSuffix(wd, "/") + "/" + path, source, nil } +// createDatabaseDirectory creates the directory the database at path +// goes in, and any directory above it, if missing. path is absolute, as +// databasePath returns it. The directory is path with its last element +// cut off as text, by filepath.Split. filepath.Dir would clean it, +// dropping each ".." together with the directory name before it; when +// that name is a symlink, the cleaned path names another directory than +// the one the system reaches when it opens path. +func createDatabaseDirectory(path string) error { + dir, _ := filepath.Split(path) + + err := os.MkdirAll(dir, dbDirPerm) + if err != nil { + return fmt.Errorf("create database directory: %w", err) + } + + return nil +} + // scanParams are the connection parameters for scan: read-write, with // WAL journaling and a busy timeout, so a report can run while a cron // scan is in progress. closeScanDatabase leaves WAL mode again. @@ -179,9 +195,9 @@ func openDB(path, params string) (*sql.DB, error) { // that deleted it would let the next scan lock a new file while // another still holds the old one. func lockScanDatabase(path string) (*os.File, error) { - err := os.MkdirAll(filepath.Dir(path), dbDirPerm) + err := createDatabaseDirectory(path) if err != nil { - return nil, fmt.Errorf("create database directory: %w", err) + return nil, err } // Like SQLite, this follows a symlink at path and creates its @@ -225,9 +241,9 @@ func lockScanDatabase(path string) (*os.File, error) { // openScanDatabase opens the database for the scan subcommand, creating // the file, its parent directory, and the schema as needed. func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) { - err := os.MkdirAll(filepath.Dir(path), dbDirPerm) + err := createDatabaseDirectory(path) if err != nil { - return nil, fmt.Errorf("create database directory: %w", err) + return nil, err } db, err := openDB(path, scanParams) diff --git a/db_test.go b/db_test.go index fc84584..427fb61 100644 --- a/db_test.go +++ b/db_test.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "io/fs" "os" "path/filepath" "slices" @@ -118,6 +119,78 @@ func TestOpenScanDatabaseCreates(t *testing.T) { } } +// TestScanCreatesDatabaseDirectoryAfterSymlink has a database path go +// through a symlink to a directory, then "..", to a missing directory. +// With srv/link a symlink to data/x, srv/link/../new/db.sqlite leads to +// data/new/db.sqlite. lockScanDatabase and openScanDatabase, the two +// places scan creates the database's directory, must each create the +// database there, and nothing at srv/new, where the path leads once +// cleaned as text. +func TestScanCreatesDatabaseDirectoryAfterSymlink(t *testing.T) { + t.Parallel() + + creates := map[string]func(ctx context.Context, path string) error{ + "lockScanDatabase": func(_ context.Context, path string) error { + lock, err := lockScanDatabase(path) + if err == nil { + _ = lock.Close() + } + + return err + }, + "openScanDatabase": func(ctx context.Context, path string) error { + db, err := openScanDatabase(ctx, path) + if err == nil { + _ = db.Close() + } + + return err + }, + } + + for name, create := range creates { + t.Run(name, func(t *testing.T) { + t.Parallel() + + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + + err = os.MkdirAll(filepath.Join(dir, "data", "x"), 0o750) + if err != nil { + t.Fatal(err) + } + + err = os.Mkdir(filepath.Join(dir, "srv"), 0o750) + if err != nil { + t.Fatal(err) + } + + err = os.Symlink(filepath.Join(dir, "data", "x"), + filepath.Join(dir, "srv", "link")) + if err != nil { + t.Fatal(err) + } + + err = create(t.Context(), dir+"/srv/link/../new/db.sqlite") + if err != nil { + t.Fatalf("%s: %v", name, err) + } + + _, err = os.Stat(filepath.Join(dir, "data", "new", "db.sqlite")) + if err != nil { + t.Errorf("database the path leads to: %v", err) + } + + _, err = os.Lstat(filepath.Join(dir, "srv", "new")) + if !errors.Is(err, fs.ErrNotExist) { + t.Errorf("srv/new: stat error %v, want it missing", err) + } + }) + } +} + // TestDatabaseFiles opens a database through a symlink to its file: // databaseFiles finds the database file and its -wal and -shm files // beside the symlink's target, where SQLite keeps them, and leaves out