Fix #54352 prevent php 8.1 fatal when template parts are not found in non-debug environments

[tomjn]

What?

If the template part is not found content will be null which will cause fatal errors in PHP 8.1 when passed to shortcode_unautop, but this case is already handled if debug mode is turned on. This changeset allows it to be handled in all environments, not just when the admin has turned on WP_DEBUG and display debug messages.

Closes #54352

Why?

Because it's fixing a fatal error on PHP 8.1+

How?

This change separates the debug part into a sub-condition so that it can return '' if we are not in debugging mode.

Testing Instructions

• Create a template with a template part
• Go into the code editor and change the slug to something that does not exist on that site
• ensure you are using PHP 8.1+
• ensure that WP_DEBUG is false, it is essential that the $debug in the code has the value of false not true
• visit a page with that template on the frontend
• you should see that the page loads with this change, but crashes without it with a deprecation fatal error

[tomjn]

Noting that a PR was originally raised by @kadamwhite here WordPress/wordpress-develop#5177 which contains a small unit test for WP core

[getdave]

@tomjn Thanks for the PR.

Can I confirm whether this one is the canonical PR to fix #54352?

I believe it is which is why I've now linked it up with the Issue and then moved the Issue to "Needs Review" on the WP 6.4 Board but I felt it was wise to check given your comment above.

Also, apologies, but I was unable to replicate the original error. Perhaps it's due to my environment (I'm using wp-env) but despite following your instructions I couldn't get the original situation described in the Issue to occur. Could you think what I might be missing here?

[tomjn]

It's likely the template part you're referencing exists, a reading of the code clearly indicates a failure point where a non string value could be passed through. Note that no testing has been done on 6.4, only 6.3.

Note that the associated WP Trac ticket has a failing unit test that is fixed by this code change that was written by KAdam

[getdave]

Thanks for the update 👍

It's likely the template part you're referencing exists

Not unless I have a template part called thistempaltepartdoesnotexistsomakesurebytypingalongname.

I agree with your reading of the code, but I would like to be able to replicate. I'll continue in my efforts...

[tomjn]

In my original tests, the block template part was missing the theme attribute when I had the issue, which was the main reason it couldn't find the part

[carolinan]

I am not able to reproduce the fatal error, but I see a PHP warning for a different line, unaffected by the PR. So the warning shows when my template part slug is a non-existing template part, no matter if the PR is applied or not.

Warning: Attempt to read property "content" on null in /gutenberg/build/block-library/blocks/template-part.php on line 69

[tomjn]

I would note that $debug must resolve to false in order for testing of this PR to be possible. If it is true then the original bug will be handled by the original code which was the root of the problem. This is to prevent non-debug production sites from crashing in this situation.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: tomjn <tjnowell@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>
Co-authored-by: anton-vlasenko <antonvlasenko@git.wordpress.org>
Co-authored-by: hellofromtonya <hellofromtonya@git.wordpress.org>
Co-authored-by: carolinan <poena@git.wordpress.org>
Co-authored-by: annezazu <annezazu@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[getdave]

I'm going to re-test this.

[getdave]

Thank you for the additional testing instructions.

I felt this was worth revisiting as a bug so I re-tested this by:

• using PHP 8.1+
• setting WP_DEBUG to false.
• creating a template part with a random slug to ensure it would not exist

I then loaded the page with that template on the front of the site.

Testing on trunk

Without the change in this PR, I didn't see an error, but I did see an empty template part output to the front of the site. This is because the code managed to reach the return which outputs the HTML without erroring:

gutenberg/packages/block-library/src/template-part/index.php

Line 171 in 884c494

return "<$html_tag $wrapper_attributes>" . str_replace( ']]>', ']]&gt;', $content ) . "</$html_tag>";

I consider the empty template part to be a bug. Not sure if something changed and why I don't get the error that you saw.

Testing on this PR

With the change in this PR the code correctly exits execution of the remainder of the function regardless of the value of the WP_DEBUG.

On the front of the site no template part markup was output at all which is what I would consider correct.

---

I believe the fix is good, but I would appreciate a confidence check (maybe @anton-vlasenko ?).

I think we should also land the unit test coverage for this that you referenced. This can land directly in WP Core once this patch is in Core.

[anton-vlasenko]

Just a minor suggestion.

[getdave]

@tomjn Most of the test failures here look to be due to issues that were previously in evidence in Gutenberg trunk but have since been resolved.

If you:

• update your fork
• rebase this against the updated trunk

...then the tests should pass.

If would be ideal if this landed today so it was included in the final Gutenberg release for WP 6.5 which is happening at 16:00 UTC today.

If it doesn't make it it will need a manual backport as a bug.

[tomjn]

@getdave I rebased, just waiting for CI, also happy for any intervention that needs to happen so this isn't blocked by me

[getdave]

@getdave I rebased, just waiting for CI, also happy for any intervention that needs to happen so this isn't blocked by me

Thank you. Unfortunately I cannot push to your fork otherwise I'd happily wrangle this on your behalf.

[getdave]

@MaggieCabrera Can we bring this in to 17.7.0 RC? If it didn't make the cut please do add the Needs PHP Backport label.

[MaggieCabrera]

@MaggieCabrera Can we bring this in to 17.7.0 RC? If it didn't make the cut please do add the Needs PHP Backport label.

This made it to the RC