Hello,
Next Friday I'll open the vote for the RFC for Throw ValueError for invalid characters in number base functions.
RFC: https://wiki.php.net/rfc/throw_error_for_invalid_characters_for_number_base
Earlier RFCs:
* https://wiki.php.net/rfc/php86_valueerror_conversions#throw_error_for_invalid_characters_for_number_base
* https://wiki.php.net/rfc/base_convert_improvements
Earlier discussions:
* https://news-web.php.net/php.internals/131448
* https://news-web.php.net/php.internals/104618
* https://news-web.php.net/php.internals/105740
PR: https://github.com/php/php-src/pull/22365
This will throw an error for octdec, hexdec, bindec and base_convert on invalid input. We already agreed to this before (in RFC base_convert changes), so this RFC is not really controversial but more about following the right procedure.
Regards,
Sjoerd Langkemper
Hi
Next Friday I'll open the vote for the RFC for Throw ValueError for
invalid characters in number base functions.
From what I see this is the first email regarding this RFC, the archives
do not show any previous emails in the discussion thread:
https://news-web.php.net/php.internals/132379. New RFCs require a
minimum discussion period of 14 days before they may go to voting.
This will throw an error for octdec, hexdec, bindec and base_convert on
invalid input. We already agreed to this before (in RFC base_convert
changes), so this RFC is not really controversial but more about
following the right procedure.
I'm not sure ValueError is the correct throwable to use here: I believe
passing “user-provided” inputs to these methods might be an expected use
case, where developers would then be interested in catching the
resulting exception as an implicit validation mechanism. This is also
acknowledged by the RFC:
If invalid characters are passed, a ValueError will now be thrown,
which must be handled using try/catch blocks if invalid inputs are
expected from untrusted sources.
The Error hierarchy is not intended to be caught, though. It should thus
use something from the Exception hierarchy.
Best regards
Tim Düsterhus
This will throw an error for octdec, hexdec, bindec and base_convert on
invalid input.I'm not sure ValueError is the correct throwable to use here: I believe
passing “user-provided” inputs to these methods might be an expected use
case, where developers would then be interested in catching the
resulting exception as an implicit validation mechanism. This is also
acknowledged by the RFC: ...The Error hierarchy is not intended to be caught, though. It should thus
use something from the Exception hierarchy.
I wasn't aware of a distinction between error and exception objects like this. Is this documented somewhere? Is there a policy when to throw errors and when to throw exceptions? Does the documentation describe that developers should only catch exceptions and not errors?
Why would these functions be used for user input more often than other functions? How can we determine whether a function should throw an exception or an error?
I think ValueError is still the right thing to throw.
Next Friday I'll open the vote for the RFC for Throw ValueError for
invalid characters in number base functions.From what I see this is the first email regarding this RFC
The RFC was announced here: https://news-web.php.net/php.internals/132219
And before that the same idea in another RFC: https://news-web.php.net/php.internals/131448
This gave more than a month to respond to the proposal, and exactly two weaks for responding to the exact RFC.
Regards,
Sjoerd
Hi
The Error hierarchy is not intended to be caught, though. It should
thus
use something from the Exception hierarchy.I wasn't aware of a distinction between error and exception objects
like this. Is this documented somewhere? Is there a policy when to
throw errors and when to throw exceptions?
Yes, in:
https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#throwables
Specifically:
The Error hierarchy MUST NOT be used for errors that are expected to be
thrown (and caught) during normal operation of a PHP program.As an example, a parsing function that is expected to be used with
untrusted input must not throw an Error if the input is malformed.
Similarly a function that interacts with the network must not throw an
Error if the network operation fails. Any Error that is thrown should
usually result in a reasonably obvious fix in the PHP program.
Why would these functions be used for user input more often than other
functions? How can we determine whether a function should throw an
exception or an error?
The quoted policy provides some examples. I'd argue that “base
conversion” falls into the “parsing function” category, because it needs
to parse the inputs. It is unreasonable for the user to perform a
pre-validation, because that effectively means reimplementing large
parts of the logic of the conversion functions.
Next Friday I'll open the vote for the RFC for Throw ValueError for
invalid characters in number base functions.From what I see this is the first email regarding this RFC
The RFC was announced here:
https://news-web.php.net/php.internals/132219
Ah. That email was a reply to an existing discussion thread though and
thus didn't create a proper top-level discussion for the RFC.
This gave more than a month to respond to the proposal, and exactly two
weaks for responding to the exact RFC.
Due to the above it is likely that the email was collapsed into the
discussion thread without the [RFC] tag and was thus missed by mailing
list participants - as was the case for me.
Best regards
Tim Düsterhus
This will throw an error for octdec, hexdec, bindec and base_convert on
invalid input.I'm not sure ValueError is the correct throwable to use here ... It should thus
use something from the Exception hierarchy.
I am considering this.
Would it be InvalidCharForNumberBaseException extends Exception, or should it be more hierarchical (InvalidCharForNumberBaseException extends ValueException extends RuntimeException extends Exception)?
RangeException from SPL seems useful, but we can't use that because it is in another extension, right?
Regards,
Sjoerd Langkemper
Would it be InvalidCharForNumberBaseException extends Exception, or should it be more hierarchical (InvalidCharForNumberBaseException extends ValueException extends RuntimeException extends Exception)?
RangeException from SPL seems useful, but we can't use that because it is in another extension, right?
Both ValueException and RuntimeException are in SPL, which is always enabled, so there's no practical problem using them.
However, the policy Tim linked to lays out the details of how exceptions should be defined, and says the base should be \Exception, then something extension-specific, not any of the SPL exceptions.
This is a slightly complex case, because there's no extension or obvious group of functions to name the base under. Possibly a BaseConversionException?
Regards,
Rowan Tommins
[IMSoP]
Hi
However, the policy Tim linked to lays out the details of how exceptions should be defined, and says the base should be \Exception, then something extension-specific, not any of the SPL exceptions.
The policy also specifies:
Newly introduced extensions MUST follow the following rules, existing extensions SHOULD follow the rules for newly introduced exceptions, but MAY diverge for consistency with existing symbols.
and with the base conversion functions being in ext/standard, the
following also applies:
Symbols MUST NOT be namespaced under the Core, Standard or Spl namespaces. Instead, these extensions should be considered as a collection of different components (str_, password_), and SHOULD be namespaced according to these component names.
which then means that the following:
This is a slightly complex case, because there's no extension or obvious group of functions to name the base under.
is a relevant consideration: We are adjusting existing functionality in
very narrowly scoped fashion and based on the recent improvements to the
stdlib, it is not too unlikely that the existing functions are
completely redesigned in the future, for example by introducing a new
int\ornumber\namespace.
Possibly a BaseConversionException?
With the above considerations and the policy’s requirement that:
The exception message MUST NOT be the only means of distinguishing exception causes that the user might want to handle differently.
and me coming up with at least two reasonable failure cases (parsing
error and out of range / loss of precision), I would probably make this
a conservative change and go with the plain \Exception exception
directly. This intentionally does not make any promises and requires the
user to use “narrow try blocks” to handle the exception, which is no
worse the status quo.
Best regards
Tim Düsterhus
Hello,
Next Friday I'll open the vote for the RFC for Throw ValueError for
invalid characters in number base functions.
Just incidentally, does this list of functions include intval()?