From c0f52496f7e9f6307db8590bc2bb9d726e907b09 Mon Sep 17 00:00:00 2001 From: Jonathan Desrosiers Date: Wed, 9 Nov 2022 18:33:01 +0000 Subject: [PATCH] Query: Bypass caching for filtered `SELECT`s. Bypass caching within `WP_Query` when the `SELECT` clause has been modified via a filter. This prevents both cache key collisions and the returning of incomplete or unexpected results when the `SELECT` clause has been modified by an extender. Props pypwalters, claytoncollie, johnwatkins0, TimothyBlynJacobs, costdev, spacedmonkey, peterwilsoncc. Merges [54768] to the 6.1 branch. Fixes #57012. git-svn-id: https://develop.svn.wordpress.org/branches/6.1@54780 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/class-wp-query.php | 15 ++ tests/phpunit/tests/query/fieldsClause.php | 242 +++++++++++++++++++++ 2 files changed, 257 insertions(+) create mode 100644 tests/phpunit/tests/query/fieldsClause.php diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index 511c4a7eec..69efdf5e2a 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -3103,8 +3103,23 @@ class WP_Query { * cannot be cached. Note the space before `RAND` in the string * search, that to ensure against a collision with another * function. + * + * If `$fields` has been modified by the `posts_fields`, + * `posts_fields_request`, `post_clauses` or `posts_clauses_request` + * filters, then caching is disabled to prevent caching collisions. */ $id_query_is_cacheable = ! str_contains( strtoupper( $orderby ), ' RAND(' ); + + $cachable_field_values = array( + "{$wpdb->posts}.*", + "{$wpdb->posts}.ID, {$wpdb->posts}.post_parent", + "{$wpdb->posts}.ID", + ); + + if ( ! in_array( $fields, $cachable_field_values, true ) ) { + $id_query_is_cacheable = false; + } + if ( $q['cache_results'] && $id_query_is_cacheable ) { $new_request = str_replace( $fields, "{$wpdb->posts}.*", $this->request ); $cache_key = $this->generate_cache_key( $q, $new_request ); diff --git a/tests/phpunit/tests/query/fieldsClause.php b/tests/phpunit/tests/query/fieldsClause.php new file mode 100644 index 0000000000..4522923a8a --- /dev/null +++ b/tests/phpunit/tests/query/fieldsClause.php @@ -0,0 +1,242 @@ +post->create_many( 5, array( 'post_type' => 'wptests_pt' ) ); + } + + public function set_up() { + parent::set_up(); + /* + * Re-register the CPT for use within each test. + * + * Custom post types are deregistered by the default tear_down method + * so need to be re-registered for each test as WP_Query calls + * get_post_types(). + */ + register_post_type( 'wptests_pt' ); + } + + /** + * Tests limiting the WP_Query fields to the ID and parent sub-set. + * + * @ticket 57012 + */ + public function test_should_limit_fields_to_id_and_parent_subset() { + $query_args = array( + 'post_type' => 'wptests_pt', + 'fields' => 'id=>parent', + ); + + $q = new WP_Query( $query_args ); + + $expected = array(); + foreach ( self::$post_ids as $post_id ) { + // Use array_shift to populate in the reverse order. + array_unshift( + $expected, + (object) array( + 'ID' => $post_id, + 'post_parent' => 0, + ) + ); + } + + $this->assertEquals( $expected, $q->posts, 'Posts property for first query is not of expected form.' ); + $this->assertSame( 5, $q->found_posts, 'Number of found posts is not five.' ); + $this->assertEquals( 1, $q->max_num_pages, 'Number of found pages is not one.' ); + + // Test the second query's results match. + $q2 = new WP_Query( $query_args ); + $this->assertEquals( $expected, $q2->posts, 'Posts property for second query is not in the expected form.' ); + } + + /** + * Tests limiting the WP_Query fields to the IDs only. + * + * @ticket 57012 + */ + public function test_should_limit_fields_to_ids() { + $query_args = array( + 'post_type' => 'wptests_pt', + 'fields' => 'ids', + ); + + $q = new WP_Query( $query_args ); + + $expected = array_reverse( self::$post_ids ); + + $this->assertEquals( $expected, $q->posts, 'Posts property for first query is not of expected form.' ); + $this->assertSame( 5, $q->found_posts, 'Number of found posts is not five.' ); + $this->assertEquals( 1, $q->max_num_pages, 'Number of found pages is not one.' ); + + // Test the second query's results match. + $q2 = new WP_Query( $query_args ); + $this->assertEquals( $expected, $q2->posts, 'Posts property for second query is not in the expected form.' ); + } + + /** + * Tests querying all fields via WP_Query. + * + * @ticket 57012 + */ + public function test_should_query_all_fields() { + $query_args = array( + 'post_type' => 'wptests_pt', + 'fields' => 'all', + ); + + $q = new WP_Query( $query_args ); + + $expected = array_map( 'get_post', array_reverse( self::$post_ids ) ); + + $this->assertEquals( $expected, $q->posts, 'Posts property for first query is not of expected form.' ); + $this->assertSame( 5, $q->found_posts, 'Number of found posts is not five.' ); + $this->assertEquals( 1, $q->max_num_pages, 'Number of found pages is not one.' ); + + // Test the second query's results match. + $q2 = new WP_Query( $query_args ); + $this->assertEquals( $expected, $q2->posts, 'Posts property for second query is not in the expected form.' ); + } + + /** + * Tests adding fields to WP_Query via filters when requesting the ID and parent sub-set. + * + * @ticket 57012 + */ + public function test_should_include_filtered_values_in_addition_to_id_and_parent_subset() { + add_filter( 'posts_fields', array( $this, 'filter_posts_fields' ) ); + add_filter( 'posts_clauses', array( $this, 'filter_posts_clauses' ) ); + + $query_args = array( + 'post_type' => 'wptests_pt', + 'fields' => 'id=>parent', + ); + + $q = new WP_Query( $query_args ); + + $expected = array(); + foreach ( self::$post_ids as $post_id ) { + // Use array_shift to populate in the reverse order. + array_unshift( + $expected, + (object) array( + 'ID' => $post_id, + 'post_parent' => 0, + 'test_post_fields' => 1, + 'test_post_clauses' => 2, + ) + ); + } + + $this->assertEquals( $expected, $q->posts, 'Posts property for first query is not of expected form.' ); + $this->assertSame( 5, $q->found_posts, 'Number of found posts is not five.' ); + $this->assertEquals( 1, $q->max_num_pages, 'Number of found pages is not one.' ); + + // Test the second query's results match. + $q2 = new WP_Query( $query_args ); + $this->assertEquals( $expected, $q2->posts, 'Posts property for second query is not in the expected form.' ); + } + + /** + * Tests adding fields to WP_Query via filters when requesting the ID field. + * + * @ticket 57012 + */ + public function test_should_include_filtered_values_in_addition_to_id() { + add_filter( 'posts_fields', array( $this, 'filter_posts_fields' ) ); + add_filter( 'posts_clauses', array( $this, 'filter_posts_clauses' ) ); + + $query_args = array( + 'post_type' => 'wptests_pt', + 'fields' => 'ids', + ); + + $q = new WP_Query( $query_args ); + + // Fields => ID does not include the additional fields. + $expected = array_reverse( self::$post_ids ); + + $this->assertEquals( $expected, $q->posts, 'Posts property for first query is not of expected form.' ); + $this->assertSame( 5, $q->found_posts, 'Number of found posts is not five.' ); + $this->assertEquals( 1, $q->max_num_pages, 'Number of found pages is not one.' ); + + // Test the second query's results match. + $q2 = new WP_Query( $query_args ); + $this->assertEquals( $expected, $q2->posts, 'Posts property for second query is not in the expected form.' ); + } + + /** + * Tests adding fields to WP_Query via filters when requesting all fields. + * + * @ticket 57012 + */ + public function test_should_include_filtered_values() { + add_filter( 'posts_fields', array( $this, 'filter_posts_fields' ) ); + add_filter( 'posts_clauses', array( $this, 'filter_posts_clauses' ) ); + + $query_args = array( + 'post_type' => 'wptests_pt', + 'fields' => 'all', + ); + + $q = new WP_Query( $query_args ); + + $expected = array_map( 'get_post', array_reverse( self::$post_ids ) ); + foreach ( $expected as $post ) { + $post->test_post_fields = 1; + $post->test_post_clauses = 2; + } + + $this->assertEquals( $expected, $q->posts, 'Posts property for first query is not of expected form.' ); + $this->assertSame( 5, $q->found_posts, 'Number of found posts is not five.' ); + $this->assertEquals( 1, $q->max_num_pages, 'Number of found pages is not one.' ); + + // Test the second query's results match. + $q2 = new WP_Query( $query_args ); + $this->assertEquals( $expected, $q2->posts, 'Posts property for second query is not in the expected form.' ); + } + + /** + * Filters the posts fields. + * + * @param string $fields The fields to SELECT. + * @return string The filtered fields. + */ + function filter_posts_fields( $fields ) { + return "$fields, 1 as test_post_fields"; + } + + /** + * Filters the posts clauses. + * + * @param array $clauses The WP_Query database clauses. + * @return array The filtered database clauses. + */ + function filter_posts_clauses( $clauses ) { + $clauses['fields'] .= ', 2 as test_post_clauses'; + return $clauses; + } +}