Skip to content

Conversation

@mvorisek
Copy link
Member

improve redis token entropy and make the token hexadecimal (easier to debug)

@willemstuursma
Copy link
Contributor

Thank you. This does not improve the entropy of the token.

I'll take a PR that uses binhex() on the binary data, I understand that it is useful to have a human readable token.

@mvorisek
Copy link
Member Author

mvorisek commented Jan 6, 2021

@willemstuursma

  • random_bytes is supposed to be good enough even cryptographically safe entropy. Adding a microtime, which we READ ANYWAY a few lines before, can not worse it.
  • if binhex() on binary data is ok with you, then hashing the input to hex should be fine too
@willemstuursma
Copy link
Contributor

Hi @mvorisek.

There is no reason to add the microtime to the input of the hashing function.
There is no reason to use a hashing function instead of converting the random bytes to a human readable value directly.

A change should have a good reason behind it. Else it the change is arbitrary and I prefer my own over yours.

I already explained that a PR that does binhex() only is acceptable to me. That has a clear reason, e.g. to make the tokens human readable when debugging.

@mvorisek
Copy link
Member Author

mvorisek commented Jan 11, 2021

@willemstuursma ok, updated

@willemstuursma
Copy link
Contributor

Thanks.

It is not breaking locking compatibility between versions, so it can go in a minor release.

@willemstuursma willemstuursma merged commit 8b254ed into php-lock:master Jan 11, 2021
@mvorisek mvorisek deleted the better_lock_random branch January 11, 2021 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants