Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed url handling with reserved characters (fixes #402) #774

Merged
merged 2 commits into from Sep 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/tests.yml
Expand Up @@ -19,7 +19,7 @@ jobs:
mysql:
image: mysql:5.7
env:
MYSQL_ROOT_PASSWORD: root
MYSQL_ROOT_PASSWORD: root#
sio4 marked this conversation as resolved.
Show resolved Hide resolved
ports:
- 3307:3306
# needed because the mysql container does not provide a healthcheck
Expand Down Expand Up @@ -63,7 +63,7 @@ jobs:
image: postgres:10
env:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
POSTGRES_PASSWORD: postgres#
POSTGRES_DB: postgres
ports:
- 5433:5432
Expand All @@ -80,7 +80,7 @@ jobs:
- name: Build and run soda
env:
SODA_DIALECT: "postgres"
POSTGRESQL_URL: "postgres://postgres:postgres@127.0.0.1:${{ job.services.postgres.ports[5432] }}/pop_test?sslmode=disable"
POSTGRESQL_URL: "postgres://postgres:postgres%23@127.0.0.1:${{ job.services.postgres.ports[5432] }}/pop_test?sslmode=disable"
sio4 marked this conversation as resolved.
Show resolved Hide resolved
run: |
go build -v -tags sqlite -o tsoda ./soda
./tsoda drop -e $SODA_DIALECT -p ./testdata/migrations
Expand All @@ -90,7 +90,7 @@ jobs:
- name: Test
env:
SODA_DIALECT: "postgres"
POSTGRESQL_URL: "postgres://postgres:postgres@127.0.0.1:${{ job.services.postgres.ports[5432] }}/pop_test?sslmode=disable"
POSTGRESQL_URL: "postgres://postgres:postgres%23@127.0.0.1:${{ job.services.postgres.ports[5432] }}/pop_test?sslmode=disable"
run: |
go test -tags sqlite -race -cover ./...

Expand Down
12 changes: 6 additions & 6 deletions connection_details_test.go
Expand Up @@ -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"
sio4 marked this conversation as resolved.
Show resolved Hide resolved
cd := &ConnectionDetails{
URL: url,
}
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions database.yml
Expand Up @@ -4,12 +4,12 @@ mysql:
host: {{ envOr "MYSQL_HOST" "127.0.0.1" }}
port: {{ envOr "MYSQL_PORT" "3306" }}
user: {{ envOr "MYSQL_USER" "root" }}
password: {{ envOr "MYSQL_PASSWORD" "root" }}
password: {{ envOr "MYSQL_PASSWORD" "root#" }}
options:
readTimeout: 5s

postgres:
url: {{ envOr "POSTGRESQL_URL" "postgres://postgres:postgres@localhost:5433/pop_test?sslmode=disable" }}
url: {{ envOr "POSTGRESQL_URL" "postgres://postgres:postgres%23@localhost:5433/pop_test?sslmode=disable" }}
pool: 25

cockroach:
Expand Down
5 changes: 3 additions & 2 deletions dialect_cockroach.go
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"fmt"
"io"
"net/url"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -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(""))
sio4 marked this conversation as resolved.
Show resolved Hide resolved
}

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 {
Expand Down
49 changes: 45 additions & 4 deletions dialect_cockroach_test.go
Expand Up @@ -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{
Expand All @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions dialect_postgresql.go
Expand Up @@ -3,6 +3,7 @@ package pop
import (
"fmt"
"io"
"net/url"
"os/exec"
"sync"

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
25 changes: 22 additions & 3 deletions dialect_postgresql_test.go
Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions docker-compose.yml
Expand Up @@ -4,10 +4,10 @@ services:
mysql:
image: mysql:5.7
environment:
- MYSQL_ROOT_PASSWORD=root
- MYSQL_ROOT_PASSWORD=root#
- MYSQL_DATABASE=pop_test
- MYSQL_USER=pop
- MYSQL_PASSWORD=pop
- MYSQL_PASSWORD=pop#
ports:
- "3307:3306"
healthcheck:
Expand All @@ -22,7 +22,7 @@ services:
environment:
- POSTGRES_DB=pop_test
- POSTGRES_USER=postgres
- POSTGRES_PASSWORD=postgres
- POSTGRES_PASSWORD=postgres#
- POSTGRES_DB=postgres
ports:
- "5433:5432"
Expand Down