From a997ede32b563a94f2e3d0a6a70dd96fa2759624 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 00:43:20 +0000 Subject: [PATCH 1/6] Test the db_url derived from a state_dir holding %, ? and # The test sets state_dir to a directory named a%41b?c#d, opens the derived db_url with the SQLite driver, and checks that the database is state.sqlite3 in that directory, in WAL mode. It fails until the directory is escaped in the URL. Model: opus-5-5 --- .../config/config_validation_internal_test.go | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/internal/config/config_validation_internal_test.go b/internal/config/config_validation_internal_test.go index abfdee7..2e06fc9 100644 --- a/internal/config/config_validation_internal_test.go +++ b/internal/config/config_validation_internal_test.go @@ -135,6 +135,55 @@ 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) + + db, err := sql.Open("sqlite", c.DBURL) + if err != nil { + t.Fatalf("failed to open %q: %v", c.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", c.DBURL, err) + } + + if journalMode != "wal" { + t.Errorf("journal mode of %q = %q, want wal", c.DBURL, journalMode) + } + + dbFile := filepath.Join(stateDir, "state.sqlite3") + + _, err = os.Stat(dbFile) + if err != nil { + t.Errorf("opening %q did not create %q: %v", c.DBURL, dbFile, err) + } +} + func TestExplicitValidValuesAreUsed(t *testing.T) { t.Parallel() -- 2.54.0 From d104d5d936a7854e3ba60d6963213f0b41e76e54 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 00:45:38 +0000 Subject: [PATCH 2/6] Escape state_dir in the db_url derived from it (closes #290) The derived db_url put state_dir into a file: URL as it was, so SQLite decoded a %HH in the name and ended the path at a ? or #, and the journal mode parameter was lost. It is now built with net/url, which percent-encodes the directory name; SQLite decodes it back and opens state.sqlite3 in that directory. OmitHost keeps the usual form file:/var/lib/pixa/state.sqlite3 and a relative state_dir a path. Model: opus-5-5 --- README.md | 12 +++++++----- TODO.md | 6 ++++++ internal/config/config.go | 21 +++++++++++++++++++-- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index d9b3c86..17ceda1 100644 --- a/README.md +++ b/README.md @@ -725,11 +725,13 @@ 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`. 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..e8044db 100644 --- a/TODO.md +++ b/TODO.md @@ -62,6 +62,12 @@ P2: security: per-origin rate limiting another way, such as with `%2F` in the path, whatever else the path holds, or with a user name and password. It also answers 400 for a source path holding a `..` segment, as `/v1/image/` refuses one. +- 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`. - 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips build a huge image (closes #282): libvips' thumbnail scaled the whole source to cover the requested size and copied that into memory before it cut out the diff --git a/internal/config/config.go b/internal/config/config.go index ff4ac6c..d94fb74 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,24 @@ 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. +func defaultDBURL(stateDir string) string { + dbURL := url.URL{ + Scheme: "file", + OmitHost: true, + Path: stateDir + "/state.sqlite3", + 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 -- 2.54.0 From c593eae60315deaf95137f893743d0a4b7c9b632 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 01:18:18 +0000 Subject: [PATCH 3/6] Test the db_url derived from a state_dir starting with // The test sets state_dir to a temporary directory with a / put in front, opens the derived db_url with the SQLite driver, and checks that the database is state.sqlite3 in that directory, in WAL mode. It fails while the URL starts file://, where SQLite reads the first directory name as a host. The open-and-check part of the test before it moves into a helper both tests call. Model: opus-5-5 --- .../config/config_validation_internal_test.go | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/internal/config/config_validation_internal_test.go b/internal/config/config_validation_internal_test.go index 2e06fc9..3f4bc3c 100644 --- a/internal/config/config_validation_internal_test.go +++ b/internal/config/config_validation_internal_test.go @@ -158,9 +158,39 @@ func TestDerivedDBURLOpensStateDirHoldingURLCharacters(t *testing.T) { t.Logf("state_dir %q gives db_url %q", stateDir, c.DBURL) - db, err := sql.Open("sqlite", 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("failed to open %q: %v", c.DBURL, err) + 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")) +} + +// 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() }) @@ -169,18 +199,16 @@ func TestDerivedDBURLOpensStateDirHoldingURLCharacters(t *testing.T) { err = db.QueryRowContext(t.Context(), "PRAGMA journal_mode").Scan(&journalMode) if err != nil { - t.Fatalf("failed to read the journal mode of %q: %v", c.DBURL, err) + 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", c.DBURL, journalMode) + t.Errorf("journal mode of %q = %q, want wal", dbURL, journalMode) } - dbFile := filepath.Join(stateDir, "state.sqlite3") - _, err = os.Stat(dbFile) if err != nil { - t.Errorf("opening %q did not create %q: %v", c.DBURL, dbFile, err) + t.Errorf("opening %q did not create %q: %v", dbURL, dbFile, err) } } -- 2.54.0 From afe000fb0fb49a9011e4ea58cd056d84b56635a4 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 01:20:27 +0000 Subject: [PATCH 4/6] Keep the empty host for a state_dir starting with // With OmitHost set, a state_dir starting with // gave a db_url starting file://, and SQLite read the first directory name as a host, so pixa failed to start. OmitHost is now left off for such a directory: the URL starts file://// and SQLite reads an empty host followed by the path. Every other state_dir gives the same URL as before. Model: opus-5-5 --- README.md | 13 +++++++------ TODO.md | 4 +++- internal/config/config.go | 6 ++++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 17ceda1..8cbaa2c 100644 --- a/README.md +++ b/README.md @@ -726,12 +726,13 @@ Key settings in more detail: - `db_url` — the SQLite database to open; omitted, it is `file:/state.sqlite3?_pragma=journal_mode(WAL)`, which keeps the database in WAL mode. `state_dir` is percent-encoded in it, so a `%`, `?` or - `#` in the name is written as `%25`, `%3F` or `%23`. 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 + `#` in the name is written as `%25`, `%3F` or `%23`. For a `state_dir` + starting with `//` it starts `file:////`, so SQLite does not read the first + directory name 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 e8044db..96263bc 100644 --- a/TODO.md +++ b/TODO.md @@ -67,7 +67,9 @@ P2: security: per-origin rate limiting 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`. + characters as `%25`, `%3F` and `%23`. For a `state_dir` starting with `//` it + starts `file:////`, so SQLite no longer reads the first directory name as a + host. - 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips build a huge image (closes #282): libvips' thumbnail scaled the whole source to cover the requested size and copied that into memory before it cut out the diff --git a/internal/config/config.go b/internal/config/config.go index d94fb74..bc657ff 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -359,11 +359,13 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { // 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. +// as a host name. A stateDir starting with // keeps the empty host, so the +// URL starts file://// and SQLite does not read the first directory name +// as a host. func defaultDBURL(stateDir string) string { dbURL := url.URL{ Scheme: "file", - OmitHost: true, + OmitHost: !strings.HasPrefix(stateDir, "//"), Path: stateDir + "/state.sqlite3", RawQuery: "_pragma=journal_mode(WAL)", } -- 2.54.0 From 339c149a4e8224976a5b5116007a34a86a72ab9b Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 01:45:30 +0000 Subject: [PATCH 5/6] Test the db_url derived from a state_dir of / The path written into the URL is then //state.sqlite3, so the URL starts file:// and SQLite reads state.sqlite3 as a host. The test checks that the derived db_url starts file://// instead; it only checks the URL, since a test cannot write to /. Model: opus-5-5 --- .../config/config_validation_internal_test.go | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/internal/config/config_validation_internal_test.go b/internal/config/config_validation_internal_test.go index 3f4bc3c..1324050 100644 --- a/internal/config/config_validation_internal_test.go +++ b/internal/config/config_validation_internal_test.go @@ -182,6 +182,27 @@ func TestDerivedDBURLOpensStateDirStartingWithTwoSlashes(t *testing.T) { 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. -- 2.54.0 From 78fe6957de4259f09b83f789a02fefe727e4afd9 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 01:46:35 +0000 Subject: [PATCH 6/6] Keep the empty host when the db_url path starts with // defaultDBURL kept the empty host only when state_dir started with //, but SQLite reads the path written into the URL, state_dir followed by /state.sqlite3. For a state_dir of / that path is //state.sqlite3, the URL started file:// and SQLite read state.sqlite3 as a host, so pixa failed to start. The decision now looks at that path: a state_dir of / gives file:////state.sqlite3. Every other state_dir gives the same URL as before. Model: opus-5-5 --- README.md | 14 +++++++------- TODO.md | 16 ++++++++-------- internal/config/config.go | 14 +++++++++----- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 8cbaa2c..7a71a5d 100644 --- a/README.md +++ b/README.md @@ -726,13 +726,13 @@ Key settings in more detail: - `db_url` — the SQLite database to open; omitted, it is `file:/state.sqlite3?_pragma=journal_mode(WAL)`, which keeps the 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` - starting with `//` it starts `file:////`, so SQLite does not read the first - directory name 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 + `#` 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 96263bc..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 @@ -62,14 +70,6 @@ P2: security: per-origin rate limiting another way, such as with `%2F` in the path, whatever else the path holds, or with a user name and password. It also answers 400 for a source path holding a `..` segment, as `/v1/image/` refuses one. -- 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` starting with `//` it - starts `file:////`, so SQLite no longer reads the first directory name as a - host. - 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips build a huge image (closes #282): libvips' thumbnail scaled the whole source to cover the requested size and copied that into memory before it cut out the diff --git a/internal/config/config.go b/internal/config/config.go index bc657ff..40eb799 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -359,14 +359,18 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { // 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. A stateDir starting with // keeps the empty host, so the -// URL starts file://// and SQLite does not read the first directory name -// as a host. +// 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(stateDir, "//"), - Path: stateDir + "/state.sqlite3", + OmitHost: !strings.HasPrefix(dbPath, "//"), + Path: dbPath, RawQuery: "_pragma=journal_mode(WAL)", } -- 2.54.0