Hi all, Following commit is reverted because someone requested "RFC" and/or demand "Merge approval from RM",
http://git.php.net/?p=php-src.git;a=commitdiff;h=48f1a17886d874dc90867c669481804de90509e8 I don't understand the reasoning why it requires RFC nor Merge approval. As we all know, there are many bug fixes without RFC nor discussion. So let's discuss about what bug fix requires RFC and merge approval. The patch fixes "uniqid() is not unique enough" bug. --------------- ==Rationale== uniqid() is simply _broken_ because it does not provide expected uniqueness due to timestamp based php_combined_lcg(). (large warning is added to the manual recently) unique id (time stamp) + entropy (timestamp based entropy) Problems of this implementation - NTP is used to adjust system time. - Result is not reasonably unique by logic. ==Resolution== Replace php_combined_lcg() (Poor PRNG) to php_random_bytes() (CSPRNG). Patch http://git.php.net/?p=php-src.git;a=commitdiff;h=48f1a17886d874dc90867c669481804de90509e8 (less than 30 lines of diff) ==BC impact== There is no BC. php_random_bytes() raises exception for bad CSPRNG, but this very unlikely and such system shouldn't be used anyway. --------------- Discussions in previous thread --------------- =="new uniqid() can cause /dev/urandom cannot read error"== This is FUD. Almost all "/dev/urandom cannot read" issues on the net is due to "open_basedir" restriction. "open_basedir" is nothing to do with internal functions, so the discussion is FUD. =="Any new error is BC so the patch should be reverted"== We do have many bug fixes emit new errors. We don't revert them at all even when it's very common and annoying. Example: https://bugs.php.net/bug.php?id=73238 This bug fix made WordPress display few additional E_WARNING errors that can be remove by php.ini or code fix. =="Someone explicitly requested RFC/Merge approval, so it should be reverted"== IMHO, the patch is very simple patch only fixes problem. Many bug fixes include more severe BC issues than the patch, behavior changes and raised errors. Yet, no RFC nor discussions. If someone requested RFC/Merge request for very simple patch, should we follow always? =="It does not have to hurry"== I agree with this. However, reverting simple bug fix patch does not make sense. -------------- Question is: - What kind of bug fix requires RFC? - What kind of bug fix requires discussion and approval to merge released versions? It does not have to be generic. It's okay for providing reasons why this specific bug fix requires RFC or merge approval. If this bug fix requires RFC or merge approval, almost all bug fixes requires RFC and merge approval. Thank you. P.S. The original discussion is done in "[RFC][DISCUSSION] Improve uniqid() uniqueness" thread. After all, my question is "Should we discuss all bugs before commits?" "The revert is valid and reasonable?" -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
