Hi Kalle and all,

On Wed, Oct 19, 2016 at 1:43 AM, Kalle Sommer Nielsen <ka...@php.net> wrote:
> 2016-10-18 18:41 GMT+02:00 Anatol Belski <anatol....@belski.net>:
>> 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.
>
> I must add, despite not following the discussion entirely, that it
> should also be approved by the two 7.1 RMs to be committed,
> considering we are in RC4 stage at this point and I don't think we
> should just commit things this late without the RM consent to it.

This is usually I do. You'll see my mails discussing which branches to
merge that is not simple. For almost all bug fixes, I do not see
discussion for merging released branchs.

(Following questions are not for Kalle)

Most bug fixes are not discussed at all here.
What is making this simple bug special?

What's wrong with this simple fix?
What makes this a special requires RFC?
---------------
The patch committed is pure bug fix.

uniqid() is simply _broken_ because it does not provide expected uniqueness due
to timestamp based php_combined_lcg(). (I added large warning to the manual
recently, though)

unique id (time stamp) + entropy (timestamp based entropy)

Who argue result is reasonably unique?
Who don't use NTP to adjust system time?
---------------

If any new errors cannot be tolerated with bug fix, are we going to
revert any bug fixes with new error?
Besides, "uniqid() will emit error because it uses /dev/urandom" is
FUD, isn't it?

If there is no reasonable / logical answers for these,
The patch should be included PHP 7.0 and up.


BTW, who really think the patch is offending patch to be merged to
released branches?
Please raise your hand now. I don't think there are many.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to