-
Notifications
You must be signed in to change notification settings - Fork 157
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
Check return value of get_post_meta #1167
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhanendran Thanks for the PR. As the return value of get_post_meta() is mixed, it may return false
and we will end up overwriting it with an array()
which may throw the PHP warning in v8+.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kirtangajjar thanks for the fix. A couple of minor PHPCS issues are noted below.
An early return would work but will never reach the dt_prepared_meta
filter. We can consider it as an exception though (i.e. empty meta would not reach the filter).
Co-authored-by: Faisal Alvi <[email protected]>
Co-authored-by: Faisal Alvi <[email protected]>
@faisal-alvi Made the requested changes. LMk if there are any more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignoring E2E failures as they are being addressed in #1242
Description of the Change
Validate the return value of get_post_meta and make sure it is array.
Closes #1107
How to test the Change
wp shell
and call the function with an invalid post ID:\Distributor\Utils\prepare_meta('bug');
debug.log
file in yourwp-content
directory and observe the no warning is thrown:PHP Warning: Invalid argument supplied for foreach() in .../includes/utils.php on line 427
Changelog Entry
Credits
Props @dhanendran
Checklist: