![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#55705 closed enhancement (fixed)
Safeguarding has_blocks() Against Fatal Errors
| Reported by: | Howdy_McGee | Owned by: | audrasjb |
|---|---|---|---|
| Priority: | normal | Milestone: | 6.1 |
| Component: | Editor | Version: | 5.0 |
| Severity: | normal | Keywords: | has-patch has-unit-tests has-test-info commit |
| Cc: | Focuses: |
Description
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.
Attachments (4)
Change History (20)
@
4 years ago
A plugin for reproducing the issue and testing the patch. Requires PHP 7+. See instructions below.
#2
@
4 years ago
- Keywords has-patch has-unit-tests needs-testing has-testing-info added
Reproduction/Testing steps
- Download has_blocks.zip.
- Navigate to
Plugins > Add New > Upload Plugin. - Select
has_blocks.zip, upload and activate. - 🐞 There should be an admin notice:
A Fatal Error occurred: Object of class stdClass could not be converted to string.
- Apply patch 55705.diff.
- Refresh the page.
- ✅ 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.
#3
@
4 years ago
- Keywords needs-testing removed
The attached patch works as expected. I tested it using the steps from the previous comment and it behaves like described.
#4
@
4 years ago
- Owner set to
- Status new → reviewing
Many thanks for the great testing instruction!
Self assigning for review.
This ticket was mentioned in PR #3058 on WordPress/wordpress-develop by audrasjb.
4 years ago
#5
Trac ticket: https://core.trac.wordpress.org/ticket/55705
#6
@
4 years ago
I put together a PR to refresh the previous patch against trunk and to address a small WPCS issue.
#7
@
4 years ago
- Keywords changes-requested added
@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 :)
This ticket was mentioned in PR #3059 on WordPress/wordpress-develop by costdev.
4 years ago
#9
Trac ticket: https://core.trac.wordpress.org/ticket/55705
#11
@
4 years ago
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!
4 years ago
#15
Committed in https://core.trac.wordpress.org/changeset/53858
Adds a
_doing_it_wrong()and returnsfalsewhenget_post()returnsnull. Includes PHPUnit test.