Skip to content

Commit 07e0e44

Browse files
committed
fix(postgres,pgx): error instead of panicking on unquoted MigrationsTable
When MigrationsTableQuoted is enabled and MigrationsTable contains no double quotes (including when it is left empty and defaults to schema_migrations), the quote matcher returns no matches and result[len(result)-1] panics with an out-of-range index. Return a configuration error instead, matching the existing too many dot characters path. Add regression tests for the postgres, pgx and pgx/v5 drivers.
1 parent 504568a commit 07e0e44

6 files changed

Lines changed: 171 additions & 0 deletions

File tree

‎database/pgx/pgx.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ func WithInstance(instance *sql.DB, config *Config) (database.Driver, error) {
129129
if config.MigrationsTableQuoted {
130130
re := regexp.MustCompile(`"(.*?)"`)
131131
result := re.FindAllStringSubmatch(config.MigrationsTable, -1)
132+
if len(result) == 0 {
133+
return nil, fmt.Errorf("\"%s\" MigrationsTable is not quoted", config.MigrationsTable)
134+
}
132135
config.migrationsTableName = result[len(result)-1][1]
133136
if len(result) == 2 {
134137
config.migrationsSchemaName = result[0][1]

‎database/pgx/pgx_quoted_test.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package pgx
2+
3+
import (
4+
"context"
5+
"database/sql"
6+
"database/sql/driver"
7+
"errors"
8+
"testing"
9+
)
10+
11+
// pingOnlyDriver lets tests build a *sql.DB that answers Ping without a real
12+
// database, which is all WithInstance needs when DatabaseName and SchemaName
13+
// are set in the config.
14+
type pingOnlyDriver struct{}
15+
16+
func (pingOnlyDriver) Open(string) (driver.Conn, error) { return pingOnlyConn{}, nil }
17+
18+
type pingOnlyConn struct{}
19+
20+
func (pingOnlyConn) Prepare(string) (driver.Stmt, error) {
21+
return nil, errors.New("prepare not supported")
22+
}
23+
24+
func (pingOnlyConn) Close() error { return nil }
25+
26+
func (pingOnlyConn) Begin() (driver.Tx, error) {
27+
return nil, errors.New("transactions not supported")
28+
}
29+
30+
func (pingOnlyConn) Ping(context.Context) error { return nil }
31+
32+
var _ driver.Pinger = pingOnlyConn{}
33+
34+
func init() { sql.Register("migrate-fake-pgx", pingOnlyDriver{}) }
35+
36+
// TestWithInstanceQuotedTableWithoutQuotes ensures a quoted-table config whose
37+
// table name contains no quotes returns an error instead of panicking.
38+
func TestWithInstanceQuotedTableWithoutQuotes(t *testing.T) {
39+
db, err := sql.Open("migrate-fake-pgx", "")
40+
if err != nil {
41+
t.Fatal(err)
42+
}
43+
defer func() { _ = db.Close() }()
44+
45+
_, err = WithInstance(db, &Config{
46+
DatabaseName: "testdb",
47+
SchemaName: "public",
48+
MigrationsTable: "schema_migrations",
49+
MigrationsTableQuoted: true,
50+
})
51+
if err == nil {
52+
t.Fatal("expected an error when MigrationsTableQuoted is set without a quoted table name")
53+
}
54+
}

‎database/pgx/v5/pgx.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ func WithInstance(instance *sql.DB, config *Config) (database.Driver, error) {
109109
if config.MigrationsTableQuoted {
110110
re := regexp.MustCompile(`"(.*?)"`)
111111
result := re.FindAllStringSubmatch(config.MigrationsTable, -1)
112+
if len(result) == 0 {
113+
return nil, fmt.Errorf("\"%s\" MigrationsTable is not quoted", config.MigrationsTable)
114+
}
112115
config.migrationsTableName = result[len(result)-1][1]
113116
if len(result) == 2 {
114117
config.migrationsSchemaName = result[0][1]

‎database/pgx/v5/pgx_quoted_test.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package pgx
2+
3+
import (
4+
"context"
5+
"database/sql"
6+
"database/sql/driver"
7+
"errors"
8+
"testing"
9+
)
10+
11+
// pingOnlyDriver lets tests build a *sql.DB that answers Ping without a real
12+
// database, which is all WithInstance needs when DatabaseName and SchemaName
13+
// are set in the config.
14+
type pingOnlyDriver struct{}
15+
16+
func (pingOnlyDriver) Open(string) (driver.Conn, error) { return pingOnlyConn{}, nil }
17+
18+
type pingOnlyConn struct{}
19+
20+
func (pingOnlyConn) Prepare(string) (driver.Stmt, error) {
21+
return nil, errors.New("prepare not supported")
22+
}
23+
24+
func (pingOnlyConn) Close() error { return nil }
25+
26+
func (pingOnlyConn) Begin() (driver.Tx, error) {
27+
return nil, errors.New("transactions not supported")
28+
}
29+
30+
func (pingOnlyConn) Ping(context.Context) error { return nil }
31+
32+
var _ driver.Pinger = pingOnlyConn{}
33+
34+
func init() { sql.Register("migrate-fake-pgx5", pingOnlyDriver{}) }
35+
36+
// TestWithInstanceQuotedTableWithoutQuotes ensures a quoted-table config whose
37+
// table name contains no quotes returns an error instead of panicking.
38+
func TestWithInstanceQuotedTableWithoutQuotes(t *testing.T) {
39+
db, err := sql.Open("migrate-fake-pgx5", "")
40+
if err != nil {
41+
t.Fatal(err)
42+
}
43+
defer func() { _ = db.Close() }()
44+
45+
_, err = WithInstance(db, &Config{
46+
DatabaseName: "testdb",
47+
SchemaName: "public",
48+
MigrationsTable: "schema_migrations",
49+
MigrationsTableQuoted: true,
50+
})
51+
if err == nil {
52+
t.Fatal("expected an error when MigrationsTableQuoted is set without a quoted table name")
53+
}
54+
}

‎database/postgres/postgres.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ func WithConnection(ctx context.Context, conn *sql.Conn, config *Config) (*Postg
109109
if config.MigrationsTableQuoted {
110110
re := regexp.MustCompile(`"(.*?)"`)
111111
result := re.FindAllStringSubmatch(config.MigrationsTable, -1)
112+
if len(result) == 0 {
113+
return nil, fmt.Errorf("\"%s\" MigrationsTable is not quoted", config.MigrationsTable)
114+
}
112115
config.migrationsTableName = result[len(result)-1][1]
113116
if len(result) == 2 {
114117
config.migrationsSchemaName = result[0][1]
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package postgres
2+
3+
import (
4+
"context"
5+
"database/sql"
6+
"database/sql/driver"
7+
"errors"
8+
"testing"
9+
)
10+
11+
// pingOnlyDriver lets tests build a *sql.DB that answers Ping without a real
12+
// database, which is all WithInstance needs when DatabaseName and SchemaName
13+
// are set in the config.
14+
type pingOnlyDriver struct{}
15+
16+
func (pingOnlyDriver) Open(string) (driver.Conn, error) { return pingOnlyConn{}, nil }
17+
18+
type pingOnlyConn struct{}
19+
20+
func (pingOnlyConn) Prepare(string) (driver.Stmt, error) {
21+
return nil, errors.New("prepare not supported")
22+
}
23+
24+
func (pingOnlyConn) Close() error { return nil }
25+
26+
func (pingOnlyConn) Begin() (driver.Tx, error) {
27+
return nil, errors.New("transactions not supported")
28+
}
29+
30+
func (pingOnlyConn) Ping(context.Context) error { return nil }
31+
32+
var _ driver.Pinger = pingOnlyConn{}
33+
34+
func init() { sql.Register("migrate-fake-postgres", pingOnlyDriver{}) }
35+
36+
// TestWithInstanceQuotedTableWithoutQuotes ensures a quoted-table config whose
37+
// table name contains no quotes returns an error instead of panicking.
38+
func TestWithInstanceQuotedTableWithoutQuotes(t *testing.T) {
39+
db, err := sql.Open("migrate-fake-postgres", "")
40+
if err != nil {
41+
t.Fatal(err)
42+
}
43+
defer func() { _ = db.Close() }()
44+
45+
_, err = WithInstance(db, &Config{
46+
DatabaseName: "testdb",
47+
SchemaName: "public",
48+
MigrationsTable: "schema_migrations",
49+
MigrationsTableQuoted: true,
50+
})
51+
if err == nil {
52+
t.Fatal("expected an error when MigrationsTableQuoted is set without a quoted table name")
53+
}
54+
}

0 commit comments

Comments
 (0)