-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[Messenger][Notifier] use more entropy with random_bytes() #57872
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
Conversation
xabbuh
commented
Jul 29, 2024
Q | A |
---|---|
Branch? | 7.2 |
Bug fix? | no |
New feature? | no |
Deprecations? | no |
Issues | #57856 (comment) |
License | MIT |
based on the PR feedback I chose to pick Alexander's suggestion from #57588 |
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.
I think, switching to UUIDs for truly collision-free identifiers is a good call. Thank you for this PR.
src/Symfony/Component/Messenger/Bridge/Redis/Transport/Connection.php
Outdated
Show resolved
Hide resolved
ab424d5
to
2eadda7
Compare
is it actually worth introducing a dependency to generate this (mostly internal) message identifier ? To me, using |
We need random and collision-free identifiers, so we're pulling a dependency that does exactly that. This sounds absolutely reasonable to me. |
|
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.
I think I agree with @stof, no need for the dep to dedup messages.
But I'd go with less entropy and a shorter hash (using 9 to generate tn trailing =
with base64):
base64_encode(random_bytes(9))
src/Symfony/Component/Messenger/Bridge/Redis/Tests/Transport/ConnectionTest.php
Outdated
Show resolved
Hide resolved
Thank you @xabbuh. |