Skip to content

Commit

Permalink
Improve error messaging when URL parsing fails
Browse files Browse the repository at this point in the history 
    - Improve docs around escaping/encoding reserved URL characters
    - Add tests for net/url Parse() and reserved characters to document understanding
    - Addresses: golang-migrate#77
  • Loading branch information
@dhui
dhui committed Jul 25, 2018
1 parent 97eb0d1 commit b5edb8e
Show file tree
Hide file tree
Showing 3 changed files with 265 additions and 1 deletion.
19 changes: 19 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion database/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand Down
244 changes: 244 additions & 0 deletions database/parse_test.go
View file
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}

0 comments on commit b5edb8e

Please sign in to comment.