On 10/30/15 2:36 PM, Anatol Belski wrote:
Hi Anthony,

-----Original Message-----
From: Anthony Ferrara [mailto:ircmax...@gmail.com]
Sent: Friday, October 30, 2015 11:58 AM
All,

On Tue, Oct 20, 2015 at 11:35 PM, Anatol Belski <anatol....@belski.net>
wrote:
Could php_random_bytes() be extended with a flag that would tell exceptions
to pass? Or maybe exceptions could be moved out from the
php_random_bytes() and thrown directly in the new functions that was
undergoing the RFC but not password_hash() ? I'd be actually for the second
variant.

That ways FAILURE would be returned, but a decision about what to do about
it would be made outside. IMHO it were also useful as the API is intended to be
exported. So for the cases where php_random_bytes() came to use as a library
function outside immediate PHP userspace (for example in SAPI), it should not
throw exceptions from itself.

Do this suggestions sound eligible?


I have created a PR for this: https://github.com/php/php-src/pull/1614

It changes the public API of the internal php_random_bytes function to have a
third "should_throw" parameter, as well as defining two macros:
php_random_bytes_throw(dest, len) and php_random_bytes_silent(dest, len).

If this looks acceptable, it can be pulled into 7.0, and then the move for
password_hash can be done at a later date.

IMHO this looks fine for 7.0. The API is new and was not exported, so no ABI 
breach and no behavior change when used for internal refactoring.

This change to php_random_bytes() should make it easier to migrate various old stuff (e.g. Leigh proposed sessions) to the new source, and I like it for that.

But silenced failure should probably not be used for anything new and it would be good if a developer refactoring an old API would comment how the new code processes FAILURE together with a justification. Could comments be included in this PR to encourage it? As it stands, this change allows hazardous use without mentioning it.

Tom

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

Reply via email to