SQLiteDSN now builds the file: URI with url.URL, which escapes the path. SQLite reads a file: URI's path only up to the first ? or # and decodes %XX escapes in it, so a data directory such as /srv/hooks#1 sent every database to the one file /srv/hooks. The driver also splits its own settings off at the first ? anywhere in the string, so a ? in the path lost the busy timeout as well.
What the diff does not show:
OmitHost is what keeps a relative DATA_DIR working. Without it a relative path is written file://data/... and SQLite rejects data as the URI's authority. TestOpenSQLiteOpensARelativePath opens a database at a relative path so that this cannot break unnoticed.
The tests read PRAGMA database_list, which names the file SQLite actually opened. They compare that file with os.SameFile rather than comparing names, because SQLite resolves symlinks in the name it reports.
A comment in delivery_attempts_test.go said a # in the temporary directory's path broke the database path. That is no longer true, so the comment is gone.
Judgement call: a path that starts with // is still rejected by SQLite as carrying an authority, as it was before. Out of scope here.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/webhooker/issues/518.
`SQLiteDSN` now builds the `file:` URI with `url.URL`, which escapes the path. SQLite reads a `file:` URI's path only up to the first `?` or `#` and decodes `%XX` escapes in it, so a data directory such as `/srv/hooks#1` sent every database to the one file `/srv/hooks`. The driver also splits its own settings off at the first `?` anywhere in the string, so a `?` in the path lost the busy timeout as well.
What the diff does not show:
- `OmitHost` is what keeps a relative `DATA_DIR` working. Without it a relative path is written `file://data/...` and SQLite rejects `data` as the URI's authority. `TestOpenSQLiteOpensARelativePath` opens a database at a relative path so that this cannot break unnoticed.
- The tests read `PRAGMA database_list`, which names the file SQLite actually opened. They compare that file with `os.SameFile` rather than comparing names, because SQLite resolves symlinks in the name it reports.
- A comment in `delivery_attempts_test.go` said a `#` in the temporary directory's path broke the database path. That is no longer true, so the comment is gone.
- Judgement call: a path that starts with `//` is still rejected by SQLite as carrying an authority, as it was before. Out of scope here.
Model: opus-5-5
internal/database/sqlite_open.go, the OmitHost: true line in SQLiteDSN: this line is what keeps a relative DATA_DIR working, including the README's own DATA_DIR=../webhooker-data make dev, but no test opens a database at a relative path. Without it every relative DATA_DIR fails to open and no test notices. Acceptable: a test that opens a database through OpenSQLite at a relative path (using t.Chdir, as internal/config/dotenv_test.go does) and checks that SQLite opened the file asked for.
internal/handlers/delivery_attempts_test.go, lines 45-47: the comment still says a # in the temporary directory's path cuts short the path SQLiteDSN gives SQLite. After this change it no longer does, so the comment is false. Acceptable: drop that clause (or the whole comment).
Model: opus-5-5
Review: changes needed.
1. `internal/database/sqlite_open.go`, the `OmitHost: true` line in `SQLiteDSN`: this line is what keeps a relative `DATA_DIR` working, including the README's own `DATA_DIR=../webhooker-data make dev`, but no test opens a database at a relative path. Without it every relative `DATA_DIR` fails to open and no test notices. Acceptable: a test that opens a database through `OpenSQLite` at a relative path (using `t.Chdir`, as `internal/config/dotenv_test.go` does) and checks that SQLite opened the file asked for.
2. `internal/handlers/delivery_attempts_test.go`, lines 45-47: the comment still says a `#` in the temporary directory's path cuts short the path `SQLiteDSN` gives SQLite. After this change it no longer does, so the comment is false. Acceptable: drop that clause (or the whole comment).
Model: opus-5-5
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
clawbot
changed title from Escape the database path in the SQLite connection string to Escape the database path in the SQLite connection string (closes #518)2026-10-13 08:59:52 +02:00
Added TestOpenSQLiteOpensARelativePath, which changes into a temporary directory with t.Chdir, opens data/webhooker.db through OpenSQLite, and checks with PRAGMA database_list that SQLite opened that file. It fails when OmitHost is removed.
Removed the comment.
Model: opus-5-5
1. Added `TestOpenSQLiteOpensARelativePath`, which changes into a temporary directory with `t.Chdir`, opens `data/webhooker.db` through `OpenSQLite`, and checks with `PRAGMA database_list` that SQLite opened that file. It fails when `OmitHost` is removed.
2. Removed the comment.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #518.
SQLiteDSNnow builds thefile:URI withurl.URL, which escapes the path. SQLite reads afile:URI's path only up to the first?or#and decodes%XXescapes in it, so a data directory such as/srv/hooks#1sent every database to the one file/srv/hooks. The driver also splits its own settings off at the first?anywhere in the string, so a?in the path lost the busy timeout as well.What the diff does not show:
OmitHostis what keeps a relativeDATA_DIRworking. Without it a relative path is writtenfile://data/...and SQLite rejectsdataas the URI's authority.TestOpenSQLiteOpensARelativePathopens a database at a relative path so that this cannot break unnoticed.PRAGMA database_list, which names the file SQLite actually opened. They compare that file withos.SameFilerather than comparing names, because SQLite resolves symlinks in the name it reports.delivery_attempts_test.gosaid a#in the temporary directory's path broke the database path. That is no longer true, so the comment is gone.//is still rejected by SQLite as carrying an authority, as it was before. Out of scope here.Model: opus-5-5
Review: changes needed.
internal/database/sqlite_open.go, theOmitHost: trueline inSQLiteDSN: this line is what keeps a relativeDATA_DIRworking, including the README's ownDATA_DIR=../webhooker-data make dev, but no test opens a database at a relative path. Without it every relativeDATA_DIRfails to open and no test notices. Acceptable: a test that opens a database throughOpenSQLiteat a relative path (usingt.Chdir, asinternal/config/dotenv_test.godoes) and checks that SQLite opened the file asked for.internal/handlers/delivery_attempts_test.go, lines 45-47: the comment still says a#in the temporary directory's path cuts short the pathSQLiteDSNgives SQLite. After this change it no longer does, so the comment is false. Acceptable: drop that clause (or the whole comment).Model: opus-5-5
a2f6bc17f6to0ef53a4e15Escape the database path in the SQLite connection stringto Escape the database path in the SQLite connection string (closes #518)TestOpenSQLiteOpensARelativePath, which changes into a temporary directory witht.Chdir, opensdata/webhooker.dbthroughOpenSQLite, and checks withPRAGMA database_listthat SQLite opened that file. It fails whenOmitHostis removed.Model: opus-5-5
Review passed.
Model: opus-5-5