From 17d48b8a794a8988d6331fd0ad5004a3dac93d39 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 11 Aug 2022 01:18:27 +0200
Subject: [PATCH] [DISCUSS] wpdb::__unset(): bug fix

There were three properties, which were introduced in [30345](https://core.trac.wordpress.org/changeset/30345), which were protected from being overwritten (set).

That protection, however, was not extended to the `__unset()` method, leaving these properties only partially protected.
IMO this is an oversight (bug) in the initial implementation.

This commit fixes this by applying the same protection as was already given to these properties for `__set()`, to these properties in the `__unset()` method.

Includes updating the unit tests to match.

Note: this _could_ be considered a BC-break, in which case, an alternative implementation would be to throw a `_doing_it_wrong()` and to continue to unset the property when so requested.
---
 src/wp-includes/class-wpdb.php          | 23 ++++++++++++++++------
 tests/phpunit/tests/db/magicmethods.php | 26 ++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/src/wp-includes/class-wpdb.php b/src/wp-includes/class-wpdb.php
index e81b9caf790c1..c3af6593545e9 100644
--- a/src/wp-includes/class-wpdb.php
+++ b/src/wp-includes/class-wpdb.php
@@ -718,6 +718,19 @@ class wpdb {
 	 */
 	public $error = null;
 
+	/**
+	 * Declared private/protected properties which should remain accessible via the magic methods for BC reasons.
+	 *
+	 * @since 6.1.0
+	 *
+	 * @var array<string, bool> Key is the property name, value whether setting the property is allowed.
+	 */
+	private $compat_accessible_props = array(
+		'check_current_query' => false,
+		'col_meta'            => false,
+		'table_charset'       => false,
+	);
+
 	/**
 	 * Connects to the database server and selects a database.
 	 *
@@ -785,12 +798,7 @@ public function __get( $name ) {
 	 * @param mixed  $value The value to set.
 	 */
 	public function __set( $name, $value ) {
-		$protected_members = array(
-			'col_meta',
-			'table_charset',
-			'check_current_query',
-		);
-		if ( in_array( $name, $protected_members, true ) ) {
+		if ( isset( $this->compat_accessible_props[ $name ] ) && false === $this->compat_accessible_props[ $name ] ) {
 			return;
 		}
 		$this->$name = $value;
@@ -816,6 +824,9 @@ public function __isset( $name ) {
 	 * @param string $name  The private member to unset
 	 */
 	public function __unset( $name ) {
+		if ( isset( $this->compat_accessible_props[ $name ] ) && false === $this->compat_accessible_props[ $name ] ) {
+			return;
+		}
 		unset( $this->$name );
 	}
 
diff --git a/tests/phpunit/tests/db/magicmethods.php b/tests/phpunit/tests/db/magicmethods.php
index 074f28d4467b3..dba06086daf29 100644
--- a/tests/phpunit/tests/db/magicmethods.php
+++ b/tests/phpunit/tests/db/magicmethods.php
@@ -168,7 +168,6 @@ public function test_magic_methods_declared_properties( $name, $default = null )
 	 * @ticket 56034
 	 *
 	 * @dataProvider data_magic_methods_declared_settable_properties
-	 * @dataProvider data_magic_methods_declared_non_settable_properties
 	 *
 	 * @param string $name Property name.
 	 */
@@ -282,6 +281,31 @@ public function test_magic_methods_declared_properties_non_settable( $name, $def
 		$this->assertSame( $default, $obj->$name, 'Property has been assigned a new value' );
 	}
 
+	/**
+	 * Verify that select declared properties cannot be unset.
+	 *
+	 * Also note that as this test expects an error message, it cannot be combined with the
+	 * test for the other magic methods.
+	 *
+	 * @ticket 56034
+	 *
+	 * @dataProvider data_magic_methods_declared_non_settable_properties
+	 *
+	 * @param string $name    Property name.
+	 * @param mixed  $default Default value for the property.
+	 */
+	public function test_magic_unset_declared_properties_non_settable( $name, $default ) {
+		$obj = $this->get_clean_wpdb();
+
+		// Verify initial state. Note: each of these properties has a default value.
+		$this->assertTrue( isset( $obj->$name ), 'Unexpected initial state' );
+
+		// Unset the property value and verify the new state.
+		unset( $obj->$name );
+		$this->assertTrue( isset( $obj->$name ), 'Unsetting the property succeeded' );
+		$this->assertSame( $default, $obj->$name, 'Property has been unset' );
+	}
+
 	/**
 	 * Data provider.
 	 *