Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
Deprecate the SecureRandom class#15879
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
8018c29 to00e5013CompareThis implementation is broken: the Symfony codebase still use the deprecated function, meaning it triggers deprecation warnings for everyone (and this is why the testsuite is failing) |
@stof What would be the best way to deprecate this class then? Should the dependency on |
@pierredup yes, this is the way it should be done. A deprecated API should not be used anymore. If you cannot avoid using it, it cannot be a deprecated API. |
| @@ -33,7 +33,8 @@ | |||
| "symfony/http-foundation": "", | |||
| "symfony/validator": "For using the user password constraint", | |||
| "symfony/expression-language": "For using the expression voter", | |||
| "ircmaxell/password-compat": "For using the BCrypt password encoder in PHP <5.5" | |||
| "ircmaxell/password-compat": "For using the BCrypt password encoder in PHP <5.5", | |||
| "paragonie/random_compat": "For secure number generation" | |||
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, we should document that this library is only a compatibility layer for PHP 5:"For secure random number generation in PHP 5.x"
Status: Needs Work |
b527c03 to5077d83CompareI would like to see our testsuite being fixed and then rerunning the builds for this PR, to ensure things work properly though. |
Can the build for this be re-run? Or should I do a push to trigger new builds? |
@pierredup there are conflicts preventing the merge, so this needs a rebase anyway. After pushing the rebased version, a new build with run. |
44a1d43 to0d781d4Compare| @@ -12,6 +12,7 @@ CHANGELOG | |||
| `Symfony\Component\Security\Http\Authentication\SimpleFormAuthenticatorInterface` instead | |||
| * deprecated `Symfony\Component\Security\Core\Util\ClassUtils`, use | |||
| `Symfony\Component\Security\Acl\Util\ClassUtils` instead | |||
| * deprecated `Symfony\Component\Security\Core\Util\SecureRandom` class in favour of the `random_bytes` function | |||
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.
deprecated the [...]
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.
in favor of
0c50c1f to5bb8732CompareThanks for the info@stof |
All comments have been addressed |
5bb8732 toee3f67aCompareStatus: Reviewed |
@pierredup You also have to update the |
Status: Needs work |
Thanks@xabbuh. Is it still necessary for the test, since the SecureRandom class is now basically only a wrapper for the |
Imo you can simply drop the OpenSSL related part of that test. |
ee3f67a to1afe984CompareStatus: Reviewed |
👍 ping @symfony/deciders |
| * each token (in bits) | ||
| * Note: The $random parameter is deprecated since version 2.8 and will be removed in 3.0. | ||
| * | ||
| * @param SecureRandomInterface|int|null $random The random value generator used for |
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.
the deprecated parameter value should not be documented anymore
👍 apart from the above comments |
should also be rebased to resolve conflict. |
1afe984 tof02973aCompareDone |
Thank you@pierredup. |
This PR was merged into the 2.8 branch.Discussion----------Deprecate the SecureRandom class| Q | A| ------------- | ---| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Deprecate the `Symfony\Component\Security\Core\Util\SecureRandom` and encourage the usage of the `random_bytes` function insteadCommits-------f02973a Deprecate the SecureRandom class
Deprecate the
Symfony\Component\Security\Core\Util\SecureRandomand encourage the usage of therandom_bytesfunction instead