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

Safe-guard against sizes that are not strings #871

Closed
MathieuLamiot opened this issue Apr 4, 2024 · 4 comments · Fixed by #877
Closed

Safe-guard against sizes that are not strings #871

MathieuLamiot opened this issue Apr 4, 2024 · 4 comments · Fixed by #877
Assignees
Milestone

Comments

@MathieuLamiot
Copy link
Contributor

MathieuLamiot commented Apr 4, 2024

Context
Issue reported on Slack: https://wp-media.slack.com/archives/C056ZJMHG0P/p1712244597119379?thread_ts=1712218266.722999&cid=C056ZJMHG0P

Expected behavior

  • The optimization process should not throw fatal errors in case a size can't be handled as expected. Instead, the size should just be discarded and the process should continue as if it did not exist.

Acceptance Criteria

  • Force a size to be something different than a string

Additional information

Maybe this should be checked at the beginning of optimize_sizes?

Here is the stacktrace of the reported issue:

[2024-03-31T18:00:58.812075+00:00] PHP Fatal error: Uncaught TypeError: preg_match() expects parameter 2 to be string, int given in /nas/content/live/user/wp-content/plugins/imagify/classes/Optimization/Process/AbstractProcess.php:1554#012Stack trace:#012#0 /nas/content/live/user/wp-content/plugins/imagify/classes/Optimization/Process/AbstractProcess.php(1554): preg_match('/^(?<size>.+)@i...', 1920, NULL)#012#1 /nas/content/live/user/wp-content/plugins/imagify/classes/Optimization/Process/AbstractProcess.php(500): Imagify\Optimization\Process\AbstractProcess->is_size_next_gen(1920)#012#2 /nas/content/live/user/wp-content/plugins/imagify/classes/Job/MediaOptimization.php(181): Imagify\Optimization\Process\AbstractProcess->optimize_size(1920, 0)#012#3 /nas/content/live/user/wp-content/plugins/imagify/classes/Job/MediaOptimization.php(66): Imagify\Job\MediaOptimization->task_optimize(Array)#012#4 /nas/content/live/user/wp-content/plugins/imagify/inc/classes/Dependencies/deliciousbrains/wp-background-processing/classes/wp-background-process.php(516): Imagify\Job\M in /nas/content/live/user/wp-content/plugins/imagify/classes/Optimization/Process/AbstractProcess.php on line 1554

@remyperona
Copy link
Contributor

if we receive a int, we can expect it to be a size value, and convert it to a string to make it work with the preg_match().

It probably worked before because this function was not strict on type, but now it is with more recent versions of PHP.

@MathieuLamiot
Copy link
Contributor Author

Oh right, good catch! That would be better to try to convert then!

@wordpressfan
Copy link
Contributor

I found the following line of code inside their theme functions.php

add_action( 'after_setup_theme', function() {
    add_image_size( '1920', 1920, 1080 );
} );

If u take a look at the shared error message, you will see this 1920 size coming as integer

PHP Fatal error: Uncaught TypeError: preg_match() expects parameter 2 to be string, int given in /nas/content/live/xx/wp-content/plugins/imagify/classes/Optimization/Process/AbstractProcess.php:1554#012Stack trace:#012#0 /nas/content/live/xx/wp-content/plugins/imagify/classes/Optimization/Process/AbstractProcess.php(1554): preg_match('/^(?<size>.+)@i...', 1920, NULL)#012#1 /nas/content/live/xx/wp-content/plugins/imagify/classes/Optimization/Process/AbstractProcess.php(500): 

Then I got this snippet in our test site and tried uploading a huge image but I can't see the avif image generated for this size (1920X1080) but again can't see any error in the log.

I'm mentioning the snippet here so QA can add while they validate this case.

@MathieuLamiot
Copy link
Contributor Author

Awesome! Could it be something we check with an integration test directly in the codebase?

@remyperona remyperona added this to the 2.2.2 milestone Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants