Skip to content
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

composer dependencies #1

Open
jnorell opened this issue Dec 23, 2021 · 2 comments · May be fixed by #2
Open

composer dependencies #1

jnorell opened this issue Dec 23, 2021 · 2 comments · May be fixed by #2

Comments

@jnorell
Copy link

jnorell commented Dec 23, 2021

Hello,

I'm evaluating this for a simple TOTP implementation, and in looking at the composer dependencies, this requires php7.3 and also your php-random-data class, which itself uses mcrypt and hence is incompatible with php7.2 and above, where mcrypt was removed. Is that simply an error in the composer dependency, and this might actually work with php7.1 or lower?

Thanks...

@jnorell
Copy link
Author

jnorell commented Dec 23, 2021

Sorry, was reading in too much hurry on too small of a screen, I now see that php-random-class does check if the mcrypt function exists and is effectively a wrapper for random_bytes() for php7.0 and up.

As the minimum php version in composer.json is 7.3, it might make more sense to just use random_bytes directly?

@jnorell jnorell linked a pull request Dec 23, 2021 that will close this issue
@pedrosancao
Copy link
Owner

Sorry for taking a long time to answer and thanks for your contribution.

I really didn't take a look on this lib since the initial release but this compatibility issue is crucial, tho I'd like to test myself before merging. Hope this won't take long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants