polyfill
polyfill copied to clipboard
symfony
`ValueError` class not available in PHP < 8
The MBString polyfill supports PHP >= 7.2 but it makes use of the ValueError exception class, which was only introduced in PHP 8.
In a lot of instances, there is a version check before it is used, and in these cases there is no problem.
However, there are three unguarded locations that refer to the class, which will result in fatal errors on PHP 7:
https://github.com/symfony/polyfill/blob/731f6e101c33d3d1c2e4a8a87e4e4fe9b2f50f72/src/Mbstring/Mbstring.php#L837-L841
https://github.com/symfony/polyfill/blob/731f6e101c33d3d1c2e4a8a87e4e4fe9b2f50f72/src/Mbstring/Mbstring.php#L849-L851
https://github.com/symfony/polyfill/blob/731f6e101c33d3d1c2e4a8a87e4e4fe9b2f50f72/src/Mbstring/Mbstring.php#L1040-L1043
Indeed, this looks like a bug. As a workaround, you can require the symfony/polyfill-php80 package in your project which should declare the missing ValueError class.
What shall we do here, @nicolas-grekas? The easy way would of course be to register the PHP 8.0 polyfill as a dependency of the mbstring polyfill.
I'm aware that these new methods were only added in PHP 8 and therefore have always thrown ValueError exceptions, but I think if they had been present in older versions then they would have triggered errors/returned false like other similar mb_* functions.
Therefore, my recommendation would be to update the locations that are throwing ValueError to work the same as the older functions, which already have logic in place to handle this.
I guess, if you wanted to, you could use class_exists("ValueError") rather than a version check, so that if the shim was installed it would use it. However, I am very wary of an additional dependency being added and making the php8 polyfill a mandatory requirement.
Another thought: If using trigger_error()/return false seems a bit icky for functions that were never implemented in that way, you could just throw a standard Exception on older PHP versions.
tl;dr I'd prefer it if the solution didn't add an additional dependency, whatever it is.
Another alternative is to define the specific polyfill for this exception class in the Mbstring polyfill codebase, that way behaviour is unchanged and there are no additional dependencies.
I'm not sure how that sits re: code duplication, but this looks like a pretty trivial class to shim, so perhaps it's not really an issue at this scale. It would certainly be the simplest/safest solution, imho.
I'd say the polyfill shouldn't throw if it doesn't run on PHP 8. That'd also be the correct polyfilling behavior on PHP7.
I'd say the polyfill shouldn't throw if it doesn't run on PHP 8. That'd also be the correct polyfilling behavior on PHP7.
So, oldschool trigger_error() and return false;?
Came here to report the same problem for the PHP 8.3 polyfills.
They have a minimum supported PHP version of PHP 7.2 (at this moment), but in the https://github.com/symfony/polyfill/blob/1.x/src/Php83/Php83.php file, the PHP 8.0+ ValueError class is referenced 13 times, while for PHP 7.2-7.4, the ValueError class is not available.
Edit: considering that the polyfilled functions for PHP 8.3 natively throw that exception, I do believe polyfilling the ValueError class for the PHP 8.3 polyfills is the appropriate fix, either by including the PHP 8.0 polyfills or by adding a plain polyfill for the ValueError class to the PHP 8.3 polyfills.
Just confirmed the same issue also exists for the PHP 8.4 polyfills.
The PHP 8.0+ ValueError class is referenced 10 times in the https://github.com/symfony/polyfill/blob/1.x/src/Php84/Php84.php file, which makes the polyfills incompatible with PHP 7.2-7.4.