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

pgdriver.WithDSN panic when password contains a '#' and at least one letter before '#' #935

Open
zhuyanxi opened this issue Nov 29, 2023 · 5 comments

Comments

@zhuyanxi
Copy link

My PostgreSQL's password has a character '#'
(my dsn sample: postgres://postgres:1A#34@localhost:5432/postgres?sslmode=disable)

And I see pgdriver.WithDSN calls url.Parse() which will cause this error: parse "postgres://postgres:1A": invalid port ":1A" after host

The panic stack is:

panic: parse "postgres://postgres:1A": invalid port ":1A" after host

goroutine 1 [running]:
github.com/uptrace/bun/driver/pgdriver.WithDSN.func1(0xc0001ad040)
	C:/Users/yxzhu/go/pkg/mod/github.com/uptrace/bun/driver/pgdriver@v1.1.16/config.go:191 +0x1d2
github.com/uptrace/bun/driver/pgdriver.NewConnector({0xc00051dc28, 0x1, 0x1})
	C:/Users/yxzhu/go/pkg/mod/github.com/uptrace/bun/driver/pgdriver@v1.1.16/driver.go:76 +0xf1
@bevzzz
Copy link
Contributor

bevzzz commented Nov 29, 2023

As per documentation, the DSN should be a valid URL:

The connection URI needs to be encoded with percent-encoding if it includes symbols with special meaning in any of its parts. Here is an example where the equal sign (=) is replaced with %3D and the space character with %20:

There are many guides online for URL-encoding a string in Golang and this simple change should fix your issue:

password := "invalid#pwd"
pgdriver.WithDSN(fmt.Sprintf("postgres://postgres:%s@localhost:5432/postgres?sslmode=disable", url.QueryEscape(password)))

IMO, ensuring the validity of the URL should be the client code's responsibility.

@zhuyanxi
Copy link
Author

As per documentation, the DSN should be a valid URL:

The connection URI needs to be encoded with percent-encoding if it includes symbols with special meaning in any of its parts. Here is an example where the equal sign (=) is replaced with %3D and the space character with %20:

There are many guides online for URL-encoding a string in Golang and this simple change should fix your issue:

password := "invalid#pwd"
pgdriver.WithDSN(fmt.Sprintf("postgres://postgres:%s@localhost:5432/postgres?sslmode=disable", url.QueryEscape(password)))

IMO, ensuring the validity of the URL should be the client code's responsibility.

Oh, sorry, I made a stupid mistake. I forgot to escape the dsn string.
Everything is fine now, thank you so much. Wish you a good day.

@betrok
Copy link

betrok commented Apr 11, 2024

I feel like there should be safe way to check/use DSN without panic.

Maybe just export parseDSN function so users can check error on theirs end.

@bevzzz
Copy link
Contributor

bevzzz commented Apr 19, 2024

I feel like there should be safe way to check/use DSN without panic.
Maybe just export parseDSN function so users can check error on theirs end.

@betrok here parseDSN panics because the DSN passed by the user is not a valid URL. The underlying error is returned by url.Parse, as this example demonstrates.
There are several things one can already do to avoid the panic:

  • always URL-encode your DSN
  • in case there's a preferred way of handling it, check for the error with _, err := url.Parse(dsn) before passing the DSN along
  • pass all parameters via dedicated functional options (see an example from the docs below)
pgconn := pgdriver.NewConnector(
	pgdriver.WithNetwork("tcp"),
	pgdriver.WithAddr("localhost:5437"),
	pgdriver.WithTLSConfig(&tls.Config{InsecureSkipVerify: true}),
	pgdriver.WithUser("test"),
	pgdriver.WithPassword(`^_^Sp00kyP@$$w0rd#`),  // <-- invalid URL characters here
	pgdriver.WithDatabase("test"),
)

@betrok
Copy link

betrok commented Apr 19, 2024

There are more situations where parseDSN can return error leading to panic other than straight up invalid URL, including certificate verification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants