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.


git-svn-id: https://develop.svn.wordpress.org/branches/5.6@49806 602fd350-edb4-49c9-b593-d223f7449a82
This commit is contained in:
Ian Dunn 2020-12-16 14:24:07 +00:00
parent 8bfede5779
commit b5cc4dc44a
3 changed files with 1059 additions and 1 deletions

View File

@ -10,6 +10,9 @@
/**
* Core class for fetching remote files and reading local files with SimplePie.
*
* This uses Core's HTTP API to make requests, which gives plugins the ability
* to hook into the process.
*
* @since 2.8.0
*
* @see SimplePie_File
@ -21,6 +24,8 @@ class WP_SimplePie_File extends SimplePie_File {
*
* @since 2.8.0
* @since 3.2.0 Updated to use a PHP5 constructor.
* @since 5.6.1 Multiple headers are concatenated into a comma-separated string, rather than remaining
* an array.
*
* @param string $url Remote file URL.
* @param int $timeout Optional. How long the connection should stay open in seconds.
@ -60,8 +65,32 @@ class WP_SimplePie_File extends SimplePie_File {
if ( is_wp_error( $res ) ) {
$this->error = 'WP HTTP Error: ' . $res->get_error_message();
$this->success = false;
} else {
$this->headers = wp_remote_retrieve_headers( $res );
$this->headers = wp_remote_retrieve_headers( $res );
/*
* SimplePie expects multiple headers to be stored as a comma-separated string, but
* `wp_remote_retrieve_headers()` returns them as an array, so they need to be
* converted.
*
* The only exception to that is the `content-type` header, which should ignore any
* previous values and only use the last one.
*
* @see SimplePie_HTTP_Parser::new_line().
*/
foreach ( $this->headers as $name => $value ) {
if ( ! is_array( $value ) ) {
continue;
}
if ( 'content-type' === $name ) {
$this->headers[ $name ] = array_pop( $value );
} else {
$this->headers[ $name ] = implode( ', ', $value );
}
}
$this->body = wp_remote_retrieve_body( $res );
$this->status_code = wp_remote_retrieve_response_code( $res );
}

File diff suppressed because one or more lines are too long

View File

@ -0,0 +1,119 @@
<?php
/**
* Unit tests for methods in `WP_SimplePie_File`.
*
* @package WordPress
* @subpackage UnitTests
* @since 5.6.1
*/
/**
* Tests the `WP_SimplePie_File` class.
*
* @group feed
* @group wp-simplepie-file
*
* @since 5.6.1
*/
class Tests_WP_SimplePie_File extends WP_UnitTestCase {
public static function setUpBeforeClass() {
require_once ABSPATH . '/wp-includes/class-simplepie.php';
require_once ABSPATH . '/wp-includes/class-wp-simplepie-file.php';
}
/**
* Test that single- and multiple-value headers are parsed in the way that SimplePie expects.
*
* @dataProvider data_header_parsing
*
* @covers WP_SimplePie_File::__construct
*
* @since 5.6.1
*
* @ticket 51056
*/
public function test_header_parsing( $callback, $header_field, $expected ) {
add_filter( 'pre_http_request', array( $this, $callback ) );
$file = new WP_SimplePie_File( 'https://wordpress.org/news/feed/' );
$this->assertSame( $expected, $file->headers[ $header_field ] );
}
/**
* Provide test cases for `test_header_parsing()`.
*
* @return array
*/
public function data_header_parsing() {
return array(
'single content type header works' => array(
'mocked_response_single_header_values',
'content-type',
'application/rss+xml; charset=UTF-8',
),
'single generic header works' => array(
'mocked_response_single_header_values',
'link',
'<https://wordpress.org/news/wp-json/>; rel="https://api.w.org/"',
),
'only the final content-type header should be used' => array(
'mocked_response_multiple_header_values',
'content-type',
'application/rss+xml; charset=UTF-8',
),
'multiple generic header values should be merged into a comma separated string' => array(
'mocked_response_multiple_header_values',
'link',
'<https://wordpress.org/news/wp-json/>; rel="https://api.w.org/", <https://wordpress.org/news/wp/v2/categories/3>; rel="alternate"; type="application/json"',
),
);
}
/**
* Mock a feed HTTP response where headers only have one value.
*/
public function mocked_response_single_header_values() {
$single_value_headers = array(
'content-type' => 'application/rss+xml; charset=UTF-8',
'link' => '<https://wordpress.org/news/wp-json/>; rel="https://api.w.org/"',
);
return array(
'headers' => new Requests_Utility_CaseInsensitiveDictionary( $single_value_headers ),
'body' => file_get_contents( DIR_TESTDATA . '/feed/wordpress-org-news.xml' ),
'response' => array(
'code' => 200,
'message' => 'OK',
),
'cookies' => array(),
'filename' => null,
);
}
/**
* Mock a feed HTTP response where headers have multiple values.
*/
public function mocked_response_multiple_header_values() {
$response = $this->mocked_response_single_header_values();
$multiple_value_headers = array(
'content-type' => array(
'application/rss+xml; charset=ISO-8859-2',
'application/rss+xml; charset=UTF-8',
),
'link' => array(
'<https://wordpress.org/news/wp-json/>; rel="https://api.w.org/"',
'<https://wordpress.org/news/wp/v2/categories/3>; rel="alternate"; type="application/json"',
),
);
$response['headers'] = new Requests_Utility_CaseInsensitiveDictionary( $multiple_value_headers );
return $response;
}
}