Commit 581eb450 authored by cznic's avatar cznic
Browse files

sqlite: validate the connector dsn with getVFSName, not a bare ParseQuery

NewConnector parsed the query string itself to reject a malformed dsn early.
getVFSName is what newConn calls for the same purpose, and it does strictly
more: the same url.ParseQuery, plus a check for conflicting vfs parameters.
Calling it instead drops an import, removes the repeated parse, and keeps what
NewConnector rejects aligned with what opening a connection rejects by
construction rather than by matching two call sites by hand.

The eager contract widens accordingly: "file:x?vfs=a&vfs=b" is now reported by
NewConnector rather than by the first Connect. It was already an error either
way, so no dsn changes from accepted to rejected. Everything a connection must
exist to check - unknown parameters, out-of-range values - is still reported by
Connect, as documented.

v1.56.0 is not tagged, so no release shipped the narrower behavior.

Co-Authored-By: default avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
parent e7a39d2d
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
# Changelog

 - 2026-07-30 v1.56.0:
     - Add `NewConnector`, returning a `database/sql/driver.Connector` for use with `sql.OpenDB`. It opens the same connections `sql.Open("sqlite", dsn)` does, from the same registered driver, so every function, collation, connection hook and virtual table module registered through this package applies to them. It exists for callers that need to interpose on the physical connections `database/sql` opens — tracing, metrics, connection-scoped setup — which `sql.Open` gives no access to: such a caller can embed the returned `Connector`, override `Connect`, and pass its own wrapper to `sql.OpenDB`. Previously the only way to reach the registered driver was the `db, _ := sql.Open("sqlite", ""); drv := db.Driver(); db.Close()` idiom, which works only because `sql.Open` does not connect and this driver does not implement `driver.DriverContext`; and the only way to get a wrapper into a `*sql.DB` was `sql.Register`, which is process-global, panics on a name it has already seen, and cannot be undone, so a library had to invent a unique driver name per configuration. `sql.OpenDB` registers nothing. Constructing a `&sqlite.Driver{}` is not an alternative — its fields are unexported, so it carries none of the registrations. Only the syntax of the DSN query string is checked by `NewConnector`; parameter values continue to be validated when the connection is opened, so an unknown parameter or an out-of-range value is reported by `Connect` rather than at construction. Nothing about the existing `sql.Open` path changes: `*Driver` deliberately still does not implement `driver.DriverContext`, so `sql.Open` remains lazy and DSN errors continue to surface where they always have. A runnable sample is in `examples/connector`. Resolves [GitLab issue #253](https://gitlab.com/cznic/sqlite/-/issues/253), thanks Alessandro Segala (@ItalyPaleAle)!
     - Add `NewConnector`, returning a `database/sql/driver.Connector` for use with `sql.OpenDB`. It opens the same connections `sql.Open("sqlite", dsn)` does, from the same registered driver, so every function, collation, connection hook and virtual table module registered through this package applies to them. It exists for callers that need to interpose on the physical connections `database/sql` opens — tracing, metrics, connection-scoped setup — which `sql.Open` gives no access to: such a caller can embed the returned `Connector`, override `Connect`, and pass its own wrapper to `sql.OpenDB`. Previously the only way to reach the registered driver was the `db, _ := sql.Open("sqlite", ""); drv := db.Driver(); db.Close()` idiom, which works only because `sql.Open` does not connect and this driver does not implement `driver.DriverContext`; and the only way to get a wrapper into a `*sql.DB` was `sql.Register`, which is process-global, panics on a name it has already seen, and cannot be undone, so a library had to invent a unique driver name per configuration. `sql.OpenDB` registers nothing. Constructing a `&sqlite.Driver{}` is not an alternative — its fields are unexported, so it carries none of the registrations. `NewConnector` checks the DSN only as far as it can without opening a database — a query string that does not parse, and conflicting `vfs` parameters; everything else continues to be validated when the connection is opened, so an unknown parameter or an out-of-range value is reported by `Connect` rather than at construction. Nothing about the existing `sql.Open` path changes: `*Driver` deliberately still does not implement `driver.DriverContext`, so `sql.Open` remains lazy and DSN errors continue to surface where they always have. A runnable sample is in `examples/connector`. Resolves [GitLab issue #253](https://gitlab.com/cznic/sqlite/-/issues/253), thanks Alessandro Segala (@ItalyPaleAle)!
     - Document that a caller-constructed `sqlite.Driver` is not the driver this package registers as `"sqlite"`. Its fields are unexported, so it starts with no functions, collations or connection hooks and the only way to give it any is its own `RegisterConnectionHook` method; the package-level `Register*` functions always apply to the registered driver. Connections such a `Driver` opens therefore run without the package-level functions and collations — and because a registered function silently replaces a SQLite built-in of the same name, a `Driver` you construct can evaluate `upper(x)`, `date(x)` and the like differently from one opened through `sql.Open`. Virtual table modules are the one exception: they are held process-globally and reach every `Driver`. Constructing one remains supported for the private-hook pattern — a driver registered under a name of its own with `sql.Register` so its connection hooks apply only to its own connections — and is otherwise best avoided in favour of `sql.Open` or `NewConnector`. Documentation only; no behavior changes.

 - 2026-07-20 v1.55.0:
+12 −7
Original line number Diff line number Diff line
@@ -7,7 +7,6 @@ package sqlite // import "modernc.org/sqlite"
import (
	"context"
	"database/sql/driver"
	"net/url"
	"strings"
)

@@ -53,17 +52,23 @@ import (
// no way to undo a registration; a library doing the above would have to
// invent a unique name per configuration. sql.OpenDB registers nothing.
//
// Only the syntax of the dsn query string is checked here -- a malformed
// query, such as one carrying an invalid percent-escape, is reported
// immediately. Parameter values are validated when a connection is opened, so
// an unknown parameter or an out-of-range value is reported by Connect, and
// hence by the first use of the sql.DB, rather than by NewConnector.
// The dsn is checked here only as far as it can be without opening a database:
// a query string that does not parse, such as one carrying an invalid
// percent-escape, and conflicting vfs parameters are reported immediately.
// Everything else is validated when the connection is opened, so an unknown
// parameter or an out-of-range value is reported by Connect, and hence by the
// first use of the sql.DB, rather than by NewConnector.
//
// The returned Connector is safe for concurrent use; database/sql calls
// Connect from multiple goroutines as it grows the pool. As with sql.Open, it
// does not itself open a connection.
func NewConnector(dsn string) (driver.Connector, error) {
	if _, err := url.ParseQuery(dsnQuery(dsn)); err != nil {
	// getVFSName is the first thing newConn does with the query string: it
	// parses it, which rejects a malformed query, and it rejects conflicting
	// vfs parameters. Calling that same function rather than repeating its
	// parse here keeps what NewConnector rejects aligned with what opening a
	// connection rejects, by construction.
	if _, err := getVFSName(dsnQuery(dsn)); err != nil {
		return nil, err
	}

+4 −1
Original line number Diff line number Diff line
@@ -181,12 +181,15 @@ func TestConnectorWrapping(t *testing.T) {
}

// TestConnectorRejectsMalformedQuery covers the eager half of the validation
// contract: a dsn whose query string is not parseable fails at construction.
// contract: what can be rejected without opening a database is rejected at
// construction. That is a query string that does not parse, and conflicting
// vfs parameters.
func TestConnectorRejectsMalformedQuery(t *testing.T) {
	for _, dsn := range []string{
		"file::memory:?_pragma=%zz",
		"file::memory:?%",
		"file::memory:?a=%2",
		"file::memory:?vfs=a&vfs=b",
	} {
		c, err := NewConnector(dsn)
		if err == nil {