From 2724a495afb2f7c60d4b2c6b9158c4c82973fb58 Mon Sep 17 00:00:00 2001
From: Jonny Harris <spacedmonkey@git.wordpress.org>
Date: Tue, 14 Mar 2023 15:51:28 +0000
Subject: [PATCH] Cache API: Add a warning when calling `_get_non_cached_ids`
 with invalid ids.

Sanitize the array of ids passed to the `_get_non_cached_ids` function and add a `_doing_it_wrong` call, if an invalid type is passed.

Props tillkruess, spacedmonkey, peterwilsoncc, flixos90, SergeyBiryukov.
Fixes #57593.

git-svn-id: https://develop.svn.wordpress.org/trunk@55543 602fd350-edb4-49c9-b593-d223f7449a82
---
 src/wp-includes/functions.php                 |  31 ++++++
 .../tests/functions/getNonCachedIds.php       | 102 ++++++++++++++++++
 2 files changed, 133 insertions(+)
 create mode 100644 tests/phpunit/tests/functions/getNonCachedIds.php

diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php
index 8a02df2d5c..e3e281b1d6 100644
--- a/src/wp-includes/functions.php
+++ b/src/wp-includes/functions.php
@@ -7014,6 +7014,13 @@ function wp_debug_backtrace_summary( $ignore_class = null, $skip_frames = 0, $pr
  * @return int[] Array of IDs not present in the cache.
  */
 function _get_non_cached_ids( $object_ids, $cache_key ) {
+	$object_ids = array_filter( $object_ids, '_validate_cache_id' );
+	$object_ids = array_unique( array_map( 'intval', $object_ids ), SORT_NUMERIC );
+
+	if ( empty( $object_ids ) ) {
+		return array();
+	}
+
 	$non_cached_ids = array();
 	$cache_values   = wp_cache_get_multiple( $object_ids, $cache_key );
 
@@ -7026,6 +7033,30 @@ function _get_non_cached_ids( $object_ids, $cache_key ) {
 	return $non_cached_ids;
 }
 
+/**
+ * Checks whether the given cache ID is either an integer or iterger-like strings.
+ * Both `16` and `"16"` are considered valid, other numeric types and numeric
+ * strings (`16.3` and `"16.3"`) are considered invalid.
+ *
+ * @since 6.3.0
+ *
+ * @param mixed $object_id The cache id to validate.
+ * @return bool Whether the given $object_id is a valid cache id.
+ */
+function _validate_cache_id( $object_id ) {
+	// Unfortunately filter_var() is considered an optional extension
+	if ( is_int( $object_id )
+		|| ( is_string( $object_id ) && (string) (int) $object_id === $object_id ) ) {
+		return true;
+	}
+
+	/* translators: %s: The type of the given object id. */
+	$message = sprintf( __( 'Object id must be integer, %s given.' ), gettype( $object_id ) );
+	_doing_it_wrong( '_get_non_cached_ids', $message, '6.3.0' );
+
+	return false;
+}
+
 /**
  * Tests if the current device has the capability to upload files.
  *
diff --git a/tests/phpunit/tests/functions/getNonCachedIds.php b/tests/phpunit/tests/functions/getNonCachedIds.php
new file mode 100644
index 0000000000..300477d8ad
--- /dev/null
+++ b/tests/phpunit/tests/functions/getNonCachedIds.php
@@ -0,0 +1,102 @@
+<?php
+/**
+ * Test class for `_get_non_cached_ids()`.
+ *
+ * @package WordPress
+ *
+ * @group cache
+ *
+ * @covers ::_get_non_cached_ids
+ */
+class Tests_Functions_GetNonCachedIds extends WP_UnitTestCase {
+
+	/**
+	 * @ticket 57593
+	 */
+	public function test_uncached_valid_ids_should_be_unique() {
+		$object_id = 1;
+
+		$this->assertSame(
+			array( $object_id ),
+			_get_non_cached_ids( array( $object_id, $object_id, (string) $object_id ), 'fake-group' ),
+			'Duplicate object IDs should be removed.'
+		);
+	}
+
+	/**
+	 * @ticket 57593
+	 *
+	 * @dataProvider data_valid_ids_should_be_returned_as_integers
+	 *
+	 * @param mixed $object_id The object id.
+	 */
+	public function test_valid_ids_should_be_returned_as_integers( $object_id ) {
+		$this->assertSame(
+			array( (int) $object_id ),
+			_get_non_cached_ids( array( $object_id ), 'fake-group' ),
+			'Object IDs should be returned as integers.'
+		);
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array[]
+	 */
+	public function data_valid_ids_should_be_returned_as_integers() {
+		return array(
+			'(int) 1'    => array( 1 ),
+			'(string) 1' => array( '1' ),
+		);
+	}
+
+	/**
+	 * @ticket 57593
+	 */
+	public function test_mix_of_valid_and_invalid_ids_should_return_the_valid_ids_and_throw_a_notice() {
+		$object_id = 1;
+
+		$this->setExpectedIncorrectUsage( '_get_non_cached_ids' );
+		$this->assertSame(
+			array( $object_id ),
+			_get_non_cached_ids( array( $object_id, null ), 'fake-group' ),
+			'Valid object IDs should be returned.'
+		);
+	}
+
+	/**
+	 * @ticket 57593
+	 *
+	 * @dataProvider data_invalid_cache_ids_should_throw_a_notice
+	 *
+	 * @param mixed $object_id The object id.
+	 */
+	public function test_invalid_cache_ids_should_throw_a_notice( $object_id ) {
+		$this->setExpectedIncorrectUsage( '_get_non_cached_ids' );
+		$this->assertSame(
+			array(),
+			_get_non_cached_ids( array( $object_id ), 'fake-group' ),
+			'Invalid object IDs should be dropped.'
+		);
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array[]
+	 */
+	public function data_invalid_cache_ids_should_throw_a_notice() {
+		return array(
+			'null'         => array( null ),
+			'false'        => array( false ),
+			'true'         => array( true ),
+			'(float) 1.0'  => array( 1.0 ),
+			'(string) 5.0' => array( '5.0' ),
+			'string'       => array( 'johnny cache' ),
+			'empty string' => array( '' ),
+			'array'        => array( array( 1 ) ),
+			'empty array'  => array( array() ),
+			'stdClass'     => array( new stdClass ),
+		);
+	}
+}