Repository: amacneil/dbmate
Pull request #200 (2021-02-26 19:33:49)

mysql: Use url.PathUnescape instead of url.QueryEscape so that '+' in passwords works
This addresses #199 which I just opened. As the [documentation][pathunescape] says:

> PathUnescape is identical to QueryUnescape except that it does not unescape '+' to ' ' (space).

I think that makes sense, since `+` in passwords gets converted to a space otherwise, and this isn't correct.

[pathunescape]: https://golang.org/pkg/net/url/#PathUnescape

amacneil: Thanks for the PR and bug report! I think this makes sense - it seems like an oversight (I don't think I was even aware of those two separate methods).

There are a bunch of tests for `connectionString()`, can you add another one (or maybe modify the `"special chars"` test) with a comment explaining the correct `+` handling we are looking for?
matt-snider: @amacneil I added a test, but I sadly couldn't get `make test` to work properly a reasonable amount of time, gave up, and pushed, hoping the CI would tell me if it was correct. I have only written a small amount of Go so the problem is definitely on my end. The CI still hasn't responded, but I guess we'll see shortly whether it was correct!
amacneil: Strange, I'm not sure why CI didn't trigger. I'll take a look later today!

Patch:
diff --git a/pkg/dbmate/db.go b/pkg/dbmate/db.go
index b955686..815a5cb 100644
--- a/pkg/dbmate/db.go
+++ b/pkg/dbmate/db.go
@@ -72,7 +72,7 @@ func New(databaseURL *url.URL) *DB {
 // GetDriver initializes the appropriate database driver
 func (db *DB) GetDriver() (Driver, error) {
 	if db.DatabaseURL == nil || db.DatabaseURL.Scheme == "" {
-		return nil, errors.New("invalid url")
+		return nil, errors.New("invalid url, have you set your --url flag or DATABASE_URL environment variable?")
 	}
 
 	driverFunc := drivers[db.DatabaseURL.Scheme]
diff --git a/pkg/driver/mysql/mysql.go b/pkg/driver/mysql/mysql.go
index ca67330..fd25747 100644
--- a/pkg/driver/mysql/mysql.go
+++ b/pkg/driver/mysql/mysql.go
@@ -52,7 +52,7 @@ func connectionString(u *url.URL) string {
 
 	// Get decoded user:pass
 	userPassEncoded := u.User.String()
-	userPass, _ := url.QueryUnescape(userPassEncoded)
+	userPass, _ := url.PathUnescape(userPassEncoded)
 
 	// Build DSN w/ user:pass percent-decoded
 	normalizedString := ""


Test patch:
diff --git a/pkg/dbmate/db_test.go b/pkg/dbmate/db_test.go
index 0c07457..7b50daf 100644
--- a/pkg/dbmate/db_test.go
+++ b/pkg/dbmate/db_test.go
@@ -55,14 +55,14 @@ func TestGetDriver(t *testing.T) {
 		db := dbmate.New(nil)
 		drv, err := db.GetDriver()
 		require.Nil(t, drv)
-		require.EqualError(t, err, "invalid url")
+		require.EqualError(t, err, "invalid url, have you set your --url flag or DATABASE_URL environment variable?")
 	})
 
 	t.Run("missing schema", func(t *testing.T) {
 		db := dbmate.New(dbutil.MustParseURL("//hi"))
 		drv, err := db.GetDriver()
 		require.Nil(t, drv)
-		require.EqualError(t, err, "invalid url")
+		require.EqualError(t, err, "invalid url, have you set your --url flag or DATABASE_URL environment variable?")
 	})
 
 	t.Run("invalid driver", func(t *testing.T) {
diff --git a/pkg/driver/mysql/mysql_test.go b/pkg/driver/mysql/mysql_test.go
index 0ff97d0..a4d2485 100644
--- a/pkg/driver/mysql/mysql_test.go
+++ b/pkg/driver/mysql/mysql_test.go
@@ -78,6 +78,18 @@ func TestConnectionString(t *testing.T) {
 		require.Equal(t, "duhfsd7s:123!@123!@@tcp(host:123)/foo?flag=on&multiStatements=true", s)
 	})
 
+	t.Run("url encoding", func(t *testing.T) {
+		u, err := url.Parse("mysql://bob%2Balice:secret%5E%5B%2A%28%29@host:123/foo")
+		require.NoError(t, err)
+		require.Equal(t, "bob+alice:secret%5E%5B%2A%28%29", u.User.String())
+		require.Equal(t, "123", u.Port())
+
+		s := connectionString(u)
+		// ensure that '+' is correctly encoded by url.PathUnescape as '+'
+		// (not whitespace as url.QueryUnescape generates)
+		require.Equal(t, "bob+alice:secret^[*()@tcp(host:123)/foo?multiStatements=true", s)
+	})
+
 	t.Run("socket", func(t *testing.T) {
 		// test with no user/pass
 		u, err := url.Parse("mysql:///foo?socket=/var/run/mysqld/mysqld.sock&flag=on")