[DoctrineBridge] fix case sensitivity issue in RememberMe\DoctrineTokenProvider #27314
Conversation
PF4Public
commented
May 19, 2018
•
| Q | A |
|---|---|
| Branch? | 2.8 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | ? |
| Fixed tickets | #21467 |
| License | MIT |
| Doc PR | - |
|
Hi @PF4Public, thanks for you PR. Would you mind rebasing/retargetting for branch 2.7? 2.3 is not maintained anymore and 2.7 will go out of maintenance in 10 days btw) |
|
Hi @nicolas-grekas |
|
We now need to figure out how not to break existing apps. |
|
Should I rebase it against newest version, which allows BC breaks? |
|
We always strive for creating a continuous upgrade path so that there is never a hard BC break. This means e.g. triggering a deprecation notice when applicable, etc. |
|
How can we move forward here? |
|
Since this change could in fact break existing installations, it probably needs to be done in two steps: with deprecation notice first. |
|
@nicolas-grekas , Thank you for fixing this. You've found a really clever way! |
| @@ -50,15 +50,16 @@ public function __construct(Connection $conn) | |||
| */ | |||
| public function loadTokenBySeries($series) | |||
| { | |||
| $sql = 'SELECT class, username, value, lastUsed' | |||
| // the alias for lastUsed works around case insensitivity in PostgreSQL | |||
| $sql = 'SELECT class, username, value, lastUsed as last_used' | |||
|
Thank you @PF4Public. |
…DoctrineTokenProvider (PF4Public) This PR was merged into the 2.8 branch. Discussion ---------- [DoctrineBridge] fix case sensitivity issue in RememberMe\DoctrineTokenProvider | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | ? | Fixed tickets | #21467 | License | MIT | Doc PR | - Commits ------- 0248d4f [DoctrineBridge] fix case sensitivity issue in RememberMe\DoctrineTokenProvider
…h postgres connection (GromNaN) This PR was merged into the 5.4 branch. Discussion ---------- [DoctrineBridge] Add integration test for RememberMe with postgres connection | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT Related to - #27314 - #58523 Commits ------- ca417c0 Add integration test for RememberMe with pg connection