Fatal error with PHP8 in header link RSS feed on dashboard widget

[NicolasKulka]

I just found an issue with a native WordPress RSS feed on a category.

It returns 2 different links via the rest_output_link_wp_head function in the wp-includes/rest-api.php file.

Example : ​https://www.wpserveur.net/securite-wps/feed/
Link: <​https://www.wpserveur.net/wp-json/>; rel="​https://api.w.org/"
Link: <​https://www.wpserveur.net/wp-json/wp/v2/categories/45>; rel="alternate";

I created a widget on the dashboard via this rss feed with the wp_dashboard_primary_output function.

With PHP 7.4 => Warning: preg_match() expects parameter 2 to be string, array given in /wp-includes/class-simplepie.php on line 2620

With PHP 8.0 => Fatal error: Uncaught TypeError: preg_match(): Argument #2 ($subject) must be of type string, array given in /wp-includes/class-simplepie.php:2620

[NicolasKulka]

Same problem here => #51056

[iandunn]

Hi, thanks for reporting that.

It sounds like you encountered this error with a plugin, is that right? Can you share the full code? If you're seeing a stack trace, please share that too. I'm not sure how to reproduce it yet.

[NicolasKulka]

To reproduce the problem :

add_action( 'wp_dashboard_setup', 'add_dashboard_widget_wpsecu' );
	function add_dashboard_widget_wpsecu() {
		// Add dashboard Widget
		wp_add_dashboard_widget( 'dashboard_widget_wpsecu', __( 'Sécurité de WordPress' ), 'dashboard_widget_function_secu' );

		// Sort
		global $wp_meta_boxes;
		$normal_dashboard               = $wp_meta_boxes['dashboard']['normal']['core'];
		$dashboard_widget_wpsecu_backup = array( 'dashboard_widget_wpsecu' => $normal_dashboard['dashboard_widget_wpsecu'] );
		unset( $normal_dashboard['dashboard_widget_wpsecu'] );
		$sorted_dashboard                             = array_merge( $dashboard_widget_wpsecu_backup, $normal_dashboard );
		$wp_meta_boxes['dashboard']['normal']['core'] = $sorted_dashboard;
	}

	function dashboard_widget_function_secu() {
		$feeds = array(
			'news' => array(
				'link'         => 'https://www.wpserveur.net/securite-wps/',
				'url'          => 'https://www.wpserveur.net/securite-wps/feed/',
				'title'        => 'Securite WordPress',
				'items'        => 3,
				'show_summary' => 1,
				'show_author'  => 0,
				'show_date'    => 1,
			)
		);
		wp_dashboard_primary_output( 'dashboard_widget_wpsecu', $feeds );
	}

[afragen]

Please confirm which version of WordPress is being used. I thought this or a similar error was fixed in RC2 or thereabouts.

[NicolasKulka]

I am using the latest WP 5.5 version with the PHP7.4 version for the warning.

For the fatal error, I am using the latest RC of version WP5.6 with version PHP8.0

[mbabker]

Seems like an unreported bug in the upstream SimplePie package, see ​https://github.com/simplepie/simplepie/blob/1c68e14ca3ac84346b6e6fe3c5eedf725d0f92c6/library/SimplePie.php#L2583-L2585 for the same code in the upstream package.

[iandunn]

In ticket:51056#comment:3 @david.binda suggested that we should fix this internally, because SimplePie wasn't affected. The patch there doesn't seem to fix this problem, though. I'm not sure yet if this is a duplicate, or just related.

This is still a WIP, but this demonstrates one possible fix.

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

Why this fix over the patch in https://core.trac.wordpress.org/attachment/ticket/51056/51056.diff?

🤦 I misread your comment as that patch did work, never mind!

No worries :)

[iandunn]

Assigning to 5.6.1, since there isn't enough time for fit this in for 5.6, and the full impact/solution isn't clear yet.

xref ​https://wordpress.slack.com/archives/C02RQBWTW/p1607372723178400
xref ​https://make.wordpress.org/core/2020/11/23/wordpress-and-php-8-0/

[iandunn]

This may be a duplicate after all.

I've always had lots of problems with the #43055 build tool, and got frustrated enough yesterday that I tried out the #44492 method again. Now that I'm running directly from src/, 51056.diff does fix this for me.

@hellofromtonya, @SergeyBiryukov, I'm wondering if that could have been part of the reason the patch didn't work for you either? See #51960 for some details.

Another reason might be because the transients were cached?

Here's the mu-plugins I'm using to make these easier to test. They effectively prevent the feeds from being cached, so you don't have to manually delete the transients. (You'll still have to delete any existing ones before running this, though).

51056.php:

<?php

// don't cache results, so can easily test changes
add_filter( 'wp_feed_cache_transient_lifetime', function() {
        return 1;
} );

add_action( 'template_redirect', function() {
        // it only happens on some feeds, isolate which ones and figure out why
        $urls = array(
                // these are fine
                'https://wordpress.org/news/category/releases/feed/',
                'https://iandunn.name/tag/z/feed/',
                'https://wptavern.com/2020/12/feed/',
                'https://heropress.com/feed/',

                // these cause errors, b/c they have multiple 'Link' headers
                'https://www.wpserveur.net/securite-wps/feed/',
        );

        foreach ( $urls as $url ) {
                $feed = fetch_feed( $url );

                $items = $feed->get_items( 0, 2 );

                foreach ( $items as $item ) {
                        echo $item->get_permalink() . "\n";
                }
        }

        wp_die(__FILE__);
} );

51956.php:

<?php

// don't cache results, so can easily test changes
add_filter( 'wp_feed_cache_transient_lifetime', function() {
        return 1;
} );

add_action( 'wp_dashboard_setup', 'add_dashboard_widget_wpsecu' );

function add_dashboard_widget_wpsecu() {
        wp_add_dashboard_widget( 'dashboard_widget_wpsecu', __( 'Sécurité de WordPress' ), 'dashboard_widget_function_secu' );
}

function dashboard_widget_function_secu() {
        $feeds = array(
                'news'   => array(
                        // should get fatal w/ these b/c category
                        'link'         => 'https://www.wpserveur.net/securite-wps/',
                        'url'          => 'https://www.wpserveur.net/securite-wps/feed/',

                        // shouldn't get fatal w/ these b/c not category
//                      'link'         => 'https://www.wpserveur.net/',
//                      'url'          => 'https://www.wpserveur.net/feed/',

                        'title'        => apply_filters( 'dashboard_primary_title', __( 'WordPress Blog' ) ),
                        'items'        => 2,
                        'show_summary' => 0,
                        'show_author'  => 0,
                        'show_date'    => 0,
                ),
        );

        wp_dashboard_primary_output( 'dashboard_widget_wpsecu', $feeds );
}

[NicolasKulka]

ye, function 'rest_output_link_wp_head' return two links

/**
 * Outputs the REST API link tag into page header.
 *
 * @since 4.4.0
 *
 * @see get_rest_url()
 */
function rest_output_link_wp_head() {
	$api_root = get_rest_url();

	if ( empty( $api_root ) ) {
		return;
	}

	printf( '<link rel="https://api.w.org/" href="%s" />', esc_url( $api_root ) );

	$resource = rest_get_queried_resource_route();

	if ( $resource ) {
		printf( '<link rel="alternate" type="application/json" href="%s" />', esc_url( rest_url( $resource ) ) );
	}
}

This function 'rest_get_queried_resource_route' add a second link, add WP 5.5

/**
 * Gets the REST route for the currently queried object.
 *
 * @since 5.5.0
 *
 * @return string The REST route of the resource, or an empty string if no resource identified.
 */
function rest_get_queried_resource_route() {
	if ( is_singular() ) {
		$route = rest_get_route_for_post( get_queried_object() );
	} elseif ( is_category() || is_tag() || is_tax() ) {
		$route = rest_get_route_for_term( get_queried_object() );
	} elseif ( is_author() ) {
		$route = '/wp/v2/users/' . get_queried_object_id();
	} else {
		$route = '';
	}

	/**
	 * Filters the REST route for the currently queried object.
	 *
	 * @since 5.5.0
	 *
	 * @param string $link The route with a leading slash, or an empty string.
	 */
	return apply_filters( 'rest_queried_resource_route', $route );
}

[iandunn]

Assuming this is a duplicate, it seems like 51056.diff is a better solution that PR 795.

It seems valid for SimplePie's get_links() to expect a string, since it's parser combines multiple headers into a comma-separated string. That doesn't happen in Core b/c we override the File handler, though.

https://core.trac.wordpress.org/ticket/51056#comment:3 has a more detailed explanation. It just took the build tool realization and a 6th reading for it to all fit together :)

[hellofromTonya]

Using the Dashboard (using the custom widget by the reporter) and front-end with the mu-plugin code Ian provided:

• before: fatal error in the Dashboard and front-end as well as in debug.log
• after 51056.diff: no fatal error and works as expected
• after PR 795: no fatal error and works as expected

Both the PR in this ticket and the patch in #51056 resolve the warning (< PHP8) and fatal error (PHP 8).

I agree with @iandunn that this ticket is a duplicate of #51056, whereas 50156 reports the PHP warning for < PHP 8 and this ticket reports the fatal error for PHP 8.

Which solution is better?

IMO 50156.diff is a better solution.

Why?

The patch directly handles the conversion to the expected comma-separated string at the point where the array or Requests_Utility_CaseInsensitiveDictionary (dictionary) is returned from rest_output_link_header(), i.e. rather than letting it propagate.

[iandunn]

Duplicate of #51056.

[iandunn]

In 49803:

Feed: Merge multiple header values to avoid fatal error.

When SimplePie parses HTTP headers, it combines multiple values for the same header into a comma-separated string. WP_SimplePie_File overrides the parsing, but was leaving them as an array instead.

That lead to a fatal error in PHP 8, because other parts of the codebase ended up passing an array to a function that expected a string.

Props david.binda, litemotiv, inc2734, NicolasKulka, hellofromTonya, mbabker, skithund, SergeyBiryukov, desrosj, timothyblynjacobs.
Fixes #51056. See #51956.

[iandunn]

In 49806:

Feed: Merge multiple header values to avoid fatal error.

When SimplePie parses HTTP headers, it combines multiple values for the same header into a comma-separated string. WP_SimplePie_File overrides the parsing, but was leaving them as an array instead.

That lead to a fatal error in PHP 8, because other parts of the codebase ended up passing an array to a function that expected a string.

Props david.binda, litemotiv, inc2734, NicolasKulka, hellofromTonya, mbabker, skithund, SergeyBiryukov, desrosj, timothyblynjacobs.
Reviewed by SergeyBiryukov, iandunn.
Merges [49803] and [49805] to the 5.6 branch.
Fixes #51056. See #51956.

[Znuff]

49806 doesn't seem to fix the issue under PHP 8.0(.1).

I've applied the diff, I even replaced the full file (just in case I screwed up with the diff), but the issue is still present.

This is also a warning in PHP 7.4 (not a fatal error).

[iandunn]

Hi @Znuff, what version of WP did you apply the diff to?

Can you reproduce it on a clean install? If so, can you open a new ticket with the reproduction steps, copy/paste the exact error output, etc? Include a link to this ticket as background info.

Merged in https://core.trac.wordpress.org/changeset/49803