Escape the database path in the SQLite connection string (closes #55)
check / check (push) Failing after 1s
check / check (push) Failing after 1s
openDB put the path into the connection string unescaped, so a ? or # in it ended the file name and a % started an escape: scan could silently fill a database under a shortened name. The path now goes through net/url as a file: URI. An absolute path gets an empty host and a relative path none, because SQLite reads what follows file:// up to the next slash as a host name. The path is not cleaned, so it stays exactly what the operator gave. A test runs scan, report and trees against such a file name given as an absolute path, as one starting with //, and as a relative path, and checks that only that file and its lock file exist afterwards. Model: opus-5-5
This commit is contained in:
@@ -283,7 +283,9 @@ All three subcommands operate on a single SQLite database file:
|
|||||||
|
|
||||||
- Location: the value of the `SFDUPES_DATABASE` environment variable
|
- Location: the value of the `SFDUPES_DATABASE` environment variable
|
||||||
when set and non-empty, otherwise `/var/lib/sfdupes/db.sqlite`.
|
when set and non-empty, otherwise `/var/lib/sfdupes/db.sqlite`.
|
||||||
There is no command-line flag.
|
There is no command-line flag. The path names the file exactly,
|
||||||
|
whatever characters it holds (`?`, `#` and `%` included); a
|
||||||
|
relative path is relative to the working directory.
|
||||||
- `scan` creates the database (and its parent directory) on first
|
- `scan` creates the database (and its parent directory) on first
|
||||||
use. `report` and `trees` require an existing database; a missing
|
use. `report` and `trees` require an existing database; a missing
|
||||||
database file is a fatal error (exit 1) telling the user to run
|
database file is a fatal error (exit 1) telling the user to run
|
||||||
|
|||||||
@@ -29,6 +29,9 @@
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- a database path holding `?`, `#` or `%` opens exactly the file it names
|
||||||
|
(2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/55)
|
||||||
|
|
||||||
- `scan` rejects `--workers` below 1 as a usage error instead of
|
- `scan` rejects `--workers` below 1 as a usage error instead of
|
||||||
running single-threaded (2026-10-04,
|
running single-threaded (2026-10-04,
|
||||||
https://git.eeqj.de/sneak/sfdupes/issues/10)
|
https://git.eeqj.de/sneak/sfdupes/issues/10)
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io/fs"
|
"io/fs"
|
||||||
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"slices"
|
"slices"
|
||||||
@@ -107,7 +108,19 @@ const reportParams = "mode=ro" +
|
|||||||
// openDB opens the SQLite database at path with the connection
|
// openDB opens the SQLite database at path with the connection
|
||||||
// parameters params. It does not create or verify the schema.
|
// parameters params. It does not create or verify the schema.
|
||||||
func openDB(path, params string) (*sql.DB, error) {
|
func openDB(path, params string) (*sql.DB, error) {
|
||||||
db, err := sql.Open("sqlite", "file:"+path+"?"+params)
|
// The path is escaped into a file: URI, so ?, # and % in it stay
|
||||||
|
// part of the file name. SQLite reads what follows file:// up to
|
||||||
|
// the next / as a host name, so an absolute path goes after an
|
||||||
|
// empty host (file:///abs) and a relative path goes without one
|
||||||
|
// (file:rel).
|
||||||
|
uri := url.URL{
|
||||||
|
Scheme: "file",
|
||||||
|
OmitHost: !filepath.IsAbs(path),
|
||||||
|
Path: path,
|
||||||
|
RawQuery: params,
|
||||||
|
}
|
||||||
|
|
||||||
|
db, err := sql.Open("sqlite", uri.String())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("open database %s: %w", path, err)
|
return nil, fmt.Errorf("open database %s: %w", path, err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import (
|
|||||||
"io/fs"
|
"io/fs"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"slices"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -639,6 +640,73 @@ func TestRunReportsNeedOnlyReadAccess(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// assertRunsUseDatabase runs scan, then report and trees, against the
|
||||||
|
// database that SFDUPES_DATABASE names, the file name in dir. It fails
|
||||||
|
// unless the reports find the duplicate pair the scan recorded and dir
|
||||||
|
// then holds only that file and its lock file: nothing was created
|
||||||
|
// under a shortened name.
|
||||||
|
func assertRunsUseDatabase(t *testing.T, dir, name string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
dupes := scanFixture(t)
|
||||||
|
|
||||||
|
want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n"
|
||||||
|
if got := runStdout(t, cmdReport); got != want {
|
||||||
|
t.Errorf("report stdout = %q, want %q", got, want)
|
||||||
|
}
|
||||||
|
|
||||||
|
want = "first\tdupe\tfiles\tsize\n" +
|
||||||
|
filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) + "\t1\t300\n"
|
||||||
|
if got := runStdout(t, cmdTrees); got != want {
|
||||||
|
t.Errorf("trees stdout = %q, want %q", got, want)
|
||||||
|
}
|
||||||
|
|
||||||
|
entries, err := os.ReadDir(dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
got := make([]string, 0, len(entries))
|
||||||
|
for _, e := range entries {
|
||||||
|
got = append(got, e.Name())
|
||||||
|
}
|
||||||
|
|
||||||
|
if wantFiles := []string{name, name + ".lock"}; !slices.Equal(got, wantFiles) {
|
||||||
|
t.Errorf("%s holds %q, want %q", dir, got, wantFiles)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRunDatabasePathUsedAsGiven(t *testing.T) {
|
||||||
|
// README §Database: the path names the database file exactly. In
|
||||||
|
// SQLite's connection string an unescaped ? or # would end the file
|
||||||
|
// name and % would start an escape, and a path starting with //
|
||||||
|
// could be read as a host name. %25 is a valid escape, so unescaped
|
||||||
|
// this name opens a file named a without any error.
|
||||||
|
const name = "a?b#c%25d e.sqlite"
|
||||||
|
|
||||||
|
t.Run("absolute", func(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
t.Setenv(databaseEnv, filepath.Join(dir, name))
|
||||||
|
|
||||||
|
assertRunsUseDatabase(t, dir, name)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("leading double slash", func(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
t.Setenv(databaseEnv, "/"+filepath.Join(dir, name))
|
||||||
|
|
||||||
|
assertRunsUseDatabase(t, dir, name)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("relative", func(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
t.Chdir(dir)
|
||||||
|
t.Setenv(databaseEnv, name)
|
||||||
|
|
||||||
|
assertRunsUseDatabase(t, dir, name)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// holdScanLock takes the lock on the database at path, as a running
|
// holdScanLock takes the lock on the database at path, as a running
|
||||||
// scan does, and holds it until the test ends. It fails the test when
|
// scan does, and holds it until the test ends. It fails the test when
|
||||||
// the lock is already held.
|
// the lock is already held.
|
||||||
|
|||||||
Reference in New Issue
Block a user