Escape the database path in the SQLite connection string (closes #518)
check / check (push) Waiting to run
check / check (push) Waiting to run
SQLite reads a file: URI's path only up to the first ? or #, and decodes %XX escapes in it. With a data directory such as /srv/hooks#1, every database opened the one file /srv/hooks. The connection string is now built with url.URL, which escapes the path, and OmitHost keeps a relative path relative. Tests open databases in a directory whose name holds #, ? and %, and at a relative path, and check through PRAGMA database_list that each opened the file asked for. A handlers test comment that blamed # in the temporary directory's path is dropped, as that is no longer true. Model: opus-5-5
This commit was merged in pull request #523.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -2,6 +2,8 @@ package database_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -112,6 +114,79 @@ 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()
|
||||
|
||||
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()) })
|
||||
|
||||
assertOpenedFile(t, sqlDB, path)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOpenSQLiteOpensARelativePath opens a database at a path relative
|
||||
// to the working directory, which is what a relative DATA_DIR such as
|
||||
// the README's DATA_DIR=../webhooker-data gives OpenSQLite.
|
||||
//
|
||||
//nolint:paralleltest // t.Chdir moves the whole process.
|
||||
func TestOpenSQLiteOpensARelativePath(t *testing.T) {
|
||||
t.Chdir(t.TempDir())
|
||||
require.NoError(t, os.Mkdir("data", 0o700))
|
||||
|
||||
path := filepath.Join("data", database.MainDBFileName)
|
||||
|
||||
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
||||
require.NoError(t, err)
|
||||
|
||||
defer func() { require.NoError(t, sqlDB.Close()) }()
|
||||
|
||||
assertOpenedFile(t, sqlDB, path)
|
||||
}
|
||||
|
||||
// assertOpenedFile checks that sqlDB is the SQLite file at path.
|
||||
// database_list names the file SQLite actually opened, as an absolute
|
||||
// path with symlinks resolved, so the files are compared, not the names.
|
||||
func assertOpenedFile(t *testing.T, sqlDB *sql.DB, path string) {
|
||||
t.Helper()
|
||||
|
||||
var (
|
||||
seq int
|
||||
schema, file string
|
||||
)
|
||||
|
||||
require.NoError(t, sqlDB.
|
||||
QueryRowContext(t.Context(), "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
|
||||
|
||||
Reference in New Issue
Block a user