WP_Query::generate_cache_key should consider changes to request via `posts_request_ids` filter.

[thekt12]

In WP_Query::get_posts(), ​$cache_key = $this->generate_cache_key( $q, $new_request ); doesn't consider the possibility for changes to the request via ​posts_request_ids filter.

This can result in cache collision.

For example we have one page doing get_posts with this filter,
and we have a different page that doesn't use the filter but does the same get_posts with exactly same args. This will result in two different request to have same result.

To give overview, posts_request_idsfilter is used to modify final post_ids in a split operation.

Solution is to have check for has_filter( 'posts_request_ids' ), serialize outcome from global $wp_filter[ 'posts_request_ids' ] and set to to args before cache_key is generated. Seralization we also allow for the cases where just one of he array of filters set in posts_request_ids is modified.

Trac ticket: https://core.trac.wordpress.org/ticket/59661#ticket

[joemcgill]

During todays performance focus bug scrub, @thekt12 confirmed that this is a bug, but not a performance issue, since fixing this doesn't lead to any performance impact. I'm going to move to Future Release for now.

@thekt12 the description makes sense to me, but it would be nice to have some example code that someone could use to reproduce the issue more easily.

[thekt12]

A sample code to indicate the issue. In the following code I am running two queries, each should result in a different output. However, the way we generate the cache key, causes both the query to return the same output.

// IGNORE THIS 
add_action( 'init', 'add_few_posts_to_test' );
function add_few_posts_to_test() {
	// create 2 post with slug 'a' and 'b' if it doesn't exist
	$slugs = [ 'a', 'b', 'c' ];
	foreach ( $slugs as $slug ) {
		$post = get_page_by_path( $slug, OBJECT, 'post' );
		if ( ! $post ) {
			$post = wp_insert_post(
				array(
					'post_title'  => $slug,
					'post_status' => 'publish',
					'post_type'   => 'post',
					'post_name'   => $slug,
				)
			);
		}
	}
}

// ACTUAL ISSUE ->
add_filter( 'posts_request_ids', 'wp_posts_reqs', 10, 2 );
function wp_posts_reqs( $request, $query ) {
	if( $query->query_vars['post_name__in'] === [ 'a', 'b', 'c' ] ) {
		$request = str_replace( "'a',", '', $request );
	}
	return $request;
}

add_action( 'init', 'test_query_aa' );
function test_query_aa() {
	echo '<pre>';
	echo "Query has posts_request_ids filter - ". (int)(bool) has_filter( 'posts_request_ids', 'wp_posts_reqs' ) . "<br>";
	$queryR_1 = get_posts(
		array(
			'cache_results'  => true,
			'post_type'      => 'post',
			'post_name__in'  => [ 'a', 'b', 'c' ],
		)
	);
	print_r( $queryR_1 );
	remove_filter( 'posts_request_ids', 'wp_posts_reqs' );
	echo "<br>Query has posts_request_ids filter after removal - ".(int)(bool) has_filter( 'posts_request_ids', 'wp_posts_reqs' ) . "<br>";
	$queryR_2 = get_posts(
		array(
			'cache_results'  => true,
			'post_type'      => 'post',
			'post_name__in'  => [ 'a', 'b', 'c' ]
		)
	);
	print_r( $queryR_2 );
	die( '</pre>' );
}

​PR 5513 address the above scenario.

However, the PR doesn't address the problem of dynamic conditional logic inside 'posts_request_ids' like the code given below-

add_filter( 'posts_request_ids', 'wp_posts_reqs', 10, 2 );
function wp_posts_reqs( $request, $query ) {
	if( $query->query_vars['post_name__in'] === [ 'a', 'b', 'c' ] ) {
		static $count = 0;
		if( $count%2 === 0 ) {
			$request = str_replace( "'a',", '', $request );
		}
		$count++;
	}
	return $request;
}

add_action( 'init', 'test_query_aa' );
function test_query_aa() {
	echo '<pre>';
	echo "Query has posts_request_ids filter - ". (int)(bool) has_filter( 'posts_request_ids', 'wp_posts_reqs' ) . "<br>";
	$queryR_1 = get_posts(
		array(
			'cache_results'  => true,
			'post_type'      => 'post',
			'post_name__in'  => [ 'a', 'b', 'c' ],
		)
	);
	print_r( $queryR_1 );
	$queryR_2 = get_posts(
		array(
			'cache_results'  => true,
			'post_type'      => 'post',
			'post_name__in'  => [ 'a', 'b', 'c' ]
		)
	);
	print_r( $queryR_2 );
	echo '</pre>';
	die();
}

For the second scenario, disabling cache_results for the query is the only solution I could think of at the moment.