From 30c21d2031a5e9afb6ab7b9b5392e7f4dea21de6 Mon Sep 17 00:00:00 2001 From: Boone Gorges Date: Wed, 20 Mar 2019 15:48:46 +0000 Subject: [PATCH] Posts: Avoid the use of globals in `get_the_content()` and related functions. This changeset introduces `$post` parameters to `get_the_content()` and `wp_trim_excerpt()`. When a `$post` object is passed to one of these functions, the functions will operate on the data from that object, rather than from the post globals (`$authordata`, `$page`, etc). This ensures that the functions work in a predictable manner when used outside of the regular post loop. The global-mismatch problem is surfaced in cases where `get_the_excerpt()` is called outside of the post loop, on posts that don't have a defined excerpt. In these cases, the post globals - used to generate a fallback excerpt - may refer to the incorrect object, resulting in PHP notices or other unpredictable behavior. See #36934 for a related issue. Props spacedmonkey, kraftbj, Shital Patel. Fixes #42814. git-svn-id: https://develop.svn.wordpress.org/trunk@44941 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/class-wp-query.php | 40 ++++- src/wp-includes/default-filters.php | 2 +- src/wp-includes/formatting.php | 9 +- src/wp-includes/post-template.php | 48 ++++-- src/wp-includes/query.php | 20 +++ tests/phpunit/tests/post/getTheContent.php | 78 ++++++++++ tests/phpunit/tests/post/getTheExcerpt.php | 88 +++++++++++ .../phpunit/tests/query/generatePostdata.php | 145 ++++++++++++++++++ 8 files changed, 410 insertions(+), 20 deletions(-) create mode 100644 tests/phpunit/tests/post/getTheContent.php create mode 100644 tests/phpunit/tests/query/generatePostdata.php diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 02f2406479..cd938061db 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -4155,6 +4155,42 @@ class WP_Query { return; } + $elements = $this->generate_postdata( $post ); + if ( false === $elements ) { + return; + } + + $id = $elements['id']; + $authordata = $elements['authordata']; + $currentday = $elements['currentday']; + $currentmonth = $elements['currentmonth']; + $page = $elements['page']; + $pages = $elements['pages']; + $multipage = $elements['multipage']; + $more = $elements['more']; + $numpages = $elements['numpages']; + + return true; + } + + /** + * Generate post data. + * + * @since 5.2.0 + * + * @param WP_Post|object|int $post WP_Post instance or Post ID/object. + * @return array|bool $elements Elements of post or false on failure. + */ + public function generate_postdata( $post ) { + + if ( ! ( $post instanceof WP_Post ) ) { + $post = get_post( $post ); + } + + if ( ! $post ) { + return false; + } + $id = (int) $post->ID; $authordata = get_userdata( $post->post_author ); @@ -4235,7 +4271,9 @@ class WP_Query { */ do_action_ref_array( 'the_post', array( &$post, &$this ) ); - return true; + $elements = compact( 'id', 'authordata', 'currentday', 'currentmonth', 'page', 'pages', 'multipage', 'more', 'numpages' ); + + return $elements; } /** * After looping through a nested query, this function diff --git a/src/wp-includes/default-filters.php b/src/wp-includes/default-filters.php index 1319826212..631525d0e2 100644 --- a/src/wp-includes/default-filters.php +++ b/src/wp-includes/default-filters.php @@ -182,7 +182,7 @@ add_filter( 'the_excerpt', 'convert_smilies' ); add_filter( 'the_excerpt', 'convert_chars' ); add_filter( 'the_excerpt', 'wpautop' ); add_filter( 'the_excerpt', 'shortcode_unautop' ); -add_filter( 'get_the_excerpt', 'wp_trim_excerpt' ); +add_filter( 'get_the_excerpt', 'wp_trim_excerpt', 10, 2 ); add_filter( 'the_post_thumbnail_caption', 'wptexturize' ); add_filter( 'the_post_thumbnail_caption', 'convert_smilies' ); diff --git a/src/wp-includes/formatting.php b/src/wp-includes/formatting.php index 1913dbedff..635d96c20a 100644 --- a/src/wp-includes/formatting.php +++ b/src/wp-includes/formatting.php @@ -3678,14 +3678,17 @@ function human_time_diff( $from, $to = '' ) { * The ' […]' string can be modified by plugins/themes using the {@see 'excerpt_more'} filter * * @since 1.5.0 + * @since 5.2.0 Added the `$post` parameter. * - * @param string $text Optional. The excerpt. If set to empty, an excerpt is generated. + * @param string $text Optional. The excerpt. If set to empty, an excerpt is generated. + * @param WP_Post|object|int $post Optional. WP_Post instance or Post ID/object. Default is null. * @return string The excerpt. */ -function wp_trim_excerpt( $text = '' ) { +function wp_trim_excerpt( $text = '', $post = null ) { $raw_excerpt = $text; if ( '' == $text ) { - $text = get_the_content( '' ); + $post = get_post( $post ); + $text = get_the_content( '', false, $post ); $text = strip_shortcodes( $text ); $text = excerpt_remove_blocks( $text ); diff --git a/src/wp-includes/post-template.php b/src/wp-includes/post-template.php index 3cf0c7c48f..2aad0a03e0 100644 --- a/src/wp-includes/post-template.php +++ b/src/wp-includes/post-template.php @@ -253,6 +253,7 @@ function the_content( $more_link_text = null, $strip_teaser = false ) { * Retrieve the post content. * * @since 0.71 + * @since 5.2.0 Added the `$post` parameter. * * @global int $page Page number of a single post/page. * @global int $more Boolean indicator for whether single post/page is being viewed. @@ -261,14 +262,25 @@ function the_content( $more_link_text = null, $strip_teaser = false ) { * part of the content separated by the `` tag. * @global int $multipage Boolean indicator for whether multiple pages are in play. * - * @param string $more_link_text Optional. Content for when there is more text. - * @param bool $strip_teaser Optional. Strip teaser content before the more text. Default is false. + * @param string $more_link_text Optional. Content for when there is more text. + * @param bool $strip_teaser Optional. Strip teaser content before the more text. Default is false. + * @param WP_Post|object|int $post Optional. WP_Post instance or Post ID/object. Default is null. * @return string */ -function get_the_content( $more_link_text = null, $strip_teaser = false ) { +function get_the_content( $more_link_text = null, $strip_teaser = false, $post = null ) { global $page, $more, $preview, $pages, $multipage; - $post = get_post(); + $_post = get_post( $post ); + + if ( ! ( $_post instanceof WP_Post ) ) { + return ''; + } + + if ( null === $post ) { + $elements = compact( 'page', 'more', 'preview', 'pages', 'multipage' ); + } else { + $elements = generate_postdata( $_post ); + } if ( null === $more_link_text ) { $more_link_text = sprintf( @@ -276,7 +288,12 @@ function get_the_content( $more_link_text = null, $strip_teaser = false ) { sprintf( /* translators: %s: Name of current post */ __( 'Continue reading %s' ), - the_title_attribute( array( 'echo' => false ) ) + the_title_attribute( + array( + 'echo' => false, + 'post' => $_post, + ) + ) ), __( '(more…)' ) ); @@ -286,15 +303,16 @@ function get_the_content( $more_link_text = null, $strip_teaser = false ) { $has_teaser = false; // If post password required and it doesn't match the cookie. - if ( post_password_required( $post ) ) { - return get_the_password_form( $post ); + if ( post_password_required( $_post ) ) { + return get_the_password_form( $_post ); } - if ( $page > count( $pages ) ) { // if the requested page doesn't exist - $page = count( $pages ); // give them the highest numbered page that DOES exist + if ( $elements['page'] > count( $elements['pages'] ) ) { // if the requested page doesn't exist + $elements['page'] = count( $elements['pages'] ); // give them the highest numbered page that DOES exist } - $content = $pages[ $page - 1 ]; + $page_no = $elements['page']; + $content = $elements['pages'][ $page_no - 1 ]; if ( preg_match( '//', $content, $matches ) ) { $content = explode( $matches[0], $content, 2 ); if ( ! empty( $matches[1] ) && ! empty( $more_link_text ) ) { @@ -306,21 +324,21 @@ function get_the_content( $more_link_text = null, $strip_teaser = false ) { $content = array( $content ); } - if ( false !== strpos( $post->post_content, '' ) && ( ! $multipage || $page == 1 ) ) { + if ( false !== strpos( $_post->post_content, '' ) && ( ! $elements['multipage'] || $elements['page'] == 1 ) ) { $strip_teaser = true; } $teaser = $content[0]; - if ( $more && $strip_teaser && $has_teaser ) { + if ( $elements['more'] && $strip_teaser && $has_teaser ) { $teaser = ''; } $output .= $teaser; if ( count( $content ) > 1 ) { - if ( $more ) { - $output .= '' . $content[1]; + if ( $elements['more'] ) { + $output .= '' . $content[1]; } else { if ( ! empty( $more_link_text ) ) { @@ -332,7 +350,7 @@ function get_the_content( $more_link_text = null, $strip_teaser = false ) { * @param string $more_link_element Read More link element. * @param string $more_link_text Read More text. */ - $output .= apply_filters( 'the_content_more_link', ' ID}\" class=\"more-link\">$more_link_text", $more_link_text ); + $output .= apply_filters( 'the_content_more_link', ' ID}\" class=\"more-link\">$more_link_text", $more_link_text ); } $output = force_balance_tags( $output ); } diff --git a/src/wp-includes/query.php b/src/wp-includes/query.php index 9d2db7408e..98f1934b19 100644 --- a/src/wp-includes/query.php +++ b/src/wp-includes/query.php @@ -1111,3 +1111,23 @@ function setup_postdata( $post ) { return false; } + +/** + * Generates post data. + * + * @since 5.2.0 + * + * @global WP_Query $wp_query Global WP_Query instance. + * + * @param WP_Post|object|int $post WP_Post instance or Post ID/object. + * @return array|bool Elements of post, or false on failure. + */ +function generate_postdata( $post ) { + global $wp_query; + + if ( ! empty( $wp_query ) && $wp_query instanceof WP_Query ) { + return $wp_query->generate_postdata( $post ); + } + + return false; +} diff --git a/tests/phpunit/tests/post/getTheContent.php b/tests/phpunit/tests/post/getTheContent.php new file mode 100644 index 0000000000..7bb14a1f93 --- /dev/null +++ b/tests/phpunit/tests/post/getTheContent.php @@ -0,0 +1,78 @@ +Bar'; + $p = self::factory()->post->create( array( 'post_content' => $text ) ); + + $q = new WP_Query( array( 'p' => $p ) ); + while ( $q->have_posts() ) { + $q->the_post(); + + $found = get_the_content( 'Ping' ); + } + + $this->assertContains( '>Ping<', $found ); + } + + /** + * @ticket 42814 + */ + public function test_argument_back_compat_strip_teaser() { + $text = 'FooBar'; + $p = self::factory()->post->create( array( 'post_content' => $text ) ); + + $this->go_to( get_permalink( $p ) ); + + $q = new WP_Query( array( 'p' => $p ) ); + while ( $q->have_posts() ) { + $q->the_post(); + + $found = get_the_content( null, true ); + } + + $this->assertNotContains( 'Foo', $found ); + } + + /** + * @ticket 42814 + */ + public function test_content_other_post() { + $text_1 = 'FooBarBaz'; + $post_1 = self::factory()->post->create_and_get( array( 'post_content' => $text_1 ) ); + + $text_2 = 'BingBangBoom'; + $post_2 = self::factory()->post->create_and_get( array( 'post_content' => $text_2 ) ); + setup_postdata( $post_1 ); + $found = get_the_content( null, true, $post_2 ); + + $this->assertSame( 'Bing', $found ); + } + + /** + * @ticket 42814 + */ + public function test_should_respect_pagination_of_inner_post() { + $text_1 = 'FooBarBaz'; + $post_1 = self::factory()->post->create_and_get( array( 'post_content' => $text_1 ) ); + + $text_2 = 'BingBangBoom'; + $post_2 = self::factory()->post->create_and_get( array( 'post_content' => $text_2 ) ); + $go_to = add_query_arg( 'page', '2', get_permalink( $post_1->ID ) ); + $this->go_to( $go_to ); + + while ( have_posts() ) { + the_post(); + $found = get_the_content( '', false, $post_2 ); + } + + $this->assertSame( 'Bang', $found ); + } +} diff --git a/tests/phpunit/tests/post/getTheExcerpt.php b/tests/phpunit/tests/post/getTheExcerpt.php index 65e5bb0f03..8633a27c35 100644 --- a/tests/phpunit/tests/post/getTheExcerpt.php +++ b/tests/phpunit/tests/post/getTheExcerpt.php @@ -57,4 +57,92 @@ class Tests_Post_GetTheExcerpt extends WP_UnitTestCase { $post_id = self::factory()->post->create( array( 'post_excerpt' => 'Bar' ) ); $this->assertSame( 'Bar', get_the_excerpt( $post_id ) ); } + + /** + * @ticket 42814 + */ + public function test_should_fall_back_on_post_content_if_excerpt_is_empty_and_post_is_inferred_from_context() { + $post_id = self::factory()->post->create( + array( + 'post_content' => 'Foo', + 'post_excerpt' => '', + ) + ); + + $q = new WP_Query( + array( + 'p' => $post_id, + ) + ); + + while ( $q->have_posts() ) { + $q->the_post(); + $found = get_the_excerpt(); + } + + $this->assertSame( 'Foo', $found ); + } + + /** + * @ticket 42814 + */ + public function test_should_fall_back_on_post_content_if_excerpt_is_empty_and_post_is_provided() { + $GLOBALS['post'] = self::factory()->post->create_and_get( + array( + 'post_content' => 'Foo', + 'post_excerpt' => '', + ) + ); + $this->assertSame( 'Foo', get_the_excerpt( $GLOBALS['post'] ) ); + } + + /** + * @ticket 42814 + */ + public function test_should_respect_post_parameter_in_the_loop() { + $p1 = self::factory()->post->create_and_get( array( 'post_excerpt' => 'Foo' ) ); + $p2 = self::factory()->post->create_and_get( array( 'post_excerpt' => 'Bar' ) ); + $q = new WP_Query( + array( + 'p' => $p1->ID, + ) + ); + + while ( $q->have_posts() ) { + $q->the_post(); + $found = get_the_excerpt( $p2 ); + } + + $this->assertSame( 'Bar', $found ); + } + + /** + * @ticket 42814 + */ + public function test_should_respect_post_parameter_in_the_loop_when_falling_back_on_post_content() { + $p1 = self::factory()->post->create_and_get( + array( + 'post_content' => 'Foo', + 'post_excerpt' => '', + ) + ); + $p2 = self::factory()->post->create_and_get( + array( + 'post_content' => 'Bar', + 'post_excerpt' => '', + ) + ); + $q = new WP_Query( + array( + 'p' => $p1->ID, + ) + ); + + while ( $q->have_posts() ) { + $q->the_post(); + $found = get_the_excerpt( $p2 ); + } + + $this->assertSame( 'Bar', $found ); + } } diff --git a/tests/phpunit/tests/query/generatePostdata.php b/tests/phpunit/tests/query/generatePostdata.php new file mode 100644 index 0000000000..20525ee9d3 --- /dev/null +++ b/tests/phpunit/tests/query/generatePostdata.php @@ -0,0 +1,145 @@ +post->create_and_get(); + $data = generate_postdata( $p->ID ); + $this->assertSame( $p->ID, $data['id'] ); + } + + /** + * @ticket 42814 + */ + public function test_setup_by_fake_post() { + $fake = new stdClass; + $fake->ID = 98765; + $data = generate_postdata( $fake->ID ); + + // Fails because there's no post with this ID. + $this->assertNotSame( $fake->ID, $data['id'] ); + } + + /** + * @ticket 42814 + */ + public function test_setup_by_postish_object() { + $p = self::factory()->post->create(); + + $post = new stdClass(); + $post->ID = $p; + $data = generate_postdata( $p ); + + $this->assertSame( $p, $data['id'] ); + } + + /** + * @ticket 42814 + */ + public function test_authordata() { + $u = self::factory()->user->create_and_get(); + $p = self::factory()->post->create_and_get( + array( + 'post_author' => $u->ID, + ) + ); + $data = generate_postdata( $p ); + + $this->assertNotEmpty( $data['authordata'] ); + $this->assertEquals( $u, $data['authordata'] ); + } + + /** + * @ticket 42814 + */ + public function test_currentday() { + $p = self::factory()->post->create_and_get( + array( + 'post_date' => '1980-09-09 06:30:00', + ) + ); + $data = generate_postdata( $p ); + + $this->assertSame( '09.09.80', $data['currentday'] ); + } + + public function test_currentmonth() { + $p = self::factory()->post->create_and_get( + array( + 'post_date' => '1980-09-09 06:30:00', + ) + ); + $data = generate_postdata( $p ); + + $this->assertSame( '09', $data['currentmonth'] ); + } + + /** + * @ticket 42814 + */ + public function test_single_page() { + $post = self::factory()->post->create_and_get( + array( + 'post_content' => 'Page 0', + ) + ); + $data = generate_postdata( $post ); + + $this->assertSame( 0, $data['multipage'] ); + $this->assertSame( 1, $data['numpages'] ); + $this->assertEquals( array( 'Page 0' ), $data['pages'] ); + } + + /** + * @ticket 42814 + */ + public function test_multi_page() { + $post = self::factory()->post->create_and_get( + array( + 'post_content' => 'Page 0Page 1Page 2Page 3', + ) + ); + $data = generate_postdata( $post ); + + $this->assertSame( 1, $data['multipage'] ); + $this->assertSame( 4, $data['numpages'] ); + $this->assertEquals( array( 'Page 0', 'Page 1', 'Page 2', 'Page 3' ), $data['pages'] ); + } + + /** + * @ticket 42814 + */ + public function test_nextpage_at_start_of_content() { + $post = self::factory()->post->create_and_get( + array( + 'post_content' => 'Page 1Page 2Page 3', + ) + ); + $data = generate_postdata( $post ); + + $this->assertSame( 1, $data['multipage'] ); + $this->assertSame( 3, $data['numpages'] ); + $this->assertEquals( array( 'Page 1', 'Page 2', 'Page 3' ), $data['pages'] ); + } + + /** + * @ticket 42814 + */ + public function test_trim_nextpage_linebreaks() { + $post = self::factory()->post->create_and_get( + array( + 'post_content' => "Page 0\n\nPage 1\nhas a line break\nPage 2\n\nPage 3", + ) + ); + $data = generate_postdata( $post ); + + $this->assertEquals( array( 'Page 0', "Page 1\nhas a line break", 'Page 2', "\nPage 3" ), $data['pages'] ); + } +}