Skip to content

use more entropy with uniqid() #57697

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
Jul 10, 2024
Merged

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Jul 9, 2024

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

It looked like using uniqid() without opting for more entropy slipped in after #20132 and #20137.

@xabbuh xabbuh requested review from dunglas and OskarStark as code owners July 9, 2024 20:58
@carsonbot carsonbot added this to the 5.4 milestone Jul 9, 2024
@fabpot
Copy link
Member

fabpot commented Jul 10, 2024

Thank you @xabbuh.

@fabpot fabpot merged commit c2b0d72 into symfony:5.4 Jul 10, 2024
10 of 13 checks passed
@xabbuh xabbuh deleted the uniqid-more-entropy branch July 10, 2024 06:20
nicolas-grekas added a commit that referenced this pull request Jul 15, 2024
This PR was merged into the 6.4 branch.

Discussion
----------

use more entropy with `uniqid()`

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

following #57697 on the `6.4` branch

Commits
-------

b91e8a2 use more entropy with uniqid()
@natewiebe13
Copy link
Contributor

@xabbuh just an observation; looking back at the linked PRs, the original motivation appears to be an optimization as PHP used to skip the usleep(1) if more entropy was set to true. As of PHP 7.2, the function appears to always wait until the next microsecond using a do..while. So if the primary motivation is still performance, and more entropy isn't actually required, it should be less CPU cycles to not have more entropy.

Ref: https://github.com/php/php-src/blob/master/ext/standard/uniqid.c

@xabbuh
Copy link
Member Author

xabbuh commented Jul 17, 2024

@natewiebe13 Thank you for the hint. I wasn't aware of it, but this was indeed improved in php/php-src@d25049c.

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.

4 participants