Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96444
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAJW__o3ZvFSmkjfL5s+2_kth=ZOnHFJKgRH58wadjKtCiRZC-w@mail.gmail.com>
References: <CAGa2bXYg1kkoYGQNr8T2oTshuqrvweefYO+EdY41w1vwUM32iQ@mail.gmail.com>
 <CAGa2bXZLB6SsC3FQyntaDy93YfG5rDigJH6GpC5dF7MZJuJ=LQ@mail.gmail.com>
 <CAGa2bXaM_rSbmSx_bPE4erG6ok1KLdXSqnJP5ngxUNBsrzu0pw@mail.gmail.com>
 <CAPwsocHhsm9Zrh=rpF=Pbct6t5wKfAa-wirGfGjmxV8pi4nttw@mail.gmail.com>
 <CAGa2bXaMMaZn1GyrnT6Bgz3xCMDkFs+YNwHoNZe_1ge5LjLPDg@mail.gmail.com>
 <CAPwsocHXhpQNCYXVN0DHj77M0cA3eAgKuStOPquNUjx=46W1Mw@mail.gmail.com>
 <CAGa2bXaEyd+Qcc7EAQwTEVJY-zD_cK37AYXfM_534YesBG0xOA@mail.gmail.com>
 <CAPwsocHqbeYw=2S6EZG+5zUEWMSM5V2OoG9uLO05sbQffJ1GPw@mail.gmail.com>
 <CAGa2bXZGWFW1gXx4uuAehw0MGADWPMXVuD7wsSJuAQ6f6HJv6A@mail.gmail.com>
 <CAGa2bXZ1wGO_x4z5agJ0+uWX4xfuOYX1or4KziwbMa9v_+-JxA@mail.gmail.com>
 <CANUQDCh9RSnOs__c4crYsH44z0L-mHkTTCgAn-d1bSgH09yL0w@mail.gmail.com>
 <CAGa2bXbDR4EnSU0UUMQR4OGaEAJGPkf7s9M+56CyCPvJ=XSbUQ@mail.gmail.com>
 <CANUQDChPsJHZ73=ke_yo+Pbu2B8rRcFrUYZWJiD5TrwECY4hEA@mail.gmail.com>
 <CAGa2bXZ98ZKER2hRp=2pvTYGVLGXFzkdefFfoERojgQvarHUBw@mail.gmail.com>
 <CAGa2bXZWj21vNVaoWHwUYxp_Zw0_eR4sSwReUC4qBn_6oqEJmg@mail.gmail.com>
 <CAGa2bXbcWQm_AXeSxB-BSRjwYD1de8NKvD+6-J+6q5Ru3HmRVg@mail.gmail.com>
 <070001d2295e$76b7d730$64278590$@belski.net> <CAJW__o3ZvFSmkjfL5s+2_kth=ZOnHFJKgRH58wadjKtCiRZC-w@mail.gmail.com>
Date: Wed, 19 Oct 2016 05:10:45 +0900
Message-ID: <CAGa2bXbbgVqswZU5t=ox03zyzjVeKd-e7ZD9MLNz6bwPaDj3=Q@mail.gmail.com>
To: Kalle Sommer Nielsen <kalle@php.net>
Cc: Anatol Belski <anatol.php@belski.net>, Joe Watkins <pthreads@pthreads.org>, 
	Niklas Keller <me@kelunik.com>, Leigh <leight@gmail.com>, 
	PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Kalle and all,

On Wed, Oct 19, 2016 at 1:43 AM, Kalle Sommer Nielsen <kalle@php.net> wrote=
:
> 2016-10-18 18:41 GMT+02:00 Anatol Belski <anatol.php@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 d=
one at the place and the way that a BC breach is unlikely. Using the throwi=
ng variant is for sure a BC breach, but also the way pushing while being ex=
plicitly 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
yohgaki@ohgaki.net