hi, On Wed, Jul 20, 2011 at 1:50 AM, Scott MacVicar <sc...@macvicar.net> wrote: > Why did you even commit such a change without discussing it?
I maintain this part of the OpenSsl extension and the php windows port. There was a horrible bug and slowdown with this code and I fixed it. Users tested it and are happy with the fix. > I understand your logic for doing this but there was no RFC or discussion > around the impact of this. I do not need to do a RFC for a fix and even less for an extension I maintain. > OpenSSL has been FIPS certified, your change has changed this contract and > it's calling back into a Windows API. Has it been reviewed for correctness? Be serious two and half second. I gave you the explanation in my reply. > Whats the speed difference between OpenSSL and your version. I know you > removed the screen code which was causing a long delay. 300% Now move on, we have other things to do that arguing about this little broken function introduced back then. > > On Jul 19, 2011, at 4:13 PM, Pierre Joye wrote: > >> Why did you not ask in the 1st place before reverting it? >> >> Please don't waste our time with such things. Users expect this >> (tested) fix in the next releases. >> >> Now, openssl has lower minimum windows version support that we do, and >> does all possible things to improve the entropy, which is not required >> nor necessary for the windows we support. >> >> The idea in the 1st place was to have a standard set of random >> functions instead of this, as you well know. Now it is too late and we >> have to live with this function. While the set of random will surely >> come at some point too. >> >> On Wed, Jul 20, 2011 at 1:04 AM, Scott MacVicar <sc...@macvicar.net> wrote: >>> Why isn't this fixed upstream? This is a horrible idea to make core changes >>> like this without a discussion. >>> >>> I'll revert this again so we can at least have the opportunity to discuss >>> this. >>> >>> S >>> >>> On 19 Jul 2011, at 15:55, Pierre Joye <pierre....@gmail.com> wrote: >>> >>>> Please restore that, now. That's not your cup of tea and it is the way >>>> it should have been in the 1st place. >>>> >>>> On Wed, Jul 20, 2011 at 12:29 AM, Scott MacVicar <scott...@php.net> wrote: >>>>> scottmac Tue, 19 Jul 2011 22:29:55 +0000 >>>>> >>>>> Revision: http://svn.php.net/viewvc?view=revision&revision=313455 >>>>> >>>>> Log: >>>>> Revert change to use a special Windows version of >>>>> openssl_random_pseudo_bytes(). >>>>> >>>>> Lets discuss this on internals first. We're advertising something from >>>>> the OpenSSL library >>>>> and then subverting it with another Windows OS call. >>>>> >>>>> What are the implications of this? Should we make this available in >>>>> ext/standard/ instead? >>>>> >>>>> Changed paths: >>>>> U php/php-src/branches/PHP_5_4/ext/openssl/openssl.c >>>>> U php/php-src/trunk/ext/openssl/openssl.c >>>>> >>>>> Modified: php/php-src/branches/PHP_5_4/ext/openssl/openssl.c >>>>> =================================================================== >>>>> --- php/php-src/branches/PHP_5_4/ext/openssl/openssl.c 2011-07-19 >>>>> 22:18:08 UTC (rev 313454) >>>>> +++ php/php-src/branches/PHP_5_4/ext/openssl/openssl.c 2011-07-19 >>>>> 22:29:55 UTC (rev 313455) >>>>> @@ -4930,19 +4930,10 @@ >>>>> >>>>> buffer = emalloc(buffer_length + 1); >>>>> >>>>> -#ifdef PHP_WIN32 >>>>> - strong_result = 1; >>>>> - /* random/urandom equivalent on Windows */ >>>>> - if (php_win32_get_random_bytes(buffer, (size_t) buffer_length) == >>>>> FAILURE){ >>>>> - efree(buffer); >>>>> - RETURN_FALSE; >>>>> - } >>>>> -#else >>>>> if ((strong_result = RAND_pseudo_bytes(buffer, buffer_length)) < >>>>> 0) { >>>>> efree(buffer); >>>>> RETURN_FALSE; >>>>> } >>>>> -#endif >>>>> >>>>> buffer[buffer_length] = 0; >>>>> RETVAL_STRINGL((char *)buffer, buffer_length, 0); >>>>> >>>>> Modified: php/php-src/trunk/ext/openssl/openssl.c >>>>> =================================================================== >>>>> --- php/php-src/trunk/ext/openssl/openssl.c 2011-07-19 22:18:08 UTC >>>>> (rev 313454) >>>>> +++ php/php-src/trunk/ext/openssl/openssl.c 2011-07-19 22:29:55 UTC >>>>> (rev 313455) >>>>> @@ -4926,19 +4926,10 @@ >>>>> >>>>> buffer = emalloc(buffer_length + 1); >>>>> >>>>> -#ifdef PHP_WIN32 >>>>> - strong_result = 1; >>>>> - /* random/urandom equivalent on Windows */ >>>>> - if (php_win32_get_random_bytes(buffer, (size_t) buffer_length) == >>>>> FAILURE){ >>>>> - efree(buffer); >>>>> - RETURN_FALSE; >>>>> - } >>>>> -#else >>>>> if ((strong_result = RAND_pseudo_bytes(buffer, buffer_length)) < >>>>> 0) { >>>>> efree(buffer); >>>>> RETURN_FALSE; >>>>> } >>>>> -#endif >>>>> >>>>> buffer[buffer_length] = 0; >>>>> RETVAL_STRINGL((char *)buffer, buffer_length, 0); >>>>> >>>>> >>>>> -- >>>>> PHP CVS Mailing List (http://www.php.net/) >>>>> To unsubscribe, visit: http://www.php.net/unsub.php >>>>> >>>> >>>> >>>> >>>> -- >>>> Pierre >>>> >>>> @pierrejoye | http://blog.thepimp.net | http://www.libgd.org >>> >> >> >> >> -- >> Pierre >> >> @pierrejoye | http://blog.thepimp.net | http://www.libgd.org > > -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
