From efa431ba187fce1e5aa41de17363f7d583387494 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 16 Oct 2026 04:37:54 +0200 Subject: [PATCH] Escape state_dir in the db_url derived from it (closes #290) When db_url is omitted, pixa derives it from state_dir, and it put the directory name into a file: URL as it was, so a state_dir holding %, ? or # opened another file and lost the journal mode. The derived URL is now built with net/url, which percent-encodes the directory name; an ordinary path gives the same URL as before. When the path written into the URL starts with //, as for a state_dir of / or one starting with //, the empty host is kept so SQLite does not read the first name in the path as a host. Model: opus-5-5 --- README.md | 13 ++- TODO.md | 8 ++ internal/config/config.go | 27 ++++- .../config/config_validation_internal_test.go | 98 +++++++++++++++++++ 4 files changed, 139 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index d9b3c86..7a71a5d 100644 --- a/README.md +++ b/README.md @@ -725,11 +725,14 @@ Key settings in more detail: - `signing_key` — HMAC secret for URL signatures - `db_url` — the SQLite database to open; omitted, it is `file:/state.sqlite3?_pragma=journal_mode(WAL)`, which keeps the - database in WAL mode. pixa opens one connection to it, so its own reads and - writes run one at a time. pixa adds `_pragma=busy_timeout(5000)` to any - `db_url`, so a write that finds another program writing to the file waits up - to five seconds for it instead of failing. WAL mode comes only from the URL: - keep `_pragma=journal_mode(WAL)` in one you set + database in WAL mode. `state_dir` is percent-encoded in it, so a `%`, `?` or + `#` in the name is written as `%25`, `%3F` or `%23`. For a `state_dir` of `/` + or one starting with `//` it starts `file:////`, so SQLite does not read the + first name in the path as a host. pixa opens one connection to it, so its own + reads and writes run one at a time. pixa adds `_pragma=busy_timeout(5000)` to + any `db_url`, so a write that finds another program writing to the file waits + up to five seconds for it instead of failing. WAL mode comes only from the + URL: keep `_pragma=journal_mode(WAL)` in one you set - `cache_max_bytes` — disk cache size limit in bytes; `0` disables the disk cache entirely; omitted defaults to 75% of the sum of the free space on the filesystem containing `/cache/` and the bytes of source and diff --git a/TODO.md b/TODO.md index a00b1ef..da1345b 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,14 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-16 a `state_dir` whose name holds `%`, `?` or `#` keeps its database + at `state.sqlite3` in that directory (closes #290): the `db_url` derived from + it put the name into a `file:` URL as it was, so SQLite read `%41` as `A`, a + `?` or `#` ended the path early, and the journal mode parameter was lost. The + derived `db_url` is now built with Go's `net/url`, which writes those + characters as `%25`, `%3F` and `%23`. For a `state_dir` of `/` or one starting + with `//` it starts `file:////`, so SQLite no longer reads the first name in + the path as a host. - 2026-10-16 a size with one side `0` keeps the side computed from the source's proportions at 1 or more (closes #283): it was rounded down, so for a thin source it came out at 0, and libvips then made the size square. An 8192x1 diff --git a/internal/config/config.go b/internal/config/config.go index ff4ac6c..40eb799 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -338,8 +338,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { settingName(keyDBURL), errValueEmpty) } - // The driver sets the journal mode only through a _pragma parameter. - c.DBURL = fmt.Sprintf("file:%s/state.sqlite3?_pragma=journal_mode(WAL)", c.StateDir) + c.DBURL = defaultDBURL(c.StateDir) } if loader.err != nil { @@ -354,6 +353,30 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { return c, nil } +// defaultDBURL returns the db_url used when db_url is omitted: the file +// state.sqlite3 in stateDir, in WAL mode, which the driver sets only +// through a _pragma parameter. String() percent-encodes any %, ? or # in +// the directory name, which SQLite decodes back, so the URL names exactly +// this file. OmitHost writes file:/var/lib/pixa/... instead of +// file:///var/lib/pixa/..., and keeps a relative stateDir from being read +// as a host name. The path written into the URL is stateDir followed by +// /state.sqlite3; when it starts with //, as it does for a stateDir of / +// or one starting with //, the empty host is kept, so the URL starts +// file://// and SQLite does not read the first name in the path as a +// host. +func defaultDBURL(stateDir string) string { + dbPath := stateDir + "/state.sqlite3" + + dbURL := url.URL{ + Scheme: "file", + OmitHost: !strings.HasPrefix(dbPath, "//"), + Path: dbPath, + RawQuery: "_pragma=journal_mode(WAL)", + } + + return dbURL.String() +} + // validateKnownKeys rejects configuration files containing keys the // application does not understand, so typos fail at startup instead of // being silently ignored, and rejects keys that are explicitly set to diff --git a/internal/config/config_validation_internal_test.go b/internal/config/config_validation_internal_test.go index abfdee7..1324050 100644 --- a/internal/config/config_validation_internal_test.go +++ b/internal/config/config_validation_internal_test.go @@ -135,6 +135,104 @@ func TestDefaultDBURLOpensTheDatabaseInWALMode(t *testing.T) { } } +// TestDerivedDBURLOpensStateDirHoldingURLCharacters opens the db_url +// derived from a state_dir whose name holds %41, ? and #, and checks that +// the database is state.sqlite3 in that directory, in WAL mode. Put into +// a file: URL as it is, the name would lose its meaning: %41 is read as +// A, and the path ends at the ?, which also loses the journal mode. +func TestDerivedDBURLOpensStateDirHoldingURLCharacters(t *testing.T) { + t.Parallel() + + stateDir := filepath.Join(t.TempDir(), "a%41b?c#d") + + err := os.Mkdir(stateDir, 0o750) + if err != nil { + t.Fatalf("failed to create %q: %v", stateDir, err) + } + + // Quoted so YAML takes the whole name as it is. + c, err := configFromYAML(t, signingKeyLine+"state_dir: '"+stateDir+"'\n") + if err != nil { + t.Fatalf("config with state_dir %q should be valid, got: %v", stateDir, err) + } + + t.Logf("state_dir %q gives db_url %q", stateDir, c.DBURL) + + checkDBURLOpensFileInWALMode(t, c.DBURL, filepath.Join(stateDir, "state.sqlite3")) +} + +// TestDerivedDBURLOpensStateDirStartingWithTwoSlashes opens the db_url +// derived from a state_dir that starts with //, and checks that the +// database is state.sqlite3 in that directory, in WAL mode. Written right +// after file:, such a name gives a URL starting file://, and SQLite reads +// the first directory name as a host and refuses to open it. +func TestDerivedDBURLOpensStateDirStartingWithTwoSlashes(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + stateDir := "/" + dir + + c, err := configFromYAML(t, signingKeyLine+"state_dir: "+stateDir+"\n") + if err != nil { + t.Fatalf("config with state_dir %q should be valid, got: %v", stateDir, err) + } + + t.Logf("state_dir %q gives db_url %q", stateDir, c.DBURL) + + checkDBURLOpensFileInWALMode(t, c.DBURL, filepath.Join(dir, "state.sqlite3")) +} + +// TestDerivedDBURLForRootStateDir checks the db_url derived from a +// state_dir of /. The path written into the URL is then +// //state.sqlite3, and written right after file: it gives a URL starting +// file://, where SQLite reads state.sqlite3 as a host and refuses to open +// it. The test only checks the URL, since it cannot write to /. +func TestDerivedDBURLForRootStateDir(t *testing.T) { + t.Parallel() + + c, err := configFromYAML(t, signingKeyLine+"state_dir: /\n") + if err != nil { + t.Fatalf("config with state_dir / should be valid, got: %v", err) + } + + t.Logf("state_dir / gives db_url %q", c.DBURL) + + wantDBURL := "file:////state.sqlite3?_pragma=journal_mode(WAL)" + if c.DBURL != wantDBURL { + t.Errorf("DBURL = %q, want %q", c.DBURL, wantDBURL) + } +} + +// checkDBURLOpensFileInWALMode opens dbURL with the SQLite driver pixad +// uses, and checks that the database is in WAL mode and is the file +// dbFile. +func checkDBURLOpensFileInWALMode(t *testing.T, dbURL, dbFile string) { + t.Helper() + + db, err := sql.Open("sqlite", dbURL) + if err != nil { + t.Fatalf("failed to open %q: %v", dbURL, err) + } + + t.Cleanup(func() { _ = db.Close() }) + + var journalMode string + + err = db.QueryRowContext(t.Context(), "PRAGMA journal_mode").Scan(&journalMode) + if err != nil { + t.Fatalf("failed to read the journal mode of %q: %v", dbURL, err) + } + + if journalMode != "wal" { + t.Errorf("journal mode of %q = %q, want wal", dbURL, journalMode) + } + + _, err = os.Stat(dbFile) + if err != nil { + t.Errorf("opening %q did not create %q: %v", dbURL, dbFile, err) + } +} + func TestExplicitValidValuesAreUsed(t *testing.T) { t.Parallel()