Safeguarding has_blocks() Against Fatal Errors

[Howdy_McGee]

The has_blocks() has type conversion at the end function call:

return false !== strpos( (string) $post, '<!-- wp:' );

Where $post is the passed parameter. While this should be a WP_Post, or type that returns a WP_Post, the function should not throw a Fatal Error if $post is an object by the end of the call.

I think this should be revised to soft-error out if whatever parameter is passed cannot be converted to a string. For example, if a WP_Error is given, the system Fatal Errors. If the function's get_post() call returns false, the function continues to try to string convert the passed parameter and could throw a Fatal Error.

[costdev]

Adds a _doing_it_wrong() and returns false when get_post() returns null. Includes PHPUnit test.

[costdev]

A plugin for reproducing the issue and testing the patch. Requires PHP 7+. See instructions below.

[costdev]

Reproduction/Testing steps

1. Download has_blocks.zip.
2. Navigate to Plugins > Add New > Upload Plugin.
3. Select has_blocks.zip, upload and activate.
4. 🐞 There should be an admin notice:

   A Fatal Error occurred: Object of class stdClass could not be converted to string.
   

5. Apply patch 55705.diff.
6. Refresh the page.
7. ✅ There should be an admin notice and a _doing_it_wrong() notice:

   Admin notice:
   A Fatal Error did not occur.
   
   _doing_it_wrong() notice:
   Notice: Function has_blocks was called incorrectly.
   $post is not a valid post.
   Please see Debugging in WordPress for more information.
   (This message was added in version 6.1.0.) in /wp-includes/functions.php on line <number>
   

Clean up

• Deactivate and delete the "has_blocks() Fatal Error" plugin.
• Revert the patch.

[colonelphantom]

The attached patch works as expected. I tested it using the steps from the previous comment and it behaves like described.

[audrasjb]

Many thanks for the great testing instruction!
Self assigning for review.

[audrasjb]

Before patch, using comment 2 instructions - fatal error

[audrasjb]

After patch, using comment 2 instructions - _doing_it_wrong notice 👌

Trac ticket: https://core.trac.wordpress.org/ticket/55705

[audrasjb]

I put together a PR to refresh the previous patch against trunk and to address a small WPCS issue.

[audrasjb]

@costdev some other unit tests are failing using your patch: Unexpected incorrect usage notice for has_blocks.. See the above PR for more details.

Looks like it will need a few more work :)

[costdev]

Thanks @audrasjb - I'll look into this!

Trac ticket: https://core.trac.wordpress.org/ticket/55705

[costdev]

@audrasjb ​PR 3059 is ready for review.

[costdev]

I have updated ​PR 3059 to remove the _doing_it_wrong() as this is inconsistent with Core's usual approach: If the post can't be retrieved, return false (or similar). The revised PR is ready for review.

Big props to @dlh and @peterwilsoncc for advising on this!

I tested the PR and it looks good to me 👍

[audrasjb]

In 53858:

Editor: Safeguard has_blocks() against fatal errors.

This changeset ensures has_blocks() doesn't return a fatal error when $post is not a valid post. If the post can't be retrieved, the function now returns false.

Props Howdy_McGee, costdev, colonelphantom, audrasjb, dlh, peterwilsoncc.
Fixes #55705.

Committed in https://core.trac.wordpress.org/changeset/53858