Skip to content

fix: when pg password contains a $, subsituing variables crashes - #116

Open
f-necas wants to merge 2 commits into
mainfrom
fix-pgdsn-pwd
Open

fix: when pg password contains a $, subsituing variables crashes#116
f-necas wants to merge 2 commits into
mainfrom
fix-pgdsn-pwd

Conversation

@f-necas

@f-necas f-necas commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@f-necas
f-necas requested a review from pmauduit August 26, 2026 11:10

@pmauduit pmauduit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am probably missing the whole project context, but I don't really get why we need to escape characters in the username and password using urllib.parse.quote ? These are environment variables, not URLs ? also why only username and password and not the other ones ?

So, wondering if the only line 267 should suffice here ?

I am afraid I won't be able to review it at this point.

@f-necas

f-necas commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@pmauduit The probability that other variables contain a $ is much lower.

Safe substitute wasn't enough as special characters were misunderstood by PostgresDsn.build().

@pmauduit

Copy link
Copy Markdown
Member

My concern is that we have to recursively do the variable resolution in our code (I mean, it is surprising, I think that spring boot does it, and I think whatever framework being used might decide to manage this as a configuration feature, but I am perplexed as to why we would have to do it in our own code and/or if this is a good thing).

These are environment variables, we should have full control over them, but trying to recursively parse and resolve variables is IMHO out of scope and error prone.

Safe substitute wasn't enough as special characters were misunderstood by PostgresDsn.build().

I had the same issue deploying the DFv2, as I had to have the DSN already prepared in a kubernetes secret, but for me, the fact that unencoded special chars would fail would have revealed a misconfiguration from my side, easily fixed by presenting a secret/env variable correctly URL-encoded. e.g. My responsibility to provide a working configuration.

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

Successfully merging this pull request may close these issues.

2 participants