[Cache] Add url decoding of password in RedisTrait DSN
#52688
Conversation
|
Why not as a bug fix? This looks like one to me. For 5.4 then. |
| public static function setUpBeforeClass(): void | ||
| { | ||
| if (!getenv('REDIS_CLUSTER_HOSTS')) { | ||
| self::markTestSkipped('REDIS_CLUSTER_HOSTS env var is not defined.'); |
There was a problem hiding this comment.
but we still need the env var to run all the other test cases, don't we?
There was a problem hiding this comment.
Yes, but this env var presence is already checked in testCreateConnection so it can be removed from here
| } else { | ||
| $params['parameters']['password'] = $auth; | ||
| $params['parameters']['password'] = urldecode($auth); |
There was a problem hiding this comment.
everywhere this?
| $params['parameters']['password'] = urldecode($auth); | |
| $params['parameters']['password'] = rawurldecode($auth); |
| 7.1 | ||
| --- | ||
|
|
||
| * Add url decoding of password in `RedisTrait` DSN |
There was a problem hiding this comment.
not needed for bug fixes
There was a problem hiding this comment.
Oh alright, I thought it was considered as a feature. I'll update this as well as the base branch
2764fd7 to
ce06054
Compare
|
I addressed your comments and rebased on 5.4. The fix is different from other branches, I'll provide a fix for those if needed 🙂 |
ce06054 to
1717fca
Compare
|
Thank you @alexandre-daubois. |
No need, that was easy to fix as part of the merge conflict. Note that I didn't change CouchbaseCollectionAdapter nor CouchbaseBucketAdapter because while they have the same issue, they're more severely broken and would deserve a dedicated PR, if anyone is up to have a look ;) |
This PR was merged into the 7.1 branch. Discussion ---------- [Cache] Fix Couchbase password parsing | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT This is the followup for #52688 (comment). Commits ------- 4e6f052 [Cache] Add support for URL encoded characters in Couchbase DSN
This brings support for
@char in passwords