Make WordPress Core

Opened 8 months ago

Last modified 5 weeks ago

#59318 new defect (bug)

Block Themes Fatal Error in PHP 8.1 When Referring To Missing Template Parts

Reported by: tjnowell's profile TJNowell Owned by:
Milestone: 6.6 Priority: normal
Severity: normal Version: 6.3.1
Component: Themes Keywords: has-patch has-unit-tests php81
Focuses: php-compatibility Cc:

Description

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

Change History (12)

#1 @TJNowell
8 months ago

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

#2 @TJNowell
8 months ago

  • Summary changed from Bloc Themes Fatal Error in PHP 8.1 When Referring To Missing Template Parts to Block Themes Fatal Error in PHP 8.1 When Referring To Missing Template Parts

#3 @TJNowell
8 months ago

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

This ticket was mentioned in PR #5177 on WordPress/wordpress-develop by kadamwhite.


8 months ago
#4

  • Keywords has-patch has-unit-tests added

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

kadamwhite commented on PR #5177:


8 months ago
#5

@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.

@costdev commented on PR #5177:


8 months ago
#6

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

#7 @SergeyBiryukov
8 months ago

  • Focuses php-compatibility added
  • Keywords php81 added

#8 @TJNowell
8 months ago

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

@get_dave commented on PR #5177:


8 months ago
#10

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?

#11 @get_dave
3 months ago

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.

#12 @poena
5 weeks ago

  • Milestone changed from Awaiting Review to 6.6
Note: See TracTickets for help on using tickets.