From 722675f1533d586f8534d209b8c615da258d9a45 Mon Sep 17 00:00:00 2001
From: clawbot <35+clawbot@noreply.example.org>
Date: Sun, 4 Oct 2026 15:13:19 +0200
Subject: [PATCH] Escape the database path in the SQLite connection string
 (closes #55)

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
---
 README.md    |  4 +++-
 TODO.md      |  3 +++
 db.go        | 15 +++++++++++-
 main_test.go | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index 4e4ac42..98d7024 100644
--- a/README.md
+++ b/README.md
@@ -283,7 +283,9 @@ All three subcommands operate on a single SQLite database file:
 
 -   Location: the value of the `SFDUPES_DATABASE` environment variable
     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
     use. `report` and `trees` require an existing database; a missing
     database file is a fatal error (exit 1) telling the user to run
diff --git a/TODO.md b/TODO.md
index b465821..d146d28 100644
--- a/TODO.md
+++ b/TODO.md
@@ -29,6 +29,9 @@
 
 # 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
   running single-threaded (2026-10-04,
   https://git.eeqj.de/sneak/sfdupes/issues/10)
diff --git a/db.go b/db.go
index c2ec0a3..ca42037 100644
--- a/db.go
+++ b/db.go
@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
+	"net/url"
 	"os"
 	"path/filepath"
 	"slices"
@@ -107,7 +108,19 @@ const reportParams = "mode=ro" +
 // openDB opens the SQLite database at path with the connection
 // parameters params. It does not create or verify the schema.
 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 {
 		return nil, fmt.Errorf("open database %s: %w", path, err)
 	}
diff --git a/main_test.go b/main_test.go
index 8a09878..bf4150d 100644
--- a/main_test.go
+++ b/main_test.go
@@ -8,6 +8,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"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
 // scan does, and holds it until the test ends. It fails the test when
 // the lock is already held.