Fetch_feed parsing of permalinks triggers simplepie preg_match warnings

[litemotiv]

Tested with 2 sites running within the same 5.5 multisite network:

$feed = fetch_feed(<other WP site RSS URL>);
$items = $feed->get_items(0,2);

foreach ($items as $item)
    echo $item->get_permalink();

PHP warnings:

 *6667000 FastCGI sent in stderr: "PHP message: PHP Warning:  preg_match() expects parameter 2 to be string, array given in /media/www/multisite/wp-includes/class-simplepie.php on line 2620
PHP message: PHP Warning:  preg_match() expects parameter 2 to be string, array given in /media/www/multisite/wp-includes/class-simplepie.php on line 2620
PHP message: PHP Warning:  preg_match() expects parameter 2 to be string, array given in /media/www/multisite/wp-includes/class-simplepie.php on line 2620
PHP message: PHP Warning:  preg_match() expects parameter 2 to be string, array given in /media/www/multisite/wp-includes/class-simplepie.php on line 2620
[ ... ]

The permalinks are parsed okay and are returned.

Display warning when using category/tag archives feeds.
https://core.trac.wordpress.org/ticket/51056

[inc2734]

I've recreated it too. It happens when you use the feed on the archive page. I created a pull request.
​https://github.com/WordPress/wordpress-develop/pull/496

[david.binda]

I've been able to reproduce the issue as well. As it was already mentioned, the issue is easily reproducible when attempting to parse a category feed on a WordPress install.

What happens is that WordPress sends 2 link HTTP headers in the response via rest_output_link_header function (one for the api.wp.org and one alternate for application/json leading to the rest api endpoint).

SimplePie used by the fetch_feed function, however, expects the multiple headers of the same name being joined together and separated by a comma ( a notation described by rfc8288 ). SimplePie itself handles this in ​https://github.com/simplepie/simplepie/blob/a72e1dfafe7870affdae3edf0d9a494e4fa31bc6/library/SimplePie/HTTP/Parser.php#L254,L263

However the SimplePie's SimplePie_File class is being overloaded by WP_SimplePie_File via SimplePie::set_file_class in https://core.trac.wordpress.org/browser/trunk/src/wp-includes/feed.php?rev=48435#L805 , which does not handle the headers in the same way (multiple headers of the same name are being stored in an array).

IMHO, it would be better to fix the issue in WP_SimplePie_File, not touching the wp-includes/class-simplepie.php file, which is a copy of an external project, which does not suffer from the issue.

Further, the SimplePie_File class makes sure that only the last content-type header is being used. While this works as expected in the current implementation of the WP_SimplePie_File class, I believe that it might be a good idea to cover that with a unit test.

[david.binda]

I forgot to add, that the warning is new, as it was introduced in terms of the SimplePie update in WP 5.5 ( r47733 )

[iandunn]

Related or possible duplicate: #51956

[SergeyBiryukov]

Moving to 5.6.1 along with #51956.

Per comment:4, the warning is the result of updating SimplePie to version 1.5.5 for WordPress 5.5 in [47733] / #36669.

[iandunn]

#51956 was marked as a duplicate.

[iandunn]

Bringing over keywords/etc from #51956.

Additional props from that ticket: NicolasKulka, hellofromTonya, mbabker, skithund , SergeyBiryukov, desrosj, timothyblynjacobs

SimplePie combines multiple values for a HTTP header into a comma-separated string, but WP_SimplePie_File was leaving them as an array. That leads to a fatal error in PHP 8.

Trac ticket: https://core.trac.wordpress.org/ticket/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.

[prbot]

​iandunn commented on ​PR #819:

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

[iandunn]

Reopening for backport to 5.6.1.

@desrosj, @SergeyBiryukov, is one of you able to look at that?

ticket:51956#comment:17 has an mu-plugin that helps w/ manual testing.

[SergeyBiryukov]

Replying to iandunn:

is one of you able to look at that?

Thanks for getting to the bottom of this!

One note I have is that implode() should be used instead of join(), see [49193] / #50767.

Otherwise, [49803] looks good to backport.

[iandunn]

Ah, thanks for catching that!

I'll fix it, and then go ahead and backport since you've already reviewed.

[iandunn]

In 49805:

Feed: Replace join() with implode() for safety.

Canonical functions should be used instead of aliases, because aliases can be deprecated or removed withough much notice. See r49193.

Props SergeyBiryukov.
See #51056.

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

[prbot]

​hellofromtonya commented on ​PR #496:

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