Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpFoundation] Generate url-safe hashes for signed urls#59022
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ec97e3a to93eb218Comparevaltzu commentedNov 27, 2024
|
stof commentedNov 28, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There is 2 errors in fabbot, which need to be treated differently:
|
stof commentedNov 28, 2024
Fixing the deps=low tests is probably a matter of bumping the min requirement |
| ], | ||
| "require": { | ||
| "php":">=8.2", | ||
| "ext-sodium":"*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Making HttpFoundation require ext-sodium is a no-go (especially if only the UriSigner needs it as it is not the main feature of the component)
| privatefunctioncomputeHash(string$uri):string | ||
| { | ||
| returnbase64_encode(hash_hmac('sha256',$uri,$this->secret,true)); | ||
| returnsodium_bin2base64(hash_hmac('sha256',$uri,$this->secret,true),SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Introducing a dependency on ext-sodium just to provide URL-safe base64 encoding is clearly not necessary. It is very easy to do it in userland (using the samestrtr you use for the comparison)
5782c78 to4b6515eCompare4b6515e to8258bc4Compare8258bc4 toa533750Comparefabpot commentedDec 18, 2024
Thank you@valtzu. |
dd061aa intosymfony:7.3Uh oh!
There was an error while loading.Please reload this page.
This PR was merged into the 6.4 branch.Discussion----------[HttpKernel] relax assertions on generated hashes| Q | A| ------------- | ---| Branch? | 6.4| Bug fix? | no| New feature? | no| Deprecations? | no| Issues || License | MITfixes the high deps tests after#59022Commits-------85a5d13 relax assertions on generated hashes
Uh oh!
There was an error while loading.Please reload this page.
Generate url-safe signed urls since some clients can't handle parameter encoding correctly and are sending
%2Bas+which PHP reads as(space), making the signature invalid.Backwards-compatibility is included, and planned to be removed in 8.0 – which will mean all signed urls generated with <7.3 are rendered invalid.