Skip to content

Throw exception for invalid characters for number base - #22365

Open
Sjord wants to merge 1 commit into
php:masterfrom
Sjord:error-on-invalid-chars
Open

Throw exception for invalid characters for number base#22365
Sjord wants to merge 1 commit into
php:masterfrom
Sjord:error-on-invalid-chars

Conversation

@Sjord

@Sjord Sjord commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This is about the situation where a character is not within the specified number base. E.g. only 0-9, a-f are allowed for hexadecimal, so what should hexdec('z') do?

In the past, such characters were silently ignored. Since PHP 7.4 a deprecation notice would be generated. This PR changes that to a ValueError Exception.

RFC: https://wiki.php.net/rfc/throw_error_for_invalid_characters_for_number_base
Earlier RFC: https://wiki.php.net/rfc/base_convert_improvements

@Sjord
Sjord requested a review from bukka as a code owner June 19, 2026 08:58
@devnexen

devnexen commented Jun 20, 2026

Copy link
Copy Markdown
Member

Hi @Sjord. My main concern is process about your PR is effectively phase 2 of the 2018 base_convert_improvements RFC (the 8.0 exception that never shipped),
so a throw in 8.6 likely needs fresh cover. The natural home is the grouped rfc/php86_valueerror_conversions work; folding the four base functions in there, or
floating it on the list first, would keep things consistent.

It also overlaps your own precision-loss thread; both touch _php_math_basetozval, better designed together.

devnexen
devnexen reviewed Jun 20, 2026
View reviewed changes
Comment thread ext/standard/math.c Resolved
@Sjord

Sjord commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

I added it to the RFC: https://wiki.php.net/rfc/php86_valueerror_conversions#throw_error_for_invalid_characters_for_number_base

@Sjord

Sjord commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I also mailed the internals mailing list:

Functions that convert from one number base to another should only accept characters valid in that number base. E.g. only 0-9, a-f are allowed for hexadecimal, so what should hexdec('z') do?

In the past, such characters were silently ignored. Since PHP 7.4 a deprecation notice would be generated, as a result of this RFC: https://wiki.php.net/rfc/base_convert_improvements. That RFC also proposed throwing an exception as of PHP 8.0, but that was never implemented. I propose to throw a ValueError for invalid characters starting in PHP 8.6. I added this to the Grouped ValueError Conversions for PHP 8.6 RFC: https://wiki.php.net/rfc/php86_valueerror_conversions#throw_error_for_invalid_characters_for_number_base

This applies to bindec, octdec, hexdec, and base_convert.

@Sjord

Sjord commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

PHP RFC: Throw ValueError for invalid characters in number base functions

@Sjord
Sjord marked this pull request as draft August 11, 2026 10:51
@Sjord Sjord changed the title Throw error for invalid characters for number base Throw exception for invalid characters for number base Sep 5, 2026
@Sjord
Sjord force-pushed the error-on-invalid-chars branch from 453362a to 8e8a8c1 Compare October 16, 2026 12:31
@Sjord
Sjord marked this pull request as ready for review October 16, 2026 12:59
@Sjord

Sjord commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

The RFC was accepted.

@Sjord
ext/standard: throw exception for invalid characters for number base
34b831d 
This is about the situation where a character is not within the
specified number base. E.g. only 0-9, a-f are allowed for hexadecimal,
so what should `hexdec('z')` do?

In the past, such characters were silently ignored. Since PHP 7.4 a
deprecation notice would be generated. This commit changes that to a
Exception.

RFC: https://wiki.php.net/rfc/throw_error_for_invalid_characters_for_number_base
@Sjord
Sjord force-pushed the error-on-invalid-chars branch from 8e8a8c1 to 34b831d Compare November 2, 2026 21:25
@Sjord
Sjord requested a review from devnexen November 2, 2026 21:25
devnexen
devnexen reviewed Nov 3, 2026
View reviewed changes
Comment thread ext/standard/math.c
if (invalidchars > 0) {
zend_error(E_DEPRECATED, "Invalid characters passed for attempted conversion, these have been ignored");
if (invalidchars) {
zend_throw_exception_ex(NULL, 0, "Invalid characters passed for attempted conversion");

@devnexen devnexen Nov 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you can just use zend_throw_exception here I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devnexen devnexen devnexen left review comments
@bukka bukka Awaiting requested review from bukka bukka is a code owner

Assignees

No one assigned

Labels

ABI break Extension: standard

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Sjord @devnexen