fixed url handling with reserved characters (fixes #402)

connection_details_test.go

@@ -10,23 +10,23 @@ func Test_ConnectionDetails_Finalize(t *testing.T) {
 	r := require.New(t)
 
 	cd := &ConnectionDetails{
-		URL: "postgres://user:pass@host:1234/database",
+		URL: "postgres://user:pass%23@host:1234/database",
 	}
 	err := cd.Finalize()
 	r.NoError(err)
 
 	r.Equal("database", cd.Database)
 	r.Equal("postgres", cd.Dialect)
 	r.Equal("host", cd.Host)
-	r.Equal("pass", cd.Password)
+	r.Equal("pass#", cd.Password)
 	r.Equal("1234", cd.Port)
 	r.Equal("user", cd.User)
 }
 
 func Test_ConnectionDetails_Finalize_MySQL_Standard(t *testing.T) {
 	r := require.New(t)
 
-	url := "mysql://user:pass@(host:1337)/database?param1=value1&param2=value2"
+	url := "mysql://user:pass#@(host:1337)/database?param1=value1&param2=value2"
 	cd := &ConnectionDetails{
 		URL: url,
 	}
@@ -36,7 +36,7 @@ func Test_ConnectionDetails_Finalize_MySQL_Standard(t *testing.T) {
 	r.Equal(url, cd.URL)
 	r.Equal("mysql", cd.Dialect)
 	r.Equal("user", cd.User)
-	r.Equal("pass", cd.Password)
+	r.Equal("pass#", cd.Password)
 	r.Equal("host", cd.Host)
 	r.Equal("1337", cd.Port)
 	r.Equal("database", cd.Database)
@@ -46,7 +46,7 @@ func Test_ConnectionDetails_Finalize_Cockroach(t *testing.T) {
 	r := require.New(t)
 	cd := &ConnectionDetails{
 		Dialect: "cockroach",
-		URL:     "postgres://user:pass@host:1234/database?sslmode=require&sslrootcert=certs/ca.crt&sslkey=certs/client.key&sslcert=certs/client.crt",
+		URL:     "postgres://user:pass%23@host:1234/database?sslmode=require&sslrootcert=certs/ca.crt&sslkey=certs/client.key&sslcert=certs/client.crt",
 	}
 	err := cd.Finalize()
 	r.NoError(err)
@@ -55,7 +55,7 @@ func Test_ConnectionDetails_Finalize_Cockroach(t *testing.T) {
 	r.Equal("host", cd.Host)
 	r.Equal("1234", cd.Port)
 	r.Equal("user", cd.User)
-	r.Equal("pass", cd.Password)
+	r.Equal("pass#", cd.Password)
 }
 
 func Test_ConnectionDetails_Finalize_UnknownSchemeURL(t *testing.T) {

dialect_cockroach.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -175,13 +176,13 @@ func (p *cockroach) URL() string {
 		return c.URL
 	}
 	s := "postgres://%s:%s@%s:%s/%s?%s"
-	return fmt.Sprintf(s, c.User, c.Password, c.Host, c.Port, c.Database, c.OptionsString(""))
+	return fmt.Sprintf(s, c.User, url.QueryEscape(c.Password), c.Host, c.Port, c.Database, c.OptionsString(""))
 }
 
 func (p *cockroach) urlWithoutDb() string {
 	c := p.ConnectionDetails
 	s := "postgres://%s:%s@%s:%s/?%s"
-	return fmt.Sprintf(s, c.User, c.Password, c.Host, c.Port, c.OptionsString(""))
+	return fmt.Sprintf(s, c.User, url.QueryEscape(c.Password), c.Host, c.Port, c.OptionsString(""))
 }
 
 func (p *cockroach) MigrationURL() string {

dialect_cockroach_test.go

@@ -7,6 +7,44 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func Test_Cockroach_ConnectionDetails_URL_Finalize(t *testing.T) {
+	r := require.New(t)
+
+	cd := &ConnectionDetails{
+		Dialect: "cockroach",
+		URL:     "cockroach://user:pass%23@host:1234/database",
+	}
+	err := cd.Finalize()
+	r.NoError(err)
+
+	r.Equal("database", cd.Database)
+	r.Equal("cockroach", cd.Dialect)
+	r.Equal("host", cd.Host)
+	r.Equal("pass#", cd.Password)
+	r.Equal("1234", cd.Port)
+	r.Equal("user", cd.User)
+}
+
+func Test_Cockroach_ConnectionDetails_Values_Finalize(t *testing.T) {
+	r := require.New(t)
+
+	cd := &ConnectionDetails{
+		Dialect:  "cockroach",
+		Database: "database",
+		Host:     "host",
+		Port:     "1234",
+		User:     "user",
+		Password: "pass#",
+		Options:  map[string]string{"application_name": "testing"},
+	}
+	err := cd.Finalize()
+	r.NoError(err)
+
+	p := &cockroach{commonDialect: commonDialect{ConnectionDetails: cd}}
+
+	r.Equal("postgres://user:pass%23@host:1234/database?application_name=testing", p.URL())
+}
+
 func Test_Cockroach_URL_Raw(t *testing.T) {
 	r := require.New(t)
 	cd := &ConnectionDetails{
@@ -29,21 +67,24 @@ func Test_Cockroach_URL_Build(t *testing.T) {
 		Host:     "host",
 		Port:     "port",
 		User:     "user",
-		Password: "pass",
+		Password: "pass#",
 		Options: map[string]string{
 			"option1": "value1",
 		},
 	}
 	err := cd.Finalize()
 	r.NoError(err)
+
 	m := &cockroach{commonDialect: commonDialect{ConnectionDetails: cd}}
-	r.True(strings.HasPrefix(m.URL(), "postgres://user:pass@host:port/database?"), "URL() returns %v", m.URL())
+	r.True(strings.HasPrefix(m.URL(), "postgres://user:pass%23@host:port/database?"), "URL() returns %v", m.URL())
 	r.Contains(m.URL(), "option1=value1")
 	r.Contains(m.URL(), "application_name=pop.test")
-	r.True(strings.HasPrefix(m.urlWithoutDb(), "postgres://user:pass@host:port/?"), "urlWithoutDb() returns %v", m.urlWithoutDb())
+
+	r.True(strings.HasPrefix(m.urlWithoutDb(), "postgres://user:pass%23@host:port/?"), "urlWithoutDb() returns %v", m.urlWithoutDb())
 	r.Contains(m.urlWithoutDb(), "option1=value1")
 	r.Contains(m.urlWithoutDb(), "application_name=pop.test")
-	r.True(strings.HasPrefix(m.MigrationURL(), "postgres://user:pass@host:port/database?"), "MigrationURL() returns %v", m.MigrationURL())
+
+	r.True(strings.HasPrefix(m.MigrationURL(), "postgres://user:pass%23@host:port/database?"), "MigrationURL() returns %v", m.MigrationURL())
 }
 
 func Test_Cockroach_URL_UserDefinedAppName(t *testing.T) {

dialect_postgresql.go

@@ -3,6 +3,7 @@ package pop
 import (
 	"fmt"
 	"io"
+	"net/url"
 	"os/exec"
 	"sync"
 
@@ -162,7 +163,7 @@ func (p *postgresql) URL() string {
 		return c.URL
 	}
 	s := "postgres://%s:%s@%s:%s/%s?%s"
-	return fmt.Sprintf(s, c.User, c.Password, c.Host, c.Port, c.Database, c.OptionsString(""))
+	return fmt.Sprintf(s, c.User, url.QueryEscape(c.Password), c.Host, c.Port, c.Database, c.OptionsString(""))
 }
 
 func (p *postgresql) urlWithoutDb() string {
@@ -172,7 +173,7 @@ func (p *postgresql) urlWithoutDb() string {
 	// To avoid a connection problem if the user db is not here, we use the default "postgres"
 	// db, just like the other client tools do.
 	s := "postgres://%s:%s@%s:%s/postgres?%s"
-	return fmt.Sprintf(s, c.User, c.Password, c.Host, c.Port, c.OptionsString(""))
+	return fmt.Sprintf(s, c.User, url.QueryEscape(c.Password), c.Host, c.Port, c.OptionsString(""))
 }
 
 func (p *postgresql) MigrationURL() string {

dialect_postgresql_test.go

@@ -8,10 +8,29 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func Test_PostgreSQL_ConnectionDetails_Values_Finalize(t *testing.T) {
+	r := require.New(t)
+
+	cd := &ConnectionDetails{
+		Dialect:  "postgres",
+		Database: "database",
+		Host:     "host",
+		Port:     "1234",
+		User:     "user",
+		Password: "pass#",
+	}
+	err := cd.Finalize()
+	r.NoError(err)
+
+	p := &postgresql{commonDialect: commonDialect{ConnectionDetails: cd}}
+
+	r.Equal("postgres://user:pass%23@host:1234/database?", p.URL())
+}
+
 func Test_PostgreSQL_Connection_String(t *testing.T) {
 	r := require.New(t)
 
-	url := "host=host port=1234 dbname=database user=user password=pass"
+	url := "host=host port=1234 dbname=database user=user password=pass#"
 	cd := &ConnectionDetails{
 		Dialect: "postgres",
 		URL:     url,
@@ -22,7 +41,7 @@ func Test_PostgreSQL_Connection_String(t *testing.T) {
 	r.Equal(url, cd.URL)
 	r.Equal("postgres", cd.Dialect)
 	r.Equal("host", cd.Host)
-	r.Equal("pass", cd.Password)
+	r.Equal("pass#", cd.Password)
 	r.Equal("1234", cd.Port)
 	r.Equal("user", cd.User)
 	r.Equal("database", cd.Database)
@@ -31,7 +50,7 @@ func Test_PostgreSQL_Connection_String(t *testing.T) {
 func Test_PostgreSQL_Connection_String_Options(t *testing.T) {
 	r := require.New(t)
 
-	url := "host=host port=1234 dbname=database user=user password=pass sslmode=disable fallback_application_name=test_app connect_timeout=10 sslcert=/some/location sslkey=/some/other/location sslrootcert=/root/location"
+	url := "host=host port=1234 dbname=database user=user password=pass# sslmode=disable fallback_application_name=test_app connect_timeout=10 sslcert=/some/location sslkey=/some/other/location sslrootcert=/root/location"
 	cd := &ConnectionDetails{
 		Dialect: "postgres",
 		URL:     url,