Block Themes: PHP 8.1 Deprecation notice passing null to non-nullable when referring to missing template parts

[TJNowell]

We have a system that manages template and template part posts, and have noticed that if a template refers to a template part that no longer exists or did not sync to the site, we get a cryptic PHP fatal error about passing null to preg_replace:

preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated

This occurs in template-part.php in the render_block_core_template_part function when calling shortcode_unautop:

    // Run through the actions that are typically taken on the_content.
    $content                       = shortcode_unautop( $content );

In this case $content contains the value null. My expectation is that this would fail gracefully, and there is a check if it is null further up the function, but it only runs if $debug is true:

	// WP_DEBUG_DISPLAY must only be honored when WP_DEBUG. This precedent
	// is set in `wp_debug_mode()`.
	$is_debug = WP_DEBUG && WP_DEBUG_DISPLAY;

	if ( is_null( $content ) && $is_debug ) {

Meaning that unless WP_DEBUG and WP_DEBUG_DISPLAY are both set to true, there is no protection against null being passed into shortcode_autop

[TJNowell]

Investigating further, the templates in question had differing block markup:

I found this on the templates that triggered the fatal error:

<!-- wp:template-part {"slug":"footer","tagName":"footer"} /-->

This however did not trigger the error:

<!-- wp:template-part {"slug":"footer","theme":"my-theme"} /-->

I do not know how this difference occurred

[TJNowell]

Perhaps if this:

	if ( is_null( $content ) && $is_debug ) {
		if ( ! isset( $attributes['slug'] ) ) {
			// If there is no slug this is a placeholder and we dont want to return any message.
			return;
		}
		return sprintf(
			/* translators: %s: Template part slug. */
			__( 'Template part has been deleted or is unavailable: %s' ),
			$attributes['slug']
		);
	}

Was changed to this:

	if ( is_null( $content ) ) {
		if ( $is_debug ) {
			if ( ! isset( $attributes['slug'] ) ) {
				// If there is no slug this is a placeholder and we dont want to return any message.
				return;
			}
			return sprintf(
				/* translators: %s: Template part slug. */
				__( 'Template part has been deleted or is unavailable: %s' ),
				$attributes['slug']
			);
		}

		return '';
	}

Then it would degrade gracefully without a PHP deprecation fatal in PHP 8.1

Implements failing unit test demonstrating that render_block_core_template_part method can return NULL in some situations. This can go on to cause a fatal elsewhere, as detailed in 59318.

PR will be augmented with a second commit to fix this following @tomjn's comment on the trac ticket once CI completes once with failures.

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

@tomjn I'm not sure what to make of the "Ensure version-controlled files are not modified or deleted" error, that seems... tautological, if we're submitting a code change... but, I think the other PHPCS error is unrelated to this patch. May not be a perfect solution but hope it helps your advocacy for the ticket.

@kadamwhite Looks like this should be patched on the Gutenberg side, in ​this file.

[TJNowell]

Noting that I've raised ​https://github.com/WordPress/gutenberg/issues/54352 after seeing the comment that the PR should have been to the gutenberg repo

[TJNowell]

Raised a PR at the gutenberg github: ​https://github.com/WordPress/gutenberg/pull/54354

Please see ​https://github.com/WordPress/gutenberg/pull/54354 which _appears_ to fix this. Would it be good to include the unit test coverage in that PR and then they can be both backported from the Plugin to Core as part of 6.4?

[get_dave]

Now the ​https://github.com/WordPress/gutenberg/pull/54354 has merged the changes in that PR should land in Core trunk by Beta 1. At which point we should be good to land the unit tests here.

[hellofromTonya]

Package fix (maintained in Gutenberg):

• Fix PR: ​https://github.com/WordPress/gutenberg/pull/54354
• ​Committed in Core via [57578].

What's next before closing this ticket?

1. Test to confirm it is fixed in Core.
2. Review of the unit tests. Current patch: ​https://github.com/WordPress/wordpress-develop/pull/5177

[hellofromTonya]

Test Report

Patch tested: 6.6 Beta 2 or current trunk.

Steps to Reproduce or Test

Setup:

1. Use PHP 8.1 or higher.
2. Use WP 6.3.4 when attempting to reproduce the reported issue.
3. Use WP 6.6 Beta 2 or newer to test if the bug is fixed in Core.

Testing Steps:

1. Open wp-content/themes/twentytwentyfour/templates/home.html file in your favorite code editor.
2. In the footer template-part, change the slug to something that does not exist:

   <!-- wp:template-part {"slug":"doesnotexistfooter","area":"footer","tagName":"footer"} /-->	
   

3. $is_debug needs to be false. Two ways to accomplish this:
   1. Set WP_DEBUG to false in wp-config.php.
   2. Set WP_DEBUG_DISPLAY to false in wp-config.php`.
   3. Temporarily modify the following line of code in wp-includes/blocks/template-part.php to hardcode it to false. This approach lets you keep WP_DEBUG and WP_DEBUG_DISPLAY set to true to further see the deprecation notice in the front-end.

      $is_debug = false; //WP_DEBUG && WP_DEBUG_DISPLAY;	
      

4. Open the home page in the front. 🐞 A Deprecation notice is thrown (viewable in the server logs, debug.log, and/or on the screen in your browser).

Expected Results

When testing a patch to validate it works as expected:

• ✅ What should happen when running the test

When reproducing a bug:

• ❌ PHP Deprecation notice should be thrown.

Environment

• OS: macOS
• Web Server: nginx
• PHP: 8.3
• WordPress: 6.3.4 to reproduce the error and then 6.6 Beta 2 to validate the fix.
• Browser: Firefox 126.0
• Theme: TT4
• Plugins:
  • Beta Tester
  • Core Rollback

Notes:

I used step 3c as I want to see the PHP deprecation in the browser and in the debug.log file. I made sure my wp-config.php had these constants:

if ( ! defined( 'WP_DEBUG' ) ) {
	define( 'WP_DEBUG', true );
}

define( 'WP_DEBUG_LOG', true );
define( 'WP_DEBUG_DISPLAY', true );
define( 'SCRIPT_DEBUG', true );

define( 'WP_ENVIRONMENT_TYPE', 'local' );

Actual Results

When reproducing a bug/defect using 6.3.4:

• ❌ PHP Deprecation notice is thrown.

Deprecated: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /Volumes/DevSSD/Local/releaseparty/app/public/wp-includes/formatting.php on line 868	

When testing the bugfix patch using 6.6 Beta 2:

• ✅ No longer throwing the PHP Deprecation notice.

Observations

Yes, the committed fix resolves the reported issue. However, passing null to a non-nullable parameter is a deprecation notice and not a fatal error introduced in PHP 8.1. ​https://php.watch/versions/8.1/internal-func-non-nullable-null-deprecation

As @poena ​originally reported in the fix's PR, there is another issue with a higher error level:

Warning: Attempt to read property "content" on null in /Volumes/DevSSD/Local/releaseparty/app/public/wp-includes/blocks/template-part.php on line 73

This happens because $block_template is being used as an object when get_block_file_template() returns null. An additional guard check is needed.

This issue will need a separate fix. It'll need a separate Gutenberg issue and fix as well as tests.

[hellofromTonya]

Modifying the title. The severity of the PHP error is a deprecation notice, rather than a fatal error in PHP 8.1+.

PHP 8.1 introduced a deprecation for passing null to a non-nullable internal function parameter, such as preg_replace().

Ref: ​https://php.watch/versions/8.1/internal-func-non-nullable-null-deprecation

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

[rajinsharwar]

I have tested the PR 59318.
Before NULL error: ​https://img001.prntscr.com/file/img001/XEHwnH_EQiKCYajwsGwnoQ.png
After, no error 🙂: ​https://img001.prntscr.com/file/img001/ScyT3tFRRqavqkm8So_yVg.png ✅

Also @hellofromTonya, the current PR seemed to be outdated compared to the trunk, so I have sent a new PR as a refresh. PR: 6794

[hellofromTonya]

I opened an issue in Gutenberg for the PHP Warning being thrown, as noted in my test report comment:15

As @poena originally reported in the fix's PR, there is another issue with a higher error level:

Warning: Attempt to read property "content" on null in ../wp-includes/blocks/template-part.php on line 73

This happens because $block_template is being used as an object when get_block_file_template() returns null. An additional guard check is needed.

This issue will need a separate fix. It'll need a separate Gutenberg issue and fix as well as tests.

​https://github.com/WordPress/gutenberg/issues/62559

The fix will need to happen in Gutenberg first, as the function is maintained within its package, i.e. packages/block-library/src/template-part/index.php file.

​https://github.com/WordPress/gutenberg/blob/ee30a87abd733d00338730eed831cfc02d27edf0/packages/block-library/src/template-part/index.php#L117

I'd also suggest the unit test be created with the fix. When ready, both changes can be merged into Core with a package update.

[hellofromTonya]

I'll close this ticket as reported upstream.

The original issue was fixed upstream and then merged into Core with a npm package update via #60315.

The remaining PHP Warning issue is also reported upstream and needs to be fixed in Gutenberg first. When the fix and its unit test(s) are ready, they too will be merged into Core via #60315 (if landing in 6.6).

Thank you everyone for your contributions :)