Skip to content

core/database: report a garbage header as "file is not a database" - #8500

Open
penberg wants to merge 1 commit into
mainfrom
notadb
Open

core/database: report a garbage header as "file is not a database"#8500
penberg wants to merge 1 commit into
mainfrom
notadb

Conversation

@penberg

@penberg penberg commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Opening a database file with an invalid header must return SQLITE_NOTADB
("file is not a database") to be compatible with SQLite. Turso used to
report whatever field it tripped over first (e.g. "invalid page size"),
which the sqlite3 compat layer then surfaced as "out of memory" in
corrupt8.test.

SQLite applies its "not a database" rules (bad magic, bad page size,
fewer than 480 usable bytes per page) in one place, btree.c lockBtree,
after zero-filling a file shorter than a page. Mirror that structure:
the header reader is dumb and hands back either the full header or the
bytes it got from a short file, and a single plain_header_page_info
owns every rule, gated on one predicate ("no cipher and no Turso
encryption prefix"). A short file that lacks the SQLite magic can never
be a database and is reported as NotADB; a short file that has the magic
is a truncated real database and keeps the short-read I/O error so
truncation stays loud instead of becoming SQLite's "empty database"
lenience.

A page codec may transform the magic bytes, so the codec path cannot
tell a garbage short file from a truncated codec database. It keeps
reporting the short read and leaves header judgement to the codec.

Tests: tests/integration/database.rs covers a garbage full header, a
short garbage file (NotADB) and a short file with the magic (short
read). core page_codec_reopen_reports_short_header_read still passes.

Opening a database file with an invalid header must return SQLITE_NOTADB
("file is not a database") to be compatible with SQLite. Turso used to
report whatever field it tripped over first (e.g. "invalid page size"),
which the sqlite3 compat layer then surfaced as "out of memory" in
corrupt8.test.

SQLite applies its "not a database" rules (bad magic, bad page size,
fewer than 480 usable bytes per page) in one place, btree.c lockBtree,
after zero-filling a file shorter than a page. Mirror that structure:
the header reader is dumb and hands back either the full header or the
bytes it got from a short file, and a single `plain_header_page_info`
owns every rule, gated on one predicate ("no cipher and no Turso
encryption prefix"). A short file that lacks the SQLite magic can never
be a database and is reported as NotADB; a short file that has the magic
is a truncated real database and keeps the short-read I/O error so
truncation stays loud instead of becoming SQLite's "empty database"
lenience.

A page codec may transform the magic bytes, so the codec path cannot
tell a garbage short file from a truncated codec database. It keeps
reporting the short read and leaves header judgement to the codec.

Tests: tests/integration/database.rs covers a garbage full header, a
short garbage file (NotADB) and a short file with the magic (short
read). core page_codec_reopen_reports_short_header_read still passes.

@kawacukennedy kawacukennedy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a careful parity fix. The refactor of read_db_header_buf to return DbHeaderRead::{Full, Short} cleanly defers the decision of what a short or truncated header "means" to the caller that actually knows the encryption context -- the right separation of concerns.

The plain_header_page_info judgment is the crux and it is handled precisely:

  • garbage full header -> NotADB (matches btree.c lockBtree's "file is not a database")
  • garbage page-size field and invalid reserved-space -> NotADB
  • short garbage file (no magic) -> NotADB (SQLite zero-fills then fails the magic check)
  • short file that DOES carry the SQLite magic -> surfaces the ShortRead I/O error loudly (a truncated real DB, not a lenient "empty database")
  • files with the Turso prefix, or where a cipher may be applied post-open, keep the detailed corruption errors rather than being flattened to NotADB

The judge_as_sqlite guard (CipherMode::None AND not TURSO_HEADER_PREFIX) is the key correctness subtlety: it ensures encrypted DBs never get the SQLite-only judgment even when no cipher mode is known at header-read time yet (a cipher set later via PRAGMA key). The regression tests cover both the plain garbage-header case, which is the root cause of the corrupt8 cascade misreporting "invalid page size" as "out of memory", and the short-file vs truncated-magic distinction. Solid and complete. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants