From b5edb8e50c3b5d441b709224603fb02cdcb091ef Mon Sep 17 00:00:00 2001 From: Dale Hui Date: Tue, 24 Jul 2018 17:24:47 -0700 Subject: [PATCH] Improve error messaging when URL parsing fails - Improve docs around escaping/encoding reserved URL characters - Add tests for net/url Parse() and reserved characters to document understanding - Addresses: https://github.com/golang-migrate/migrate/issues/77 --- README.md | 19 ++++ database/driver.go | 3 +- database/parse_test.go | 244 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 database/parse_test.go diff --git a/README.md b/README.md index 9f1564b8b..1a45704c8 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/database/driver.go b/database/driver.go index 6c87db5b3..fa914c5d0 100644 --- a/database/driver.go +++ b/database/driver.go @@ -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 == "" { diff --git a/database/parse_test.go b/database/parse_test.go new file mode 100644 index 000000000..bfbc0ba60 --- /dev/null +++ b/database/parse_test.go @@ -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) + } + }) + } +}