Commit 266b979e authored by Ian Chechin's avatar Ian Chechin
Browse files

sqlite: validate mattn-compat DSN keys and fix auto_vacuum apply order

Addresses the !134 review:

- Apply _auto_vacuum before the _pragma list and the other shorthand keys.
  auto_vacuum only takes effect while the database is new; a _journal_mode
  change materialises page 1 and locks it in, so the previous order made
  _journal_mode=wal&_auto_vacuum=1 silently resolve to auto_vacuum=0. The
  "Test combined DSN" case now includes _auto_vacuum and asserts it reads
  back as 1, proving the fixed order.
- Validate every shorthand value against the set github.com/mattn/go-sqlite3
  accepts and return an error on anything else, instead of silently ignoring
  it, so a _synchronous or _foreign_keys typo no longer downgrades durability
  or drops enforcement. This also removes the DSN-injection surface these keys
  had, since a value is now either a known token or an error.
- When a key and its alias are both present, the alias wins, matching mattn
  (_foreign_keys=off&_fk=on -> on).
- Document the apply order, precedence, accepted values, and that only _pragma
  values are executed verbatim and must be trusted.
- CHANGELOG entry for #134.
parent 97841221
Loading
Loading
Loading
Loading
+4 −0
Original line number Diff line number Diff line
# Changelog

 - 2026-07-20 v1.55.0:
     - Add `github.com/mattn/go-sqlite3`-compatible shorthand DSN query parameters to ease migration from that driver: `_busy_timeout`/`_timeout`, `_foreign_keys`/`_fk`, `_journal_mode`/`_journal`, `_synchronous`/`_sync`, `_auto_vacuum`/`_vacuum`, and `_query_only`, each setting the correspondingly named PRAGMA. Values are validated against the same set `mattn/go-sqlite3` accepts (case-insensitive); an unrecognized value now fails the connection with an error instead of being silently ignored, so a typo such as `_synchronous=fu1l` or `_foreign_keys=yes_please` is reported rather than silently downgrading durability or dropping foreign-key enforcement. The keys are applied in a fixed order independent of their order in the DSN — `_busy_timeout` and `_auto_vacuum` before any `_pragma` values (`auto_vacuum` must be set before the database is first written), the rest after, and `_query_only` last — and where a key and its alias are both supplied the alias wins, matching `mattn/go-sqlite3`. Behavior change to note: these keys were parsed but had no effect in prior releases, so a DSN carried over from a `mattn/go-sqlite3` setup that includes e.g. `_foreign_keys=on`, `_journal_mode=wal`, or `_query_only=1` now takes effect where it previously did nothing; review such DSNs before upgrading. `_pragma` and all other existing parameters are unchanged.
     - See [GitLab merge request #134](https://gitlab.com/cznic/sqlite/-/merge_requests/134), thanks Toni Spets (@beeper-hifi) and Ian Chechin!

 - 2026-07-15 v1.54.0:
     - Upgrade to [SQLite 3.53.3](https://sqlite.org/releaselog/3_53_3.html). This also bumps the pinned `modernc.org/libc` to v1.74.1; as always, downstream modules must pin the exact same `modernc.org/libc` version this module's `go.mod` pins (see [GitLab issue #177](https://gitlab.com/cznic/sqlite/-/issues/177)).
     - Under the opt-in `_texttotime` DSN parameter, best-effort parse date-shaped TEXT values from columns SQLite reports with an empty declared type — aggregates and expressions over a date column (`MAX(d)`, `COALESCE(d, ...)`, `upper(d)`, `d || ''`), subqueries, and typeless real columns (`CREATE TABLE t(x)`) — into `time.Time`, instead of delivering them as a raw string that `Scan` cannot store into a `*time.Time`. The existing declared `DATE`/`DATETIME`/`TIME`/`TIMESTAMP` path is unchanged; this only adds the empty-decltype case. The conversion is strictly best-effort: a value that does not parse as a time falls through to the original string, so no `Scan` that worked before can newly fail. `ColumnTypeScanType` continues to report `string` for empty-decltype columns, since the declared type cannot prove the column is temporal. Without `_texttotime` the behavior is byte-for-byte unchanged. Resolves [GitLab issue #248](https://gitlab.com/cznic/sqlite/-/issues/248).
+21 −11
Original line number Diff line number Diff line
@@ -54,17 +54,27 @@ func newDriver() *Driver { return d }
// information on supported PRAGMAs see: https://www.sqlite.org/pragma.html
//
// The following shorthand keys set common PRAGMAs for easier DSN compatibility
// when migrating from github.com/mattn/go-sqlite3. Each value is passed as-is to
// the corresponding PRAGMA and is not validated. They are applied in a fixed
// order (busy_timeout first, query_only last) so an order-sensitive combination
// works in a single DSN:
//
//	_busy_timeout, _timeout   -> PRAGMA busy_timeout
//	_foreign_keys, _fk        -> PRAGMA foreign_keys
//	_journal_mode, _journal   -> PRAGMA journal_mode
//	_synchronous, _sync       -> PRAGMA synchronous
//	_auto_vacuum, _vacuum     -> PRAGMA auto_vacuum
//	_query_only               -> PRAGMA query_only
// when migrating from github.com/mattn/go-sqlite3. Each value is validated
// against the same set github.com/mattn/go-sqlite3 accepts (case-insensitive);
// an unrecognized value fails the connection with an error instead of being
// silently ignored. The keys are applied in a fixed order, independent of the
// order they appear in the DSN: _busy_timeout and _auto_vacuum first (auto_vacuum
// must be set before the database is first written), then the _pragma values,
// then the remaining keys, and _query_only last. Where a shorthand key and a
// _pragma set the same PRAGMA, whichever is applied later in that order wins. If
// a key and its alias are both supplied, the alias (the second name below) wins,
// matching github.com/mattn/go-sqlite3. Accepted values:
//
//	_busy_timeout, _timeout   -> PRAGMA busy_timeout   (an integer)
//	_foreign_keys, _fk        -> PRAGMA foreign_keys   (0 1 false true no yes off on)
//	_journal_mode, _journal   -> PRAGMA journal_mode   (DELETE TRUNCATE PERSIST MEMORY WAL OFF)
//	_synchronous, _sync       -> PRAGMA synchronous    (0 OFF 1 NORMAL 2 FULL 3 EXTRA)
//	_auto_vacuum, _vacuum     -> PRAGMA auto_vacuum    (0 NONE 1 FULL 2 INCREMENTAL)
//	_query_only               -> PRAGMA query_only     (0 1 false true no yes off on)
//
// Unlike these validated shorthand keys, each _pragma value is executed verbatim
// (with PRAGMA prepended) and is not validated, so a DSN that includes _pragma
// must come from a trusted source.
//
// _time_format: The name of a format to use when writing time values to the database.
// The currently supported values are (1) "sqlite" for YYYY-MM-DD HH:MM:SS.SSS[+-]HH:MM
+56 −23
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ import (
	"database/sql"
	"fmt"
	"path/filepath"
	"strings"
	"testing"
)

@@ -41,6 +42,27 @@ func TestDSNPragmas(t *testing.T) {
		}
	}

	// verifyPragmaError asserts that opening a connection with the given DSN
	// query fails with an error containing wantSubstr. applyQueryParams runs when
	// the connection is established, so the error surfaces on first use.
	verifyPragmaError := func(t *testing.T, query, wantSubstr string) {
		dbPath := fmt.Sprintf("%s?%s", filepath.Join(t.TempDir(), "tmp.db"), query)
		db, err := sql.Open("sqlite", dbPath)
		if err != nil {
			t.Fatal(err)
		}
		defer db.Close()

		var x int
		err = db.QueryRow("SELECT 1").Scan(&x)
		if err == nil {
			t.Fatalf("query %q: expected an error, got none", query)
		}
		if !strings.Contains(err.Error(), wantSubstr) {
			t.Fatalf("query %q: error %q does not contain %q", query, err.Error(), wantSubstr)
		}
	}

	t.Run("Test _pragma", func(t *testing.T) {
		verifyPragma(t, "", "foreign_keys", "0", "busy_timeout", "0")
		verifyPragma(t, "_pragma=foreign_keys=on", "foreign_keys", "1")
@@ -66,9 +88,13 @@ func TestDSNPragmas(t *testing.T) {
		verifyPragma(t, "_fk=off", "foreign_keys", "0")
		verifyPragma(t, "_fk=false", "foreign_keys", "0")

		// invalid settings should keep the default value
		verifyPragma(t, "_foreign_keys=x", "foreign_keys", "0")
		verifyPragma(t, "_fk=x", "foreign_keys", "0")
		// invalid settings are a hard error (mattn-compatible)
		verifyPragmaError(t, "_foreign_keys=x", "invalid _foreign_keys")
		verifyPragmaError(t, "_fk=x", "invalid _fk")

		// when a key and its alias disagree, the alias wins (mattn-compatible)
		verifyPragma(t, "_foreign_keys=off&_fk=on", "foreign_keys", "1")
		verifyPragma(t, "_foreign_keys=on&_fk=off", "foreign_keys", "0")
	})

	t.Run("Test _busy_timeout | _timeout", func(t *testing.T) {
@@ -76,11 +102,14 @@ func TestDSNPragmas(t *testing.T) {
		verifyPragma(t, "_timeout=5000", "busy_timeout", "5000")
		verifyPragma(t, "_busy_timeout=5000", "busy_timeout", "5000")

		// invalid timeouts should keep the default value
		// a negative integer is valid input; SQLite clamps it to 0
		verifyPragma(t, "_busy_timeout=-1", "busy_timeout", "0")
		verifyPragma(t, "_busy_timeout=true", "busy_timeout", "0")
		verifyPragma(t, "_busy_timeout=on", "busy_timeout", "0")
		verifyPragma(t, "_busy_timeout=x", "busy_timeout", "0")

		// non-integer values are a hard error (mattn-compatible)
		verifyPragmaError(t, "_busy_timeout=true", "invalid _busy_timeout")
		verifyPragmaError(t, "_busy_timeout=on", "invalid _busy_timeout")
		verifyPragmaError(t, "_busy_timeout=x", "invalid _busy_timeout")
		verifyPragmaError(t, "_timeout=x", "invalid _timeout")
	})

	t.Run("Test _journal_mode | _journal", func(t *testing.T) {
@@ -88,8 +117,9 @@ func TestDSNPragmas(t *testing.T) {
		verifyPragma(t, "_journal_mode=wal", "journal_mode", "wal")
		verifyPragma(t, "_journal=wal", "journal_mode", "wal")

		// invalid journal mode should keep the default value
		verifyPragma(t, "_journal=x", "journal_mode", "delete")
		// invalid journal mode is a hard error (mattn-compatible)
		verifyPragmaError(t, "_journal=x", "invalid _journal")
		verifyPragmaError(t, "_journal_mode=WAL2", "invalid _journal_mode")
	})

	t.Run("Test _synchronous | _sync", func(t *testing.T) {
@@ -114,9 +144,9 @@ func TestDSNPragmas(t *testing.T) {
		verifyPragma(t, "_sync=3", "synchronous", "3")
		verifyPragma(t, "_sync=EXTRA", "synchronous", "3")

		// invalid journal mode selects NORMAL
		verifyPragma(t, "_synchronous=x", "synchronous", "1")
		verifyPragma(t, "_sync=x", "synchronous", "1")
		// invalid synchronous mode is a hard error (mattn-compatible)
		verifyPragmaError(t, "_synchronous=x", "invalid _synchronous")
		verifyPragmaError(t, "_sync=x", "invalid _sync")
	})

	t.Run("Test _auto_vacuum | _vacuum", func(t *testing.T) {
@@ -131,9 +161,9 @@ func TestDSNPragmas(t *testing.T) {
		verifyPragma(t, "_vacuum=1", "auto_vacuum", "1")
		verifyPragma(t, "_vacuum=2", "auto_vacuum", "2")

		// invalid keeps it disabled
		verifyPragma(t, "_auto_vacuum=x", "auto_vacuum", "0")
		verifyPragma(t, "_vacuum=x", "auto_vacuum", "0")
		// invalid auto_vacuum is a hard error (mattn-compatible)
		verifyPragmaError(t, "_auto_vacuum=x", "invalid _auto_vacuum")
		verifyPragmaError(t, "_vacuum=x", "invalid _vacuum")
	})

	t.Run("Test _query_only", func(t *testing.T) {
@@ -142,18 +172,21 @@ func TestDSNPragmas(t *testing.T) {

		verifyPragma(t, "_query_only=0", "query_only", "0")

		// invalid keeps it disabled
		verifyPragma(t, "_query_only=x", "query_only", "0")
		// invalid query_only is a hard error (mattn-compatible)
		verifyPragmaError(t, "_query_only=x", "invalid _query_only")
	})

	t.Run("Test combined DSN", func(t *testing.T) {
		// The mattn-compat keys must coexist in a single DSN. busy_timeout is
		// applied first and query_only last (see applyQueryParams); the DSN lists
		// them in a different order on purpose, so this also confirms the apply
		// order is fixed by the driver, not by the order the keys appear in the
		// DSN.
		verifyPragma(t, "_query_only=1&_synchronous=NORMAL&_journal_mode=wal&_foreign_keys=on&_busy_timeout=5000",
		// The mattn-compat keys must coexist in a single DSN. busy_timeout and
		// auto_vacuum are applied before the rest and query_only last (see
		// applyQueryParams); the DSN lists them in a different order on purpose,
		// so this also confirms the apply order is fixed by the driver, not by
		// the order the keys appear in the DSN. _auto_vacuum is included
		// deliberately: it only takes effect if applied before _journal_mode
		// materialises page 1, so reading it back as 1 proves the fixed order.
		verifyPragma(t, "_query_only=1&_synchronous=NORMAL&_journal_mode=wal&_auto_vacuum=1&_foreign_keys=on&_busy_timeout=5000",
			"busy_timeout", "5000",
			"auto_vacuum", "1",
			"foreign_keys", "1",
			"journal_mode", "wal",
			"synchronous", "1",
+73 −54
Original line number Diff line number Diff line
@@ -204,26 +204,74 @@ func getErrorRcMode(query string) (bool, error) {
	return on, nil
}

// dsnPick returns the value and key name for a mattn-compatible shorthand DSN
// parameter and its alias. When both are present the alias wins, matching
// github.com/mattn/go-sqlite3. An empty value counts as absent.
func dsnPick(q url.Values, primary, alias string) (key, val string) {
	if v := q.Get(primary); v != "" {
		key, val = primary, v
	}
	if v := q.Get(alias); v != "" {
		key, val = alias, v
	}
	return key, val
}

// dsnBool reports an error unless val is a mattn-compatible boolean DSN value.
func dsnBool(key, val string) error {
	switch strings.ToLower(val) {
	case "0", "no", "false", "off", "1", "yes", "true", "on":
		return nil
	}
	return fmt.Errorf("invalid %s %q, expecting one of: 0 1 false true no yes off on", key, val)
}

// dsnEnum reports an error unless val matches one of allowed, case-insensitively.
func dsnEnum(key, val string, allowed []string) error {
	for _, a := range allowed {
		if strings.EqualFold(val, a) {
			return nil
		}
	}
	return fmt.Errorf("invalid %s %q, expecting one of: %s", key, val, strings.Join(allowed, " "))
}

func applyQueryParams(c *conn, query string) error {
	q, err := url.ParseQuery(query)
	if err != nil {
		return err
	}

	var busyTimeout string
	if v := q.Get("_busy_timeout"); v != "" {
		busyTimeout = v
	} else if v := q.Get("_timeout"); v != "" {
		busyTimeout = v
	// mattn-compatible shorthand PRAGMA keys. Each value is validated against the
	// same set github.com/mattn/go-sqlite3 accepts (case-insensitive); an
	// unrecognized value is a hard error rather than a silent no-op. When a key
	// and its alias are both present the alias wins, matching mattn. See the
	// Driver documentation in driver.go for the full apply order and precedence.
	busyKey, busyTimeout := dsnPick(q, "_busy_timeout", "_timeout")
	if busyTimeout != "" {
		if _, err := strconv.ParseInt(busyTimeout, 10, 64); err != nil {
			return fmt.Errorf("invalid %s %q: %w", busyKey, busyTimeout, err)
		}
	}

	// Busy timeout must be one of the first PRAGMAs set from query params as some
	// that make changes to the database might otherwise unexpectedly fail with
	// SQLITE_BUSY.
	if busyTimeout != "" {
		cmd := "pragma busy_timeout = " + busyTimeout
		_, err := c.exec(context.Background(), cmd, nil)
		if err != nil {
		if _, err := c.exec(context.Background(), "pragma busy_timeout = "+busyTimeout, nil); err != nil {
			return err
		}
	}

	// auto_vacuum must be applied while the database is still new: a journal_mode
	// change or the first table materialises page 1 and locks the setting in, so
	// it runs before the _pragma list and the other shorthand keys.
	autoVacuumKey, autoVacuum := dsnPick(q, "_auto_vacuum", "_vacuum")
	if autoVacuum != "" {
		if err := dsnEnum(autoVacuumKey, autoVacuum, []string{"0", "NONE", "1", "FULL", "2", "INCREMENTAL"}); err != nil {
			return err
		}
		if _, err := c.exec(context.Background(), "pragma auto_vacuum = "+autoVacuum, nil); err != nil {
			return err
		}
	}
@@ -307,62 +355,32 @@ func applyQueryParams(c *conn, query string) error {
		c.textToTime = onoff
	}

	var foreignKeys string
	if v := q.Get("_foreign_keys"); v != "" {
		foreignKeys = v
	} else if v := q.Get("_fk"); v != "" {
		foreignKeys = v
	}

	foreignKeysKey, foreignKeys := dsnPick(q, "_foreign_keys", "_fk")
	if foreignKeys != "" {
		cmd := "pragma foreign_keys = " + foreignKeys
		_, err := c.exec(context.Background(), cmd, nil)
		if err != nil {
		if err := dsnBool(foreignKeysKey, foreignKeys); err != nil {
			return err
		}
		if _, err := c.exec(context.Background(), "pragma foreign_keys = "+foreignKeys, nil); err != nil {
			return err
		}

	var journalMode string
	if v := q.Get("_journal_mode"); v != "" {
		journalMode = v
	} else if v := q.Get("_journal"); v != "" {
		journalMode = v
	}

	journalModeKey, journalMode := dsnPick(q, "_journal_mode", "_journal")
	if journalMode != "" {
		cmd := "pragma journal_mode = " + journalMode
		_, err := c.exec(context.Background(), cmd, nil)
		if err != nil {
		if err := dsnEnum(journalModeKey, journalMode, []string{"DELETE", "TRUNCATE", "PERSIST", "MEMORY", "WAL", "OFF"}); err != nil {
			return err
		}
		if _, err := c.exec(context.Background(), "pragma journal_mode = "+journalMode, nil); err != nil {
			return err
		}

	var synchronous string
	if v := q.Get("_synchronous"); v != "" {
		synchronous = v
	} else if v := q.Get("_sync"); v != "" {
		synchronous = v
	}

	synchronousKey, synchronous := dsnPick(q, "_synchronous", "_sync")
	if synchronous != "" {
		cmd := "pragma synchronous = " + synchronous
		_, err := c.exec(context.Background(), cmd, nil)
		if err != nil {
		if err := dsnEnum(synchronousKey, synchronous, []string{"0", "OFF", "1", "NORMAL", "2", "FULL", "3", "EXTRA"}); err != nil {
			return err
		}
	}

	var autoVacuum string
	if v := q.Get("_auto_vacuum"); v != "" {
		autoVacuum = v
	} else if v := q.Get("_vacuum"); v != "" {
		autoVacuum = v
	}

	if autoVacuum != "" {
		cmd := "pragma auto_vacuum = " + autoVacuum
		_, err := c.exec(context.Background(), cmd, nil)
		if err != nil {
		if _, err := c.exec(context.Background(), "pragma synchronous = "+synchronous, nil); err != nil {
			return err
		}
	}
@@ -370,9 +388,10 @@ func applyQueryParams(c *conn, query string) error {
	// query_only is applied last: it makes the connection read-only, so it must
	// not precede the write-capable pragmas above (notably auto_vacuum).
	if v := q.Get("_query_only"); v != "" {
		cmd := "pragma query_only = " + v
		_, err := c.exec(context.Background(), cmd, nil)
		if err != nil {
		if err := dsnBool("_query_only", v); err != nil {
			return err
		}
		if _, err := c.exec(context.Background(), "pragma query_only = "+v, nil); err != nil {
			return err
		}
	}