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

Fixes #721 Prevent PHP errors #735

Merged
merged 3 commits into from
Sep 13, 2023
Merged

Fixes #721 Prevent PHP errors #735

merged 3 commits into from
Sep 13, 2023

Conversation

Tabrisrp
Copy link
Contributor

@Tabrisrp Tabrisrp commented Jul 11, 2023

Check if array keys exists before using them

Fixes #721

@Tabrisrp Tabrisrp requested a review from a team July 11, 2023 15:42
@Tabrisrp Tabrisrp self-assigned this Jul 11, 2023
@Mai-Saad Mai-Saad self-requested a review July 12, 2023 08:17
@piotrbak piotrbak added this to the 2.1.2 milestone Jul 12, 2023
@Mai-Saad
Copy link

@Tabrisrp Thanks for the PR. Can you please check the following cases 🙏
Can reproduce the following error using PHP 8.2 and the PR

[12-Jul-2023 10:41:48 UTC] PHP Warning:  Undefined array key "option_page" in /var/www/new.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/inc/classes/class-imagify-settings.php on line 134
[12-Jul-2023 10:41:48 UTC] PHP Deprecated:  htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/new.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/inc/classes/class-imagify-settings.php on line 134

Scenario1:
steps:
1- use a wrong API key
2- correct the API key and save

Scenario2: => even without step 1 , we have the same error on newlabs
1- delete AS actions table
2- fresh install and activate of the PR

@Tabrisrp
Copy link
Contributor Author

Thanks, I updated the code to prevent it too

@piotrbak
Copy link

@engahmeds3ed Could you do a CR after last changes here?

@mostafa-hisham
Copy link
Contributor

@piotrbak i didn't get the warning in the issue but got this one

[30-Aug-2023 04:53:45 UTC] PHP Deprecated:  Return type of Imagify_Files_Iterator::accept() should either be compatible with FilterIterator::accept(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /home/mostafa/Local Sites/wp-copra/app/public/wp-content/plugins/imagify-plugin/inc/classes/class-imagify-files-iterator.php on line 65

[30-Aug-2023 05:29:07 UTC] PHP Deprecated:  Return type of Imagify_Files_Recursive_Iterator::accept() should either be compatible with FilterIterator::accept(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /home/mostafa/Local Sites/wp-copra/app/public/wp-content/plugins/imagify-plugin/inc/classes/class-imagify-files-recursive-iterator.php on line 55

Steps:
php v8.1
1- add wp-content/themes/twentytwentytwo/ folder in imagify settings (this triggers the first warning)
2- click bulk optimization (this triggers a second warning after the first step)

FYI: adding #[\ReturnTypeWillChange] fix the problem but i am not sure if it is the best way to handle it.

and when i did the second testing scenario here i got alot of fatal error but i don't think it is related to this PR.
I think it should be covered here

@Mai-Saad Mai-Saad added this pull request to the merge queue Sep 13, 2023
Merged via the queue into develop with commit f462e8b Sep 13, 2023
7 checks passed
@Mai-Saad Mai-Saad deleted the fix/721-php-errors branch September 13, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FILTER_SANITIZE_STRING deprecation notice with PHP 8.1
5 participants