Skip to content

[Cache] Fix Couchbase password parsing #52758

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
Dec 30, 2023

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Nov 27, 2023

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

This is the followup for #52688 (comment).

@alexandre-daubois alexandre-daubois force-pushed the couchbase branch 5 times, most recently from 7095459 to ed359c0 Compare November 28, 2023 09:04
@alexandre-daubois
Copy link
Member Author

What about deprecating the Couchbase Bucket adapter in another PR? It is only compatible with Couchbase 2, which conflicts with this adapter compatible with Couchbase 3

@alexandre-daubois
Copy link
Member Author

alexandre-daubois commented Dec 9, 2023

After discussing with @nicolas-grekas, the following changes have been made:

  • Replaced $parsed by $params to match other DSN parsing
  • Target 7.1 instead of 5.4

Note: rawurldecode() is still needed even after parse_url, which doesn't decode HTML chars. I'll send another PR to deprecate the CouchbaseBucketAdapter, only compatible with Couchbase 2 (which conflicts with this adapter requiring Couchbase 3 and making the other one not possible to test on the CI).

@alexandre-daubois
Copy link
Member Author

Rebased and conflicts fixed. All green 🙂

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.

what a mess parse_url...
so we should now add rawurldecode on all user/pass in the codebase that use parse_url, isn't it?

@alexandre-daubois
Copy link
Member Author

It seems 😕 https://3v4l.org/eshPt

I'll have a look at it. Is this something that also should target 7.1? I'd say yes.

@nicolas-grekas nicolas-grekas modified the milestones: 5.4, 7.1 Dec 30, 2023
@nicolas-grekas
Copy link
Member

Thank you @alexandre-daubois.

@nicolas-grekas nicolas-grekas merged commit 8c5881e into symfony:7.1 Dec 30, 2023
nicolas-grekas added a commit that referenced this pull request Jan 2, 2024
…fier][Translation] Url decode username and passwords from `parse_url()` results (alexandre-daubois)

This PR was merged into the 5.4 branch.

Discussion
----------

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

| 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.

Commits
-------

f0292f2 [Cache][DependencyInjection][Lock][Mailer][Messenger][Notifier][Translation] Url decode username and passwords from `parse_url()` results
symfony-splitter pushed a commit to symfony/cache that referenced this pull request Jan 2, 2024
…fier][Translation] Url decode username and passwords from `parse_url()` results (alexandre-daubois)

This PR was merged into the 5.4 branch.

Discussion
----------

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

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

Following symfony/symfony#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.

Commits
-------

f0292f23be [Cache][DependencyInjection][Lock][Mailer][Messenger][Notifier][Translation] Url decode username and passwords from `parse_url()` results
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.

3 participants