electric icon indicating copy to clipboard operation
electric copied to clipboard

some characters not supported in DATABASE_URL

Open thosil opened this issue 1 year ago • 3 comments

Hi there,

I'm testing the latest version at the time I'm writing this (electric 1.0.0-beta.1-4-geeba768), good job!

But I have an issue, my db password has a ":" inside and it looks like the parser don't like it:

ERROR! Config provider Config.Reader failed with:
** (MatchError) no match of right hand side value: {:error, "has invalid or missing username"}
    (stdlib 6.0.1) erl_eval.erl:652: :erl_eval.expr/6
    /app/releases/1.0.0-beta.1-4-geeba768/runtime.exs:115: (file)
    (elixir 1.17.2) src/elixir.erl:364: :elixir.eval_forms/4
    (elixir 1.17.2) lib/module/parallel_checker.ex:112: Module.ParallelChecker.verify/1
    (elixir 1.17.2) lib/code.ex:572: Code.validated_eval_string/3
    (elixir 1.17.2) lib/config.ex:290: Config.__eval__!/3
    (elixir 1.17.2) lib/config/reader.ex:92: Config.Reader.read!/2

Runtime terminating during boot ({{badmatch,{error,<<"has invalid or missing username">>}},[{erl_eval,expr,6,[{file,"erl_eval.erl"},{line,652}]},{elixir_eval,'__FILE__',1,[{file,"/app/releases/1.0.0-beta.1-4-geeba768/runtime.exs"},{line,115}]},{elixir,eval_forms,4,[{file,"src/elixir.erl"},{line,364}]},{'Elixir.Module.ParallelChecker',verify,1,[{file,"lib/module/parallel_checker.ex"},{line,112}]},{'Elixir.Code',validated_eval_string,3,[{file,"lib/code.ex"},{line,572}]},{'Elixir.Config','__eval__!',3,[{file,"lib/config.ex"},{line,290}]},{'Elixir.Config.Reader','read!',2,[{file,"lib/config/reader.ex"},{line,92}]}]})

Crash dump is being written to: erl_crash.dump...done

It works if I replace the ":" with "%3A" so it's really in the config parser.

thosil avatar Dec 11 '24 10:12 thosil

Hi @thosil.

This is expected behaviour. A URL has a fixed structure for parsing and splitting it into components:

scheme://username:password@host:port/path#hash?query

The separators ://, :, @, /, #, ? have specific syntactic purpose and are used as guides when parsing. As is generally the case with any form of structured input, if any of the components in the input contain a syntactically meaningful character, it needs to be escaped.

So replacing : with the percent-encoded version %3A is exactly what needs to be done for your password.

If you've used https://neon.tech, you might have seen that it uses the account email address as the database username, so it needs to have characters such as + and @ escaped in the database connection URL. For example, here's what Neon's database URL would look like for an account with the email address [email protected]:

postgresql://foo%2Bbar%40gmail.com:**********@foohost.us-east-2.aws.neon.tech/electric?sslmode=require

alco avatar Dec 11 '24 12:12 alco

Hi @alco , thanks for the answer. But imagine you have a deployment where an auto-generated password is set in once and shared between services for some reasons. If I want to use it with electric, I have to make sure it either not contain special characters or to uuencode it for electric. I say for electric because in my test env, I have another service using the same kind of url for connecting the db and... it works like a charm (the variable is directly used by sqlachemy).

But whatever you decide to do with this issue, it would be nice to have another way to specify the credentials (ex: from a file, for docker secrets).

Again thanks for the answer :-)

thosil avatar Dec 11 '24 15:12 thosil

For maximum compatibility you would need to encode the special characters right? Otherwise you'd need to check every tool you use. It seems easier to just encode?

KyleAMathews avatar Dec 12 '24 02:12 KyleAMathews

Yeah, I would say a good rule of thumb is to always percent-encode URI components to avoid any conflict with reserved characters. The worst that'll happen is you'll get a valid URI that doesn't lead to any ambiguity when parsing.

Now, there is room for a deeper discussion on the normative syntax for the userinfo component in PostgreSQL URIs since the official URI spec does not constrain it in any way. But I don't think this repo is the right place for it.

I concur with @thosil on the need to have another way to pass secret credentials to Electric and only using DATABASE_URL to specify the host, port and database name for the connection. I have created a new issue to track this - #2159.

alco avatar Dec 12 '24 14:12 alco

For the sake of argument, I would like to mention that psql works with passwords that contain colons. Its URI parser seems to split on the first colon and treat the rest of userinfo as the password. So this works fine on the terminal:

# PG server config:
#   POSTGRES_USER="postgres"
#   POSTGRES_PASSWORD="password:1:2"

$ psql postgresql://postgres:password:1:2@localhost:54321/postgres
[localhost] postgres:postgres=#

But it is an arbitrary decision. What if you generated usernames in your tests and those contained colons? That wouldn't work:

# PG server config:
#   POSTGRES_USER="postgres:0"
#   POSTGRES_PASSWORD="password:1:2"

$ psql postgresql://postgres:0:password:1:2@localhost:54321/postgres
psql: error: connection to server at "localhost" (::1), port 54321 failed: FATAL:  password authentication failed for user "postgres"

So it's best to make sure you have a valid and unambiguous connection URI with reserved characters percent-encoded before using it.

alco avatar Dec 12 '24 14:12 alco