Merge pull request #768 from WordPress/fix/potential-runtime-side-eff…

…ects-real-tables

includes/Admin/Admin_AJAX.php

@@ -145,35 +145,24 @@ public function set_up_environment() {
 	 * Handles the AJAX request to cleanup the runtime environment.
 	 *
 	 * @since 1.0.0
-	 *
-	 * @global wpdb   $wpdb         WordPress database abstraction object.
-	 * @global string $table_prefix The database table prefix.
 	 */
 	public function clean_up_environment() {
-		global $wpdb, $table_prefix;
-
 		// Verify the nonce before continuing.
 		$valid_request = $this->verify_request( filter_input( INPUT_POST, 'nonce', FILTER_SANITIZE_FULL_SPECIAL_CHARS ) );
 
 		if ( is_wp_error( $valid_request ) ) {
 			wp_send_json_error( $valid_request, 403 );
 		}
 
-		// Set the new prefix.
-		$old_prefix = $wpdb->set_prefix( $table_prefix . 'pc_' );
-
-		$message = __( 'Runtime environment was not prepared, cleanup was not run.', 'plugin-check' );
-
-		// Test if the runtime environment tables exist.
-		if ( $wpdb->posts === $wpdb->get_var( $wpdb->prepare( 'SHOW TABLES LIKE %s', $wpdb->posts ) ) || defined( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION' ) ) {
-			$runtime = new Runtime_Environment_Setup();
+		// Test if the runtime environment is prepared (and thus needs cleanup).
+		$runtime = new Runtime_Environment_Setup();
+		if ( $runtime->is_set_up() ) {
 			$runtime->clean_up();
 			$message = __( 'Runtime environment cleanup successful.', 'plugin-check' );
+		} else {
+			$message = __( 'Runtime environment was not prepared, cleanup was not run.', 'plugin-check' );
 		}
 
-		// Restore the old prefix.
-		$wpdb->set_prefix( $old_prefix );
-
 		wp_send_json_success(
 			array(
 				'message' => $message,

includes/Checker/Abstract_Check_Runner.php

@@ -358,22 +358,12 @@ final public function prepare() {
 	 *
 	 * @since 1.0.0
 	 *
-	 * @global wpdb   $wpdb         WordPress database abstraction object.
-	 * @global string $table_prefix The database table prefix.
-	 *
 	 * @return Check_Result An object containing all check results.
 	 */
 	final public function run() {
-		global $wpdb, $table_prefix;
 		$checks       = $this->get_checks_to_run();
 		$preparations = $this->get_shared_preparations( $checks );
 		$cleanups     = array();
-		$old_prefix   = null;
-
-		// Set the correct test database prefix if required.
-		if ( $this->has_runtime_check( $checks ) ) {
-			$old_prefix = $wpdb->set_prefix( $table_prefix . 'pc_' );
-		}
 
 		// Prepare all shared preparations.
 		foreach ( $preparations as $preparation ) {
@@ -389,11 +379,6 @@ final public function run() {
 			}
 		}
 
-		// Restore the old prefix.
-		if ( $old_prefix ) {
-			$wpdb->set_prefix( $old_prefix );
-		}
-
 		return $results;
 	}
 

includes/Checker/Preparations/Universal_Runtime_Preparation.php

@@ -61,12 +61,18 @@ public function prepare() {
 			$theme_folder = WP_PLUGIN_CHECK_PLUGIN_DIR_PATH . 'runtime-content/themes';
 		}
 
+		$use_custom_db_tables_preparation = new Use_Custom_DB_Tables_Preparation();
+		$cleanup_functions[]              = $use_custom_db_tables_preparation->prepare();
+
 		$use_minimal_theme_preparation = new Use_Minimal_Theme_Preparation( 'wp-empty-theme', $theme_folder );
 		$cleanup_functions[]           = $use_minimal_theme_preparation->prepare();
 
 		$force_single_plugin_preparation = new Force_Single_Plugin_Preparation( $this->check_context->basename() );
 		$cleanup_functions[]             = $force_single_plugin_preparation->prepare();
 
+		// Revert order so that earlier preparations are cleaned up later.
+		$cleanup_functions = array_reverse( $cleanup_functions );
+
 		// Return the cleanup function.
 		return function () use ( $cleanup_functions ) {
 

includes/Checker/Preparations/Use_Custom_DB_Tables_Preparation.php

@@ -0,0 +1,46 @@
+<?php
+/**
+ * Class WordPress\Plugin_Check\Checker\Preparations\Use_Custom_DB_Tables_Preparation
+ *
+ * @package plugin-check
+ */
+
+namespace WordPress\Plugin_Check\Checker\Preparations;
+
+use Exception;
+use WordPress\Plugin_Check\Checker\Preparation;
+
+/**
+ * Class for the preparation step to use the custom database tables.
+ *
+ * This ensures no side effects on the actual database tables are possible.
+ *
+ * @since 1.3.0
+ */
+class Use_Custom_DB_Tables_Preparation implements Preparation {
+
+	/**
+	 * Runs this preparation step for the environment and returns a cleanup function.
+	 *
+	 * @since 1.3.0
+	 *
+	 * @global wpdb   $wpdb         WordPress database abstraction object.
+	 * @global string $table_prefix The database table prefix.
+	 *
+	 * @return callable Cleanup function to revert any changes made here.
+	 *
+	 * @throws Exception Thrown when preparation fails.
+	 */
+	public function prepare() {
+		global $wpdb, $table_prefix;
+
+		$old_prefix = $wpdb->set_prefix( $table_prefix . 'pc_' );
+
+		// Return the cleanup function.
+		return function () use ( $old_prefix ) {
+			global $wpdb;
+
+			$wpdb->set_prefix( $old_prefix );
+		};
+	}
+}

includes/Checker/Runtime_Environment_Setup.php

@@ -137,6 +137,37 @@ public function clean_up() {
 		}
 	}
 
+	/**
+	 * Tests if the runtime environment is currently set up.
+	 *
+	 * This returns true when the plugin's object-cache.php drop-in is active in the current request and/or when the
+	 * custom runtime environment database tables are present.
+	 *
+	 * @since 1.3.0
+	 *
+	 * @global wpdb   $wpdb         WordPress database abstraction object.
+	 * @global string $table_prefix The database table prefix.
+	 *
+	 * @return bool True if the runtime environment is set up, false if not.
+	 */
+	public function is_set_up() {
+		global $wpdb, $table_prefix;
+
+		if ( defined( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION' ) ) {
+			return true;
+		}
+
+		// Set the custom prefix to check for the runtime environment tables.
+		$old_prefix = $wpdb->set_prefix( $table_prefix . 'pc_' );
+
+		$tables_present = $wpdb->posts === $wpdb->get_var( $wpdb->prepare( 'SHOW TABLES LIKE %s', $wpdb->posts ) );
+
+		// Restore the old prefix.
+		$wpdb->set_prefix( $old_prefix );
+
+		return $tables_present;
+	}
+
 	/**
 	 * Checks if the WordPress Environment can be set up for runtime checks.
 	 *

tests/phpunit/tests/Checker/AJAX_Runner_Tests.php

@@ -7,6 +7,7 @@
 
 use WordPress\Plugin_Check\Checker\AJAX_Runner;
 use WordPress\Plugin_Check\Checker\Check_Result;
+use WordPress\Plugin_Check\Checker\Runtime_Environment_Setup;
 use WordPress\Plugin_Check\Test_Data\Empty_Check;
 use WordPress\Plugin_Check\Test_Data\Error_Check;
 use WordPress\Plugin_Check\Test_Data\Runtime_Check;
@@ -16,15 +17,28 @@ class AJAX_Runner_Tests extends WP_UnitTestCase {
 
 	use With_Mock_Filesystem;
 
+	/**
+	 * Storage for preparation cleanups that need to be run after a test.
+	 *
+	 * @var array
+	 */
+	private $cleanups = array();
+
 	public function set_up() {
+		parent::set_up();
+
 		// Setup the mock filesystem so the Runtime_Environment_Setup works correctly within the AJAX_Runner.
 		$this->set_up_mock_filesystem();
 	}
 
 	public function tear_down() {
-		// Force reset the database prefix after runner prepare method called.
-		global $wpdb, $table_prefix;
-		$wpdb->set_prefix( $table_prefix );
+		if ( count( $this->cleanups ) > 0 ) {
+			$this->cleanups = array_reverse( $this->cleanups );
+			foreach ( $this->cleanups as $cleanup ) {
+				$cleanup();
+			}
+			$this->cleanups = array();
+		}
 		parent::tear_down();
 	}
 
@@ -77,8 +91,19 @@ function ( $checks ) {
 		$muplugins_loaded = $wp_actions['muplugins_loaded'];
 		unset( $wp_actions['muplugins_loaded'] );
 
-		$runner  = new AJAX_Runner();
-		$cleanup = $runner->prepare();
+		/*
+		 * The runtime environment must be prepared manually before regular runtime preparations.
+		 * This is necessary because in reality it happens in a separate AJAX request before.
+		 */
+		$runtime = new Runtime_Environment_Setup();
+		$runtime->set_up();
+		$this->cleanups[] = function () use ( $runtime ) {
+			$runtime->clean_up();
+		};
+
+		$runner           = new AJAX_Runner();
+		$cleanup          = $runner->prepare();
+		$this->cleanups[] = $cleanup;
 
 		$wp_actions['muplugins_loaded'] = $muplugins_loaded;
 
@@ -111,8 +136,9 @@ function ( $checks ) {
 			}
 		);
 
-		$runner  = new AJAX_Runner();
-		$cleanup = $runner->prepare();
+		$runner           = new AJAX_Runner();
+		$cleanup          = $runner->prepare();
+		$this->cleanups[] = $cleanup;
 
 		$this->assertIsCallable( $cleanup );
 
@@ -143,8 +169,10 @@ function ( $checks ) {
 			}
 		);
 
-		$runner = new AJAX_Runner();
-		$runner->prepare();
+		$runner           = new AJAX_Runner();
+		$cleanup          = $runner->prepare();
+		$this->cleanups[] = $cleanup;
+
 		$results = $runner->run();
 
 		$this->assertInstanceOf( Check_Result::class, $results );
@@ -167,8 +195,10 @@ function ( $checks ) {
 			}
 		);
 
-		$runner = new AJAX_Runner();
-		$runner->prepare();
+		$runner           = new AJAX_Runner();
+		$cleanup          = $runner->prepare();
+		$this->cleanups[] = $cleanup;
+
 		$results = $runner->run();
 
 		$this->assertInstanceOf( Check_Result::class, $results );
@@ -196,7 +226,9 @@ public function test_runner_initialized_early_throws_plugin_basename_exception()
 
 		$runner->set_plugin( 'invalid-plugin' );
 
-		$runner->prepare();
+		$cleanup          = $runner->prepare();
+		$this->cleanups[] = $cleanup;
+
 		$runner->run();
 	}
 
@@ -220,7 +252,9 @@ public function test_runner_initialized_early_throws_checks_exception() {
 
 		$runner->set_check_slugs( array( 'runtime_check' ) );
 
-		$runner->prepare();
+		$cleanup          = $runner->prepare();
+		$this->cleanups[] = $cleanup;
+
 		$runner->run();
 	}
 }