All,
I'd like to move the conversation towards a decision regarding PRs
1397 and 1398. These decisions are blocking random_compat as well as a
security enhancement to random_bytes (merge conflicts are the
worst).
Here's a quick recap
Arguments:
- Consistency is more important than security.
random_* should be consistent with the rest of PHP (sans
intdiv()
).
They should return false and emit anE_WARNING
orE_ERROR
warning (the latter is if we want it to fail closed).It's the responsibility of the developer to know this can
happen and explicitly check for it. Don't throw anything.
- Security is more important than consistency.
Placing more responsibility on the developer increases the
likelihood of an implementation error. We should aim for compatible
usability withrand()
andmt_rand()
forrandom_int()
, which never return
false. Forrandom_int()
andrandom_bytes()
, should PHP be unable
to generate a random value (no random device available, file
descriptor exhaustion, etc.) an exception should be thrown. If the
developer wants to handle it gracefully, they can place it in try/catch
blocks (which raising errors make messy). Otherwise, the default
state is to fail closed (i.e. terminate script execution).
Open Questions:
- Under what conditions should an Exception be thrown, and which
should throw an Error instead?
Did I miss any?
I'm in favor of throwing something. As for the particulars of what
should be an Exception and what should be an Error, I don't have a
horse in this race. Exceptions already existed and Errors were already
accepted in the Throwable RFC, so I don't believe this warrants
another RFC and putting this decision off until 7.1.
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises <https://paragonie.com
All,
I'd like to move the conversation towards a decision regarding PRs
1397 and 1398. These decisions are blocking random_compat as well as a
security enhancement to random_bytes (merge conflicts are the
worst).
Here's a quick recap
Arguments:
- Consistency is more important than security.
random_* should be consistent with the rest of PHP (sans
intdiv()
).
They should return false and emit anE_WARNING
orE_ERROR
warning (the latter is if we want it to fail closed).It's the responsibility of the developer to know this can
happen and explicitly check for it. Don't throw anything.
- Security is more important than consistency.
Placing more responsibility on the developer increases the
likelihood of an implementation error. We should aim for compatible
usability withrand()
andmt_rand()
forrandom_int()
, which never return
false. Forrandom_int()
andrandom_bytes()
, should PHP be unable
to generate a random value (no random device available, file
descriptor exhaustion, etc.) an exception should be thrown. If the
developer wants to handle it gracefully, they can place it in try/catch
blocks (which raising errors make messy). Otherwise, the default
state is to fail closed (i.e. terminate script execution).
Open Questions:
- Under what conditions should an Exception be thrown, and which
should throw an Error instead?
Did I miss any?
I'm in favor of throwing something. As for the particulars of what
should be an Exception and what should be an Error, I don't have a
horse in this race. Exceptions already existed and Errors were already
accepted in the Throwable RFC, so I don't believe this warrants
another RFC and putting this decision off until 7.1.
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises https://paragonie.com
--
We also -need- a consensus in place because we have more security sensitive
features targeting 7.1 and we don't want to have this argument again.
Throwing exception's may not be consistent right now, but it certainly
should be in the future. The transition to a new consistency has to start
somewhere.
Scott, could you setup a RFC with a vote, so we can decide?
Nikita proposed those two options:
- Error is to be used in cases where an error is attributable to
programmer mistake.
- Error signifies a failure condition that the programmer is discouraged
(and unlikely to want) to handle. It should only be dealt with at the top
level.
I'm in favor of 2), I would phrase it like: Error should be used if a
repetitive call with the same input parameters would permanently result
in a failure, otherwise Exception.
Regards, Niklas
Scott, could you setup a RFC with a vote, so we can decide?
Nikita proposed those two options:
- Error is to be used in cases where an error is attributable to
programmer mistake.
- Error signifies a failure condition that the programmer is discouraged
(and unlikely to want) to handle. It should only be dealt with at the top
level.I'm in favor of 2), I would phrase it like: Error should be used if a
repetitive call with the same input parameters would permanently result
in a failure, otherwise Exception.Regards, Niklas
I’m in favor of 1), as this was my original intention of the Error branch of
exceptions. Errors should be attributable to a programming mistake that should
be corrected. Exception should be thrown for unexpected conditions that are not
due to programmer error, but instead things like IO or permission errors. I think
this is how exceptions thrown from core functions (and all functions or methods in
extensions) should be organized.
Based on this interpretation, random_int()
should throw an Error if $min > $max
and random_bytes()
should throw an Error if $length <= 0. random_bytes()
and
random_int()
should throw an Exception if random data cannot be generated.
Another quote from Nikita’s message to the prior thread:
Another interesting aspect are the zpp calls. Currently these will throw a
warning and returnNULL
in weak mode and throw a TypeError in strict mode.
I wonder whether we should be using zpp_throw for new functions, so a
TypeError is also thrown in weak mode. Continuing to use the old
warning+NULL behavior was a BC concern, which we don't have for new
functions. The reason why I think this is worth considering, is that if all
other error conditions of a function already throw, then ordinary zpp will
still add one case where a different type is returned on error. This makes
random_int from a function returning int, to a function returning int|null.
I would also be in favor of throwing TypeError from zpp calls in new functions
(and quite frankly, from zpp calls in all functions, including old functions).
Regards,
Aaron Piotrowski