Skip to content

Removed SecureRandom class #15880

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Removed SecureRandom class #15880

wants to merge 1 commit into from

Conversation

pierredup
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Remove the Symfony\Component\Security\Core\Util\SecureRandom class and use the random_bytes function instead. Also include the paragonie/random_compat library for non PHP 7 installs

Note This PR depends on #15879 to be merged first

@pierredup pierredup mentioned this pull request Sep 23, 2015
Merged
@pierredup pierredup force-pushed the remove_secure_random branch 3 times, most recently from ea7d682 to 2e2b762 Compare September 24, 2015 07:12
@stof
Copy link
Member

stof commented Sep 24, 2015

We cannot remove it from the master branch without deprecating it first in 2.8 according to our policy. Otherwise it makes the upgrade to 3.0 painful

@pierredup
Copy link
Contributor Author

@stof the deprecation PR is pending here #15879

@stof
Copy link
Member

stof commented Sep 24, 2015

Which means this PR should not exist yet (merging the deprecation PR will make this PR unmergeable because of conflicts)

@pierredup
Copy link
Contributor Author

I'll fix the conflicts when the deprecation PR is merged, I also updated the description to say that this is dependant on the deprecation PR to be merged first.
Having this PR open already means I can get quicker feedback and fix any issues without having to wait for the other PR's to be merged first and then only start with the removal.

@stof
Copy link
Member

stof commented Sep 24, 2015

@pierredup most of the work done in this PR would become unnecessary once the deprecation PR is implemented the proper way though. The PR would just remove the class, the secure_random service and the tests for the class. So getting feedback on the current state of this PR is useless.

@pierredup
Copy link
Contributor Author

Okay makes sense, I'll close this for now

@pierredup pierredup closed this Sep 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pierredup @stof @carsonbot