diff --git a/internal/database/sqlite_open.go b/internal/database/sqlite_open.go index c162a82..e73ce57 100644 --- a/internal/database/sqlite_open.go +++ b/internal/database/sqlite_open.go @@ -242,7 +242,19 @@ func SQLiteDSN(path, mode string) string { ) q.Add("_pragma", "journal_mode(WAL)") - return "file:" + path + "?" + q.Encode() + // SQLite reads a file: URI's path only up to the first ? or #, and + // decodes %XX escapes in it, so url.URL escapes the path. OmitHost + // keeps a relative path as "file:data/x.db"; written + // "file://data/x.db", SQLite would reject "data" as the URI's + // authority. + dsn := url.URL{ + Scheme: "file", + Path: path, + OmitHost: true, + RawQuery: q.Encode(), + } + + return dsn.String() } // OpenSQLite opens the SQLite file at path with the service's diff --git a/internal/database/sqlite_open_test.go b/internal/database/sqlite_open_test.go index 448507c..1111c7c 100644 --- a/internal/database/sqlite_open_test.go +++ b/internal/database/sqlite_open_test.go @@ -2,6 +2,7 @@ package database_test import ( "context" + "os" "path/filepath" "strings" "testing" @@ -112,6 +113,54 @@ func TestMainDBAppliesPragmasOnALiveHandle(t *testing.T) { assert.Equal(t, "10000", busy) } +// TestOpenSQLiteOpensThePathAskedFor uses a data directory whose name +// holds ?, # and %. SQLite reads a file: URI's path only up to a ? or +// #, and decodes %41 to A, so an unescaped path sent every database in +// that directory to one file outside it. +func TestOpenSQLiteOpensThePathAskedFor(t *testing.T) { + t.Parallel() + + ctx := context.Background() + dir := filepath.Join(t.TempDir(), "hooks#1?x%41") + require.NoError(t, os.Mkdir(dir, 0o700)) + + for _, name := range []string{ + database.MainDBFileName, + "events-" + uuid.New().String() + ".db", + } { + path := filepath.Join(dir, name) + + sqlDB, err := database.OpenSQLite( + path, database.SQLiteModeCreate, + ) + require.NoError(t, err) + + t.Cleanup(func() { assert.NoError(t, sqlDB.Close()) }) + + // database_list names the file SQLite actually opened, with + // symlinks resolved, so the files are compared, not the names. + var ( + seq int + schema, file string + ) + + require.NoError(t, sqlDB. + QueryRowContext(ctx, "pragma database_list"). + Scan(&seq, &schema, &file)) + + asked, err := os.Stat(path) + require.NoError(t, err) + + opened, err := os.Stat(file) + require.NoError(t, err) + + assert.True( + t, os.SameFile(asked, opened), + "asked for %s, SQLite opened %s", path, file, + ) + } +} + // TestConcurrentReaderDoesNotBlockWrites is the unit-scale form of the // reproduction in // https://git.eeqj.de/sneak/webhooker/issues/256: an operator's