Fix float-to-int cast warning in Font::uchr on PHP 8.1+#825
Conversation
When a CMap contains an oversized hex value, hexdec() returns a float larger than PHP_INT_MAX. Casting it to int in Font::uchr() raised a "not representable as an int" warning on PHP 8.1+. Such a value can never be a valid Unicode code point, so it is now treated as a missing character. Fixes warnings during running the tests
|
Thank you @splitbrain! PDF parsing can be goofy sometimes, so even if that seems like a small fix, I would like to back it up. Could you add a basic test which triggers the behavior when running PHP 8.1? What is the current return value without this fix? Could it impact expected parsing results? |
|
I will look into it later. however the warnings were triggered by the existing testsuite for me on php 8.5. looking at the code I believe they should have triggered since 8.1 but I have not verified that. |
Add testUchrWithOutOfRangeFloat covering both the warning fix from the previous commit (e161d7c) for out-of-range floats, INF and NAN, as well as a check that floats in general are still cast to int (added in smalot#623). Note that the previous commit wrongly assumed that the cast warning was introduced in PHP 8.1, but it was in fact introduced in PHP 8.5 only. Previous PHP versions silently cast an unrepresentable float to system dependent "weird" results (negative integers, 0-bytes). See https://wiki.php.net/rfc/warnings-php-8-5 for details. This change enables PHPUnit's failOnWarning option which will turn warnings into actual test failures - the test in this commit as well as the existing ParserTest::testIssue621 and ParserTest::testParseFile will fail now without the patch in e161d7c. Unrelated, minor convenience fix: this also increases the memory limit for unit tests to 1GB (from default 128MB) to avoid out of memory issues during the test run via phpunit.xml
|
I added a specific test for the behavior. See 6053be6 for details. I was indeed wrong about this being a PHP 8.1 change - I thought it was related to Deprecate implicit non-integer-compatible float to int conversions, but it's actually Adding warnings to implicit fallible conversions introduced in PHP 8.5.
As far as I understand it's not 100% defined and depends on the system architecture. Most cases would be a negative integer (which is not a valid Unicode point) - see the linked RFC above.
From how I understand the code, only positively. We now return a clear "unknown gylph" while before an invalid code point was returned. |
2 participants
Type of pull request
About
When a CMap contains an oversized hex value, hexdec() returns a float larger than PHP_INT_MAX. Casting it to int in Font::uchr() raised a "not representable as an int" warning on PHP 8.1+. Such a value can never be a valid Unicode code point, so it is now treated as a missing character.
Fixes warnings during running the tests
Checklist for code / configuration changes