Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incompatibility with PHP 7.4 due to str_starts_with usage in Drawing.php on PhpSpreadsheet v1.29.0 #4215

Closed
1 of 3 tasks
gabox00 opened this issue Nov 5, 2024 · 5 comments

Comments

@gabox00
Copy link

gabox00 commented Nov 5, 2024

This is:

less

What is the current behavior?
An error occurs due to the usage of str_starts_with, which is a PHP 8.0 function, in the Drawing.php file. This function is not compatible with PHP 7.4.

What are the steps to reproduce?

$spreadsheet = IOFactory::load($_FILES['fileSEPA']['tmp_name']);

Error location
The error is located in the vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Worksheet/Drawing.php file, specifically in the isImage method:

private function isImage(string $path): bool
{
$mime = (string) @mime_content_type($path);
$retVal = false;
if (str_starts_with($mime, 'image/')) { // This line causes the error
$retVal = true;
} elseif ($mime === 'application/octet-stream') {
$extension = pathinfo($path, PATHINFO_EXTENSION);
$retVal = in_array($extension, ['bin', 'emf'], true);
}

return $retVal;

}

What features do you think are causing the issue
Reader
Writer
Styles
Data Validations
Formula Calculations
Charts
AutoFilter
Form Elements
Drawing (specifically)
Does an issue affect all spreadsheet file formats? If not, which formats are affected?
N/A

Which versions of PhpSpreadsheet and PHP are affected?
PhpSpreadsheet: 1.29.0
PHP: 7.4
Suggested Solution
Replace the str_starts_with function in Drawing.php with an alternative compatible with PHP 7.4, such as strpos($mime, 'image/') === 0.

@oleibman
Copy link
Collaborator

oleibman commented Nov 6, 2024

This is a very puzzling problem. You are, of course, correct, and I can easily verify this:

php7.4 -r "var_dump(str_starts_with('xyz','x'));"

That throws an exception, as it should. Now for the puzzling part. There is at least 1 test that exercises this code, and we run the test suite on many different Php versions, including 7.4 for 1.29.*. Test Reader/Xlsx/URLImageTest is supposed to exercise this code, and should fail because of the problem you're pointing out. I even added the following code:

if (str_starts_with($mime, 'image/')) {var_dump(PHP_VERSION_ID);

And what do I see?

C:\git\release1291>php7.4 vendor\phpunit\phpunit\phpunit tests\PhpSpreadsheetTests\Reader\xlsx\URLImageTest.php
PHPUnit 9.6.20 by Sebastian Bergmann and contributors.

.C:\git\release1291\src\PhpSpreadsheet\Worksheet\Drawing.php:170:
int(70433)
..                                                                 3 / 3 (100%)

Time: 00:01.010, Memory: 30.00 MB

OK (3 tests, 13 assertions)

So we're running 7.4.33, and yet the function call didn't fail. I have no idea how this is happening! At any rate, I'll see what I can do.

@oleibman
Copy link
Collaborator

oleibman commented Nov 6, 2024

I think I see how this happened. Among the dev packages which we use is friendsofphp/php-cs-fixer, and that package has a requirement for symfony/polyfill-php80. That "conveniently" resolves the str_starts_with call for us when we run our unit tests. Nice, but completely unexpected. While it manages to let our test pass when it should fail, it also points the way to an easy workaround for you. You can just use composer to require the polyfill package, and things should work for you. I will still try to correct this, but at least you can get around the problem while I work on it.

And, without particularly wishing to get into an argument, you should upgrade your Php ASAP.

@oleibman
Copy link
Collaborator

oleibman commented Nov 6, 2024

Just for completeness - mpdf will also polyfill this function if needed. Both (and php81 polyfill and some others which probably are not relevant to this discussion) are autoloaded in vendor/composer files autoload_files.php and autoload_static.php. It appears that if you comment out the appropriate statements in autoload_static, you can get the "true" results of the test.

@agaluf
Copy link

agaluf commented Nov 6, 2024

One solution to fix this would be to add the polyfill-php80 as a dependency to PHPSpreadsheet in Versions supporting PHP < 8.0. If they are not deprecated already, of course.

@oleibman
Copy link
Collaborator

Should be fixed with release 1.29.4.

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

No branches or pull requests

3 participants