Yasuo, > -----Original Message----- > From: Yasuo Ohgaki [mailto:yohg...@ohgaki.net] > Sent: Tuesday, October 18, 2016 9:53 PM > To: Anatol Belski <anatol....@belski.net> > Cc: Joe Watkins <pthre...@pthreads.org>; Niklas Keller <m...@kelunik.com>; > Leigh <lei...@gmail.com>; PHP Internals <internals@lists.php.net> > Subject: Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness > > Hi Anatol, > > On Wed, Oct 19, 2016 at 1:41 AM, Anatol Belski <anatol....@belski.net> wrote: > > AFM the patch is not acceptable for 7.0. It is true that some place was > > moved > to the new random int functionality (in password AFAIR). But, it is done at > the > place and the way that a BC breach is unlikely. Using the throwing variant is > for > sure a BC breach, but also the way pushing while being explicitly asked to go > through an RFC, is inappropriate. As the new random_* functions are available > and allow to implement the best possible uniqueness in user land, changing the > algorithm of the existing uniqid() doesn't look to have a valid base. > > > > Any additional error could be BC. It's the fact. > > However, your sentence does not make sense at all. > Do we revert any error emitting bug fix? No, not at all. > As far as I remember, uniqid() was never meant to be cryptographically safe. It is documented. Indeed systems might be too fast for microseconds based function nowadays. In 7.0, my simple exercise - substr(md5(random_bytes(8)), 0, 13) which does same in the way you want it. We are talking about a oneliner of new code vs. an old function that is guaranteed in use at any possible places.
> We do add errors as normal bug fix process. Many of them are w/o RFC, even > w/o discussion. > > Example: https://bugs.php.net/bug.php?id=73238 > This bug fix caused WordPress caused 3 additional E_WARNING displayed that > can be remove by php.ini or code fix. > As a reminder - there's no global rule about functions throwing exceptions, so it is not done by default. Except a couple of new places, no function throws an exception. The place in password salt code, that was migrated to the new randomness, did already depend on /dev/urandom and others. However, even it's based on the new functionality, the old behavior is kept and it is done intentionally. > Which is important? > - uniqid() is not unique > - Really broken system that shouldn't be used may emit error > > "/dev/urandom cannot read discussion" is FUD and irrelevant to this > discussion. > Issues with user script random_bytes() implementation or like does not apply > to > uniqid() fix. > But your implementation indeed uses another API that has other impacts. Php_random_bytes is crossplatform, there can be various errors on various platforms. That's the concern as I'd understand it. > > Anyway, are we going to revert anything emit new errors from now on because > it's BC? > Are we going to require RFC for this kind of very simple and reasonable fix? > I hope not. > > IMHO my discussion is logical. Please consider revert the revert. > Otherwise, we cannot fix even simple bugs. > No, IMHO you overdo it a bit. Of course it is acceptable with errors, warning, etc. where it makes sense. But it needs a base and a balance also in other areas for usability, performance, BC, language consistency, etc. If one were telling, it's impossible to do it in PHP - but there are functions in PHP 7, that provide the functionality aimed. Yes, there is also some legacy functionality, so should everything be moved to cryptographically safe? The answer is obviously - no. For crypto there are dedicated functions and extensions there. Besides that, you see many other people opposing this change. An RFC were the way to target the PHP version you want, even 7.0. As for me, I'd likely vote yes for master, if the throwing part were replaced. Regards Anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
