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

fix: URL-Encode Postgres Password #3163

Conversation

michael-genson
Copy link
Collaborator

@michael-genson michael-genson commented Feb 12, 2024

What type of PR is this?

(REQUIRED)

  • bug

What this PR does / why we need it:

(REQUIRED)

There's a bug in Pydantic V2 that doesn't properly URL-encode Postgres passwords. They used to work fine in V1, for whatever reason. This specifically affects passwords with any of these characters, at a minimum: ,#/? (see the linked ticket below).

This PR attempts to create the Postgres URL normally, and if it fails, attempts to URL parse it ourselves. I did it this way in case there's some unintended side-effect, I didn't want to bork any other users. apparently using urllib.parse.quote_plus is the right way to do it: https://stackoverflow.com/questions/1423804/writing-a-connection-string-when-password-contains-special-characters

The related Pydantic bug: pydantic/pydantic#8061

Which issue(s) this PR fixes:

(REQUIRED)

N/A, raised in Discord

Special notes for your reviewer:

(fill-in or delete this section)

Hopefully this covers all use-cases, but worst-case users can choose passwords without problematic characters (though I fully understand why we want to avoid that, since postgres does support them).

Testing

(fill-in or delete this section)

Added a test

@michael-genson michael-genson force-pushed the fix/url-encode-postgres-passwords branch from 6c7fb9c to 8db08c2 Compare February 12, 2024 16:58
@boc-the-git
Copy link
Collaborator

LGTM, good stuff @michael-genson

@boc-the-git boc-the-git merged commit fe3bd95 into mealie-recipes:mealie-next Feb 12, 2024
9 checks passed
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.

None yet

2 participants