2016-10-18 14:41 GMT+02:00 Yasuo Ohgaki <yohg...@ohgaki.net>: > Hi Niklas, > > On Tue, Oct 18, 2016 at 9:33 PM, Niklas Keller <m...@kelunik.com> wrote: > > 2016-10-18 14:12 GMT+02:00 Yasuo Ohgaki <yohg...@ohgaki.net>: > >> > >> Hi Niklas, > >> > >> On Tue, Oct 18, 2016 at 9:08 PM, Niklas Keller <m...@kelunik.com> wrote: > >> >> > >> >> As you can see from last minutes discussion. > >> >> > >> >> "/dev/urandom cannot be read" is FUD. > >> >> It's pure bug fix. (I intentionally made patch easy to extend used > >> >> chars, though) > >> >> > >> >> Would you consider revert the revert? > >> > > >> > > >> > This discussion shows there should be a RFC and a vote. I'd not > consider > >> > this a simple bug fix, after all it doesn't really fix it. > >> > > >> > If we want to fix it in core, we'd better include an UUID generation > >> > mechanism than fixing uniq_id. > >> > >> UUID like uniqueness is not the subject of uniqid(), isn't it? > > > > > > UUID = Universally Unique Identifier > > uniqid = Generate a unique ID > > > > Where is uniqueness _not_ the subject of uniqid()? > > > >> > >> As I wrote, it's simple bug fix. > > > > > > The issue is that it doesn't fix it. Maybe it band aids. But it doesn't > fix > > uniqid. > > > > It's exactly why I proposed to better deprecate uniqid. We can do that in > > 7.2 and provide UUIDs as a standardized and superior alternative. > > OK, I understand you prefer to deprecate uniqid(), but I guess > uniqid() deprecation is less likely to be passed than improving > uniqid() uniqueness with a little BC. > > If you search uniqid() usage, you'll see UUID is too much for many > usages. uniqid() has it own use cases. > > Current uniqid() is not unique at all.
Right, and it's impossible to fix it without breaking BC, because really fixing it would require more output. > The patch simply fixes it by > using proper entropy, no BC basically. > It might be fine committing this to master. But as you say, uniqid is broken and I'd not consider it fixed with just changing the source of entropy but leaving the output as is. > What's wrong with this? > Committing it directly to a frozen branch is. Regards, Niklas --------------- > 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? > --------------- > > -- > Yasuo Ohgaki > yohg...@ohgaki.net >