Skip to content

[Cache][DependencyInjection][Lock][Mailer][Messenger][Notifier][Translation] Url decode username and passwords from parse_url() results #53320

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

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Dec 30, 2023

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

Following #52758 (review)

I guess this doesn't an entry in the changelog, does it? I replaced urldecode by rawurldecode and $parsedDsn by $params, as latest updates to the code used the latter.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Thanks for having a look.
Let's go with a bugfix on this one to me.

@OskarStark
Copy link
Contributor

And again a DSN component would help us across the whole codebase

I know this was discussed quite often in the past. Anything changed your mind @nicolas-grekas ?

cc @Nyholm

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 30, 2023

All this makes me think we need a working url parser... (a dsn component is another story.)
(and I don't know where I'd put this parser...)

@OskarStark
Copy link
Contributor

To be reusable in a dedicated component ofc, but as this is util and no "domain", I don't know either 🤷‍♂️ but it sounds good 👍

@alexandre-daubois
Copy link
Member Author

alexandre-daubois commented Dec 30, 2023

Maybe this UrlParser could belong to HttpFoundation?

If this is something that could happen, I'll gladly implement this and leverage it in the numerous DSN parsing processes the codebase counts.

@alexandre-daubois alexandre-daubois changed the base branch from 7.1 to 5.4 December 31, 2023 09:34
@alexandre-daubois
Copy link
Member Author

Rebased to 5.4 to make it a bug fix. I'll have a look at what can be done with some UrlParser/DsnParser to have a global logic over every DSN parsing

@nicolas-grekas nicolas-grekas modified the milestones: 7.1, 5.4 Jan 2, 2024
…lation] Url decode username and passwords from `parse_url()` results
@nicolas-grekas
Copy link
Member

Thank you @alexandre-daubois.

@nicolas-grekas nicolas-grekas merged commit 26ad649 into symfony:5.4 Jan 2, 2024
@fabpot fabpot mentioned this pull request Jan 30, 2024
This was referenced Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants