Skip to content

Transition from mt_rand to random_int #9903

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

Closed
wants to merge 2 commits into from

Conversation

sander-bol
Copy link

Considering that mt_rand has a warning notice surrounding it, having the examples use random_int nudges people in the direction of the more secure way of generating random ints, building better habits.

Sander Bol added 2 commits June 9, 2018 09:49
See: http://php.net/manual/en/function.mt-rand.php
```
Caution
This function does not generate cryptographically secure values, and should not be used for cryptographic purposes. If you need a cryptographically secure value, consider using random_int(), random_bytes(), or openssl_random_pseudo_bytes() instead.
```

Of course, this example is not doing anything "cryptographically secure". However, it does seem to make sense to promote using the more secure way of generating random numbers, to nudge people in the right direction if they do start building cryptographically sensitive stuff.
@javiereguiluz
Copy link
Member

@sbol-coolblue this is a really nice proposal. I like it a lot! In case we're worried because random_int() is for PHP 7.0+ only ... according to latest Composer stats 80% of developers use PHP 7.0+ versions. Also, PHP 7.0 polyfill provides this function.

So I propose to merge this in 2.8 branch. There's no need for you to rebase or change anything. We can change it when merging. Thanks!

@javiereguiluz
Copy link
Member

Merged! (in 2.8 branch). Thank you Sander ... and congrats on your first Symfony Docs contribution!

javiereguiluz added a commit that referenced this pull request Jun 9, 2018
This PR was submitted for the master branch but it was squashed and merged into the 2.8 branch instead (closes #9903).

Discussion
----------

Transition from mt_rand to random_int

Considering that mt_rand has a warning notice surrounding it, having the examples use random_int nudges people in the direction of the more secure way of generating random ints, building better habits.

Commits
-------

2e68ee7 Transition from mt_rand to random_int
javiereguiluz added a commit that referenced this pull request Jun 10, 2018
This PR was merged into the 2.8 branch.

Discussion
----------

[Filesystem] Improved the code of an example

This continues #9903 and removes the last `mt_rand()` example in the docs.

Commits
-------

35e98e6 [Filesystem] Improved the code of an example
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.

3 participants