Merge pull request #78 from dhui/mysql_dsn

Allow DB connection URLs to contain encoded reserved URL characters
This commit is contained in:
Dale Hui 2018-07-24 18:22:04 -07:00 committed by GitHub
commit c8705dd876
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 376 additions and 12 deletions

View File

@ -38,6 +38,7 @@ test-with-flags:
@go test $(TEST_FLAGS) .
@go test $(TEST_FLAGS) ./cli/...
@go test $(TEST_FLAGS) ./database
@go test $(TEST_FLAGS) ./testing/...
@echo -n '$(SOURCE)' | tr -s ' ' '\n' | xargs -I{} go test $(TEST_FLAGS) ./source/{}

View File

@ -39,6 +39,25 @@ Database drivers run migrations. [Add a new database?](database/driver.go)
* [CockroachDB](database/cockroachdb)
* [ClickHouse](database/clickhouse)
### Database URLs
Database connection strings are specified via URLs. The URL format is driver dependent but generally has the form: `dbdriver://username:password@host:port/dbname?option1=true&option2=false`
Any [reserved URL characters](https://en.wikipedia.org/wiki/Percent-encoding#Percent-encoding_reserved_characters) need to be escaped. Note, the `%` character also [needs to be escaped](https://en.wikipedia.org/wiki/Percent-encoding#Percent-encoding_the_percent_character)
Explicitly, the following characters need to be escaped:
`!`, `#`, `$`, `%`, `&`, `'`, `(`, `)`, `*`, `+`, `,`, `/`, `:`, `;`, `=`, `?`, `@`, `[`, `]`
It's easiest to always run the URL parts of your DB connection URL (e.g. username, password, etc) through an URL encoder. See the example Python helpers below:
```bash
$ python3 -c 'import urllib.parse; print(urllib.parse.quote(input("String to encode: "), ""))'
String to encode: FAKEpassword!#$%&'()*+,/:;=?@[]
FAKEpassword%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D
$ python2 -c 'import urllib; print urllib.quote(raw_input("String to encode: "), "")'
String to encode: FAKEpassword!#$%&'()*+,/:;=?@[]
FAKEpassword%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D
$
```
## Migration Sources

View File

@ -81,7 +81,8 @@ type Driver interface {
func Open(url string) (Driver, error) {
u, err := nurl.Parse(url)
if err != nil {
return nil, err
return nil, fmt.Errorf("Unable to parse URL. Did you escape all reserved URL characters? "+
"See: https://github.com/golang-migrate/migrate#database-urls Error: %v", err)
}
if u.Scheme == "" {

View File

@ -13,8 +13,13 @@ import (
nurl "net/url"
"strconv"
"strings"
)
import (
"github.com/go-sql-driver/mysql"
)
import (
"github.com/golang-migrate/migrate"
"github.com/golang-migrate/migrate/database"
)
@ -89,8 +94,26 @@ func WithInstance(instance *sql.DB, config *Config) (database.Driver, error) {
return mx, nil
}
// urlToMySQLConfig takes a net/url URL and returns a go-sql-driver/mysql Config.
// Manually sets username and password to avoid net/url from url-encoding the reserved URL characters
func urlToMySQLConfig(u nurl.URL) (*mysql.Config, error) {
origUserInfo := u.User
u.User = nil
c, err := mysql.ParseDSN(strings.TrimPrefix(u.String(), "mysql://"))
if err != nil {
return nil, err
}
if origUserInfo != nil {
c.User = origUserInfo.Username()
if p, ok := origUserInfo.Password(); ok {
c.Passwd = p
}
}
return c, nil
}
func (m *Mysql) Open(url string) (database.Driver, error) {
url = strings.TrimPrefix(url, "mysql://")
purl, err := nurl.Parse(url)
if err != nil {
return nil, err
@ -100,7 +123,11 @@ func (m *Mysql) Open(url string) (database.Driver, error) {
q.Set("multiStatements", "true")
purl.RawQuery = q.Encode()
db, err := sql.Open("mysql", migrate.FilterCustomQuery(purl).String())
c, err := urlToMySQLConfig(*migrate.FilterCustomQuery(purl))
if err != nil {
return nil, err
}
db, err := sql.Open("mysql", c.FormatDSN())
if err != nil {
return nil, err
}

View File

@ -4,11 +4,15 @@ import (
"database/sql"
sqldriver "database/sql/driver"
"fmt"
// "io/ioutil"
// "log"
"net/url"
"testing"
)
import (
"github.com/go-sql-driver/mysql"
)
import (
dt "github.com/golang-migrate/migrate/database/testing"
mt "github.com/golang-migrate/migrate/testing"
)
@ -97,3 +101,55 @@ func TestLockWorks(t *testing.T) {
}
})
}
func TestURLToMySQLConfig(t *testing.T) {
testcases := []struct {
name string
urlStr string
expectedDSN string // empty string signifies that an error is expected
}{
{name: "no user/password", urlStr: "mysql://tcp(127.0.0.1:3306)/myDB?multiStatements=true",
expectedDSN: "tcp(127.0.0.1:3306)/myDB?multiStatements=true"},
{name: "only user", urlStr: "mysql://username@tcp(127.0.0.1:3306)/myDB?multiStatements=true",
expectedDSN: "username@tcp(127.0.0.1:3306)/myDB?multiStatements=true"},
{name: "only user - with encoded :",
urlStr: "mysql://username%3A@tcp(127.0.0.1:3306)/myDB?multiStatements=true",
expectedDSN: "username:@tcp(127.0.0.1:3306)/myDB?multiStatements=true"},
{name: "only user - with encoded @",
urlStr: "mysql://username%40@tcp(127.0.0.1:3306)/myDB?multiStatements=true",
expectedDSN: "username@@tcp(127.0.0.1:3306)/myDB?multiStatements=true"},
{name: "user/password", urlStr: "mysql://username:password@tcp(127.0.0.1:3306)/myDB?multiStatements=true",
expectedDSN: "username:password@tcp(127.0.0.1:3306)/myDB?multiStatements=true"},
// Not supported yet: https://github.com/go-sql-driver/mysql/issues/591
// {name: "user/password - user with encoded :",
// urlStr: "mysql://username%3A:password@tcp(127.0.0.1:3306)/myDB?multiStatements=true",
// expectedDSN: "username::pasword@tcp(127.0.0.1:3306)/myDB?multiStatements=true"},
{name: "user/password - user with encoded @",
urlStr: "mysql://username%40:password@tcp(127.0.0.1:3306)/myDB?multiStatements=true",
expectedDSN: "username@:password@tcp(127.0.0.1:3306)/myDB?multiStatements=true"},
{name: "user/password - password with encoded :",
urlStr: "mysql://username:password%3A@tcp(127.0.0.1:3306)/myDB?multiStatements=true",
expectedDSN: "username:password:@tcp(127.0.0.1:3306)/myDB?multiStatements=true"},
{name: "user/password - password with encoded @",
urlStr: "mysql://username:password%40@tcp(127.0.0.1:3306)/myDB?multiStatements=true",
expectedDSN: "username:password@@tcp(127.0.0.1:3306)/myDB?multiStatements=true"},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
u, err := url.Parse(tc.urlStr)
if err != nil {
t.Fatal("Failed to parse url string:", tc.urlStr, "error:", err)
}
if config, err := urlToMySQLConfig(*u); err == nil {
dsn := config.FormatDSN()
if dsn != tc.expectedDSN {
t.Error("Got unexpected DSN:", dsn, "!=", tc.expectedDSN)
}
} else {
if tc.expectedDSN != "" {
t.Error("Got unexpected error:", err, "urlStr:", tc.urlStr)
}
}
})
}
}

244
database/parse_test.go Normal file
View File

@ -0,0 +1,244 @@
package database_test
import (
"encoding/hex"
"net/url"
"strings"
"testing"
)
const reservedChars = "!#$%&'()*+,/:;=?@[]"
// TestUserUnencodedReservedURLChars documents the behavior of using unencoded reserved characters in usernames with
// net/url Parse()
func TestUserUnencodedReservedURLChars(t *testing.T) {
scheme := "database://"
baseUsername := "username"
urlSuffix := "password@localhost:12345/myDB?someParam=true"
urlSuffixAndSep := ":" + urlSuffix
testcases := []struct {
char string
parses bool
expectedUsername string // empty string means that the username failed to parse
encodedURL string
}{
{char: "!", parses: true, expectedUsername: baseUsername + "!",
encodedURL: scheme + baseUsername + "%21" + urlSuffixAndSep},
{char: "#", parses: true, expectedUsername: "",
encodedURL: scheme + baseUsername + "#" + urlSuffixAndSep},
{char: "$", parses: true, expectedUsername: baseUsername + "$",
encodedURL: scheme + baseUsername + "$" + urlSuffixAndSep},
{char: "%", parses: false},
{char: "&", parses: true, expectedUsername: baseUsername + "&",
encodedURL: scheme + baseUsername + "&" + urlSuffixAndSep},
{char: "'", parses: true, expectedUsername: "username'",
encodedURL: scheme + baseUsername + "%27" + urlSuffixAndSep},
{char: "(", parses: true, expectedUsername: "username(",
encodedURL: scheme + baseUsername + "%28" + urlSuffixAndSep},
{char: ")", parses: true, expectedUsername: "username)",
encodedURL: scheme + baseUsername + "%29" + urlSuffixAndSep},
{char: "*", parses: true, expectedUsername: "username*",
encodedURL: scheme + baseUsername + "%2A" + urlSuffixAndSep},
{char: "+", parses: true, expectedUsername: "username+",
encodedURL: scheme + baseUsername + "+" + urlSuffixAndSep},
{char: ",", parses: true, expectedUsername: "username,",
encodedURL: scheme + baseUsername + "," + urlSuffixAndSep},
{char: "/", parses: true, expectedUsername: "",
encodedURL: scheme + baseUsername + "/" + urlSuffixAndSep},
{char: ":", parses: true, expectedUsername: "username",
encodedURL: scheme + baseUsername + ":%3A" + urlSuffix},
{char: ";", parses: true, expectedUsername: "username;",
encodedURL: scheme + baseUsername + ";" + urlSuffixAndSep},
{char: "=", parses: true, expectedUsername: "username=",
encodedURL: scheme + baseUsername + "=" + urlSuffixAndSep},
{char: "?", parses: true, expectedUsername: "",
encodedURL: scheme + baseUsername + "?" + urlSuffixAndSep},
{char: "@", parses: true, expectedUsername: "username@",
encodedURL: scheme + baseUsername + "%40" + urlSuffixAndSep},
{char: "[", parses: false},
{char: "]", parses: false},
}
testedChars := make([]string, 0, len(reservedChars))
for _, tc := range testcases {
testedChars = append(testedChars, tc.char)
t.Run("reserved char "+tc.char, func(t *testing.T) {
s := scheme + baseUsername + tc.char + urlSuffixAndSep
u, err := url.Parse(s)
if err == nil {
if !tc.parses {
t.Error("Unexpectedly parsed reserved character. url:", s)
return
}
var username string
if u.User != nil {
username = u.User.Username()
}
if username != tc.expectedUsername {
t.Error("Got unexpected username:", username, "!=", tc.expectedUsername)
}
if s := u.String(); s != tc.encodedURL {
t.Error("Got unexpected encoded URL:", s, "!=", tc.encodedURL)
}
} else {
if tc.parses {
t.Error("Failed to parse reserved character. url:", s)
}
}
})
}
t.Run("All reserved chars tested", func(t *testing.T) {
if s := strings.Join(testedChars, ""); s != reservedChars {
t.Error("Not all reserved URL characters were tested:", s, "!=", reservedChars)
}
})
}
func TestUserEncodedReservedURLChars(t *testing.T) {
scheme := "database://"
baseUsername := "username"
urlSuffix := "password@localhost:12345/myDB?someParam=true"
urlSuffixAndSep := ":" + urlSuffix
for _, c := range reservedChars {
c := string(c)
t.Run("reserved char "+c, func(t *testing.T) {
encodedChar := "%" + hex.EncodeToString([]byte(c))
s := scheme + baseUsername + encodedChar + urlSuffixAndSep
expectedUsername := baseUsername + c
u, err := url.Parse(s)
if err != nil {
t.Fatal("Failed to parse url with encoded reserved character. url:", s)
}
if u.User == nil {
t.Fatal("Failed to parse userinfo with encoded reserve character. url:", s)
}
if username := u.User.Username(); username != expectedUsername {
t.Fatal("Got unexpected username:", username, "!=", expectedUsername)
}
})
}
}
// TestPasswordUnencodedReservedURLChars documents the behavior of using unencoded reserved characters in passwords
// with net/url Parse()
func TestPasswordUnencodedReservedURLChars(t *testing.T) {
username := "username"
schemeAndUsernameAndSep := "database://" + username + ":"
basePassword := "password"
urlSuffixAndSep := "@localhost:12345/myDB?someParam=true"
testcases := []struct {
char string
parses bool
expectedUsername string // empty string means that the username failed to parse
expectedPassword string // empty string means that the password failed to parse
encodedURL string
}{
{char: "!", parses: true, expectedUsername: username, expectedPassword: basePassword + "!",
encodedURL: schemeAndUsernameAndSep + basePassword + "%21" + urlSuffixAndSep},
{char: "#", parses: true, expectedUsername: "", expectedPassword: "",
encodedURL: schemeAndUsernameAndSep + basePassword + "#" + urlSuffixAndSep},
{char: "$", parses: true, expectedUsername: username, expectedPassword: basePassword + "$",
encodedURL: schemeAndUsernameAndSep + basePassword + "$" + urlSuffixAndSep},
{char: "%", parses: false},
{char: "&", parses: true, expectedUsername: username, expectedPassword: basePassword + "&",
encodedURL: schemeAndUsernameAndSep + basePassword + "&" + urlSuffixAndSep},
{char: "'", parses: true, expectedUsername: username, expectedPassword: "password'",
encodedURL: schemeAndUsernameAndSep + basePassword + "%27" + urlSuffixAndSep},
{char: "(", parses: true, expectedUsername: username, expectedPassword: "password(",
encodedURL: schemeAndUsernameAndSep + basePassword + "%28" + urlSuffixAndSep},
{char: ")", parses: true, expectedUsername: username, expectedPassword: "password)",
encodedURL: schemeAndUsernameAndSep + basePassword + "%29" + urlSuffixAndSep},
{char: "*", parses: true, expectedUsername: username, expectedPassword: "password*",
encodedURL: schemeAndUsernameAndSep + basePassword + "%2A" + urlSuffixAndSep},
{char: "+", parses: true, expectedUsername: username, expectedPassword: "password+",
encodedURL: schemeAndUsernameAndSep + basePassword + "+" + urlSuffixAndSep},
{char: ",", parses: true, expectedUsername: username, expectedPassword: "password,",
encodedURL: schemeAndUsernameAndSep + basePassword + "," + urlSuffixAndSep},
{char: "/", parses: true, expectedUsername: "", expectedPassword: "",
encodedURL: schemeAndUsernameAndSep + basePassword + "/" + urlSuffixAndSep},
{char: ":", parses: true, expectedUsername: username, expectedPassword: "password:",
encodedURL: schemeAndUsernameAndSep + basePassword + "%3A" + urlSuffixAndSep},
{char: ";", parses: true, expectedUsername: username, expectedPassword: "password;",
encodedURL: schemeAndUsernameAndSep + basePassword + ";" + urlSuffixAndSep},
{char: "=", parses: true, expectedUsername: username, expectedPassword: "password=",
encodedURL: schemeAndUsernameAndSep + basePassword + "=" + urlSuffixAndSep},
{char: "?", parses: true, expectedUsername: "", expectedPassword: "",
encodedURL: schemeAndUsernameAndSep + basePassword + "?" + urlSuffixAndSep},
{char: "@", parses: true, expectedUsername: username, expectedPassword: "password@",
encodedURL: schemeAndUsernameAndSep + basePassword + "%40" + urlSuffixAndSep},
{char: "[", parses: false},
{char: "]", parses: false},
}
testedChars := make([]string, 0, len(reservedChars))
for _, tc := range testcases {
testedChars = append(testedChars, tc.char)
t.Run("reserved char "+tc.char, func(t *testing.T) {
s := schemeAndUsernameAndSep + basePassword + tc.char + urlSuffixAndSep
u, err := url.Parse(s)
if err == nil {
if !tc.parses {
t.Error("Unexpectedly parsed reserved character. url:", s)
return
}
var username, password string
if u.User != nil {
username = u.User.Username()
password, _ = u.User.Password()
}
if username != tc.expectedUsername {
t.Error("Got unexpected username:", username, "!=", tc.expectedUsername)
}
if password != tc.expectedPassword {
t.Error("Got unexpected password:", password, "!=", tc.expectedPassword)
}
if s := u.String(); s != tc.encodedURL {
t.Error("Got unexpected encoded URL:", s, "!=", tc.encodedURL)
}
} else {
if tc.parses {
t.Error("Failed to parse reserved character. url:", s)
}
}
})
}
t.Run("All reserved chars tested", func(t *testing.T) {
if s := strings.Join(testedChars, ""); s != reservedChars {
t.Error("Not all reserved URL characters were tested:", s, "!=", reservedChars)
}
})
}
func TestPasswordEncodedReservedURLChars(t *testing.T) {
username := "username"
schemeAndUsernameAndSep := "database://" + username + ":"
basePassword := "password"
urlSuffixAndSep := "@localhost:12345/myDB?someParam=true"
for _, c := range reservedChars {
c := string(c)
t.Run("reserved char "+c, func(t *testing.T) {
encodedChar := "%" + hex.EncodeToString([]byte(c))
s := schemeAndUsernameAndSep + basePassword + encodedChar + urlSuffixAndSep
expectedPassword := basePassword + c
u, err := url.Parse(s)
if err != nil {
t.Fatal("Failed to parse url with encoded reserved character. url:", s)
}
if u.User == nil {
t.Fatal("Failed to parse userinfo with encoded reserve character. url:", s)
}
if n := u.User.Username(); n != username {
t.Fatal("Got unexpected username:", n, "!=", username)
}
if p, _ := u.User.Password(); p != expectedPassword {
t.Fatal("Got unexpected password:", p, "!=", expectedPassword)
}
})
}
}

View File

@ -7,7 +7,7 @@ import (
const advisoryLockIdSalt uint = 1486364155
// inspired by rails migrations, see https://goo.gl/8o9bCT
// GenerateAdvisoryLockId inspired by rails migrations, see https://goo.gl/8o9bCT
func GenerateAdvisoryLockId(databaseName string) (string, error) {
sum := crc32.ChecksumIEEE([]byte(databaseName))
sum = sum * uint32(advisoryLockIdSalt)

View File

@ -1,12 +1,28 @@
package database
import (
"testing"
)
func TestGenerateAdvisoryLockId(t *testing.T) {
id, err := p.generateAdvisoryLockId("database_name")
if err != nil {
t.Errorf("expected err to be nil, got %v", err)
testcases := []struct {
dbname string
expectedID string // empty string signifies that an error is expected
}{
{dbname: "database_name", expectedID: "1764327054"},
}
if len(id) == 0 {
t.Errorf("expected generated id not to be empty")
for _, tc := range testcases {
t.Run(tc.dbname, func(t *testing.T) {
if id, err := GenerateAdvisoryLockId("database_name"); err == nil {
if id != tc.expectedID {
t.Error("Generated incorrect ID:", id, "!=", tc.expectedID)
}
} else {
if tc.expectedID != "" {
t.Error("Got unexpected error:", err)
}
}
})
}
t.Logf("generated id: %v", id)
}