From 7bab32c792a3a6d71b46cb2b9a82ce58068c68c7 Mon Sep 17 00:00:00 2001 From: Luca Tumedei Date: Fri, 29 Nov 2024 11:51:08 +0100 Subject: [PATCH] fix(PidBasedController) check PID maps to running process This fixes the issue where one of the managed processes (Chromedriver, PHP built-in server or MySQL server) would be killed by the system or user outside of wp-browser control leaving an orphaned PID file pointing at a no-more-running process. --- CHANGELOG.md | 4 + src/Extension/BuiltInServerController.php | 2 +- src/Extension/ChromeDriverController.php | 2 +- src/Extension/MysqlServerController.php | 2 +- src/Extension/PidBasedController.php | 48 ++++++++++ .../Extension/PidBasedControllerTest.php | 88 +++++++++++++++++++ .../WPLoaderArbitraryPluginLocationTest.php | 67 +++++++++++++- 7 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 tests/unit/lucatume/WPBrowser/Extension/PidBasedControllerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a7a9039a..0d04dbfee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [unreleased] Unreleased +## Fixed + +- Check the PID file for the PHP built-in server, MySQL and Chromedriver controllers to make sure the PID maps to an actually running process. + ## [4.3.9] 2024-11-29; ## Changed diff --git a/src/Extension/BuiltInServerController.php b/src/Extension/BuiltInServerController.php index 9ffb9ee28..28f2bf870 100644 --- a/src/Extension/BuiltInServerController.php +++ b/src/Extension/BuiltInServerController.php @@ -19,7 +19,7 @@ public function start(OutputInterface $output): void { $pidFile = $this->getPidFile(); - if (is_file($pidFile)) { + if ($this->isProcessRunning($pidFile)) { $output->writeln('PHP built-in server already running.'); return; } diff --git a/src/Extension/ChromeDriverController.php b/src/Extension/ChromeDriverController.php index d425e3bae..8f61f6afc 100644 --- a/src/Extension/ChromeDriverController.php +++ b/src/Extension/ChromeDriverController.php @@ -20,7 +20,7 @@ public function start(OutputInterface $output): void { $pidFile = $this->getPidFile(); - if (is_file($pidFile)) { + if ($this->isProcessRunning($pidFile)) { $output->writeln('ChromeDriver already running.'); return; diff --git a/src/Extension/MysqlServerController.php b/src/Extension/MysqlServerController.php index c6b45d4f3..9f5b43cfa 100644 --- a/src/Extension/MysqlServerController.php +++ b/src/Extension/MysqlServerController.php @@ -20,7 +20,7 @@ public function start(OutputInterface $output): void { $pidFile = $this->getPidFile(); - if (is_file($pidFile)) { + if ($this->isProcessRunning($pidFile)) { $output->writeln('MySQL server already running.'); return; diff --git a/src/Extension/PidBasedController.php b/src/Extension/PidBasedController.php index 406f4688e..de81375a7 100644 --- a/src/Extension/PidBasedController.php +++ b/src/Extension/PidBasedController.php @@ -3,6 +3,7 @@ namespace lucatume\WPBrowser\Extension; use Codeception\Exception\ExtensionException; +use lucatume\WPBrowser\Exceptions\RuntimeException; trait PidBasedController { @@ -49,4 +50,51 @@ protected function removePidFile(string $pidFile): void ); } } + + /** + * @throws RuntimeException + */ + protected function isProcessRunning(string $pidFile):bool + { + if (!is_file($pidFile)) { + return false; + } + + try { + $pidFileContents = file_get_contents($pidFile); + if ($pidFileContents === false) { + throw new \Exception(); + } + } catch (\Exception $e) { + throw new RuntimeException( + "Failed to read PID file '{$pidFile}'." + ); + } + + $pid = trim($pidFileContents); + + if (!is_numeric($pid) || (int)$pid === 0) { + return false; + } + + if (PHP_OS_FAMILY === 'Windows') { + $output = []; + exec("tasklist /FI \"PID eq $pid\" 2>NUL", $output); + + return str_contains(implode("\n", $output), $pid); + } else { + // Check if the process is running on POSIX (Mac or Linux) + exec("ps -p $pid", $output, $resultCode); + if ($resultCode === 0 && count($output) > 1) { + // Process is running + return true; + } + } + + if (!unlink($pidFile)) { + throw new RuntimeException("Failed to delete PID file: $pidFile"); + } + + return false; + } } diff --git a/tests/unit/lucatume/WPBrowser/Extension/PidBasedControllerTest.php b/tests/unit/lucatume/WPBrowser/Extension/PidBasedControllerTest.php new file mode 100644 index 000000000..ed3bc244e --- /dev/null +++ b/tests/unit/lucatume/WPBrowser/Extension/PidBasedControllerTest.php @@ -0,0 +1,88 @@ +isProcessRunning($pidFile); + } + }; + $hash = md5(microtime()); + $pidFile = sys_get_temp_dir()."/test-{$hash}.pid"; + $pid = posix_getpid(); + if(!file_put_contents($pidFile,$pid)){ + $this->fail('Could not write pid to file '.$pidFile); + } + + $this->assertTrue($testClass->openIsProcessRunning($pidFile)); + $this->assertFileExists($pidFile); + } + + public function test_isProcessRunning_returns_false_if_pid_file_not_exists():void{ + $testClass = new class { + use PidBasedController; + + public function openIsProcessRunning(string $pidFile):bool{ + return $this->isProcessRunning($pidFile); + } + }; + $pid = posix_getpid(); + + $this->assertFalse($testClass->openIsProcessRunning(__DIR__ .'/test.pid')); + } + + public function test_isProcessRunning_throws_if_pid_file_cannot_be_read():void{ + $testClass = new class { + use PidBasedController; + + public function openIsProcessRunning(string $pidFile):bool{ + return $this->isProcessRunning($pidFile); + } + }; + $pid = posix_getpid(); + $hash = md5(microtime()); + $pidFile = sys_get_temp_dir()."/test-{$hash}.pid"; + $pid = posix_getpid(); + if(!file_put_contents($pidFile,$pid)){ + $this->fail('Could not write pid to file '.$pidFile); + } + // Change the file mode to not be readable by the current user. + chmod($pidFile,0000); + + $this->expectException(RuntimeException::class); + + $testClass->openIsProcessRunning($pidFile); + } + + public function test_isProcessRunning_returns_false_if_process_is_not_running():void{ + $testClass = new class { + use PidBasedController; + + public function openIsProcessRunning(string $pidFile):bool{ + return $this->isProcessRunning($pidFile); + } + }; + $hash = md5(microtime()); + $pidFile = sys_get_temp_dir()."/test-{$hash}.pid"; + $process = new Process(['echo', '23']); + $process->start(); + $pid = $process->getPid(); + $process->wait(); + if(!file_put_contents($pidFile,$pid)){ + $this->fail('Could not write pid to file '.$pidFile); + } + + $this->assertFalse($testClass->openIsProcessRunning($pidFile)); + $this->assertFileNotExists($pidFile); + } +} diff --git a/tests/unit/lucatume/WPBrowser/Module/WPLoaderArbitraryPluginLocationTest.php b/tests/unit/lucatume/WPBrowser/Module/WPLoaderArbitraryPluginLocationTest.php index b3b030b86..b236daa47 100644 --- a/tests/unit/lucatume/WPBrowser/Module/WPLoaderArbitraryPluginLocationTest.php +++ b/tests/unit/lucatume/WPBrowser/Module/WPLoaderArbitraryPluginLocationTest.php @@ -5,7 +5,6 @@ use Codeception\Lib\Di; use Codeception\Lib\ModuleContainer; use Codeception\Test\Unit; -use lucatume\WPBrowser\Module\WPLoader; use lucatume\WPBrowser\Tests\Traits\DatabaseAssertions; use lucatume\WPBrowser\Tests\Traits\LoopIsolation; use lucatume\WPBrowser\Tests\Traits\MainInstallationAccess; @@ -56,6 +55,72 @@ private function module(array $moduleContainerConfig = [], ?array $moduleConfig return new WPLoader($this->mockModuleContainer, ($moduleConfig ?? $this->config)); } + public function test_loads_plugins_from_default_location_correctly(): void + { + $projectDir = FS::tmpDir('wploader_'); + $installation = Installation::scaffold($projectDir); + $dbName = Random::dbName(); + $dbHost = Env::get('WORDPRESS_DB_HOST'); + $dbUser = Env::get('WORDPRESS_DB_USER'); + $dbPassword = Env::get('WORDPRESS_DB_PASSWORD'); + $installationDb = new MysqlDatabase($dbName, $dbUser, $dbPassword, $dbHost, 'wp_'); + if (!mkdir($projectDir . '/wp-content/plugins/test-one', 0777, true)) { + throw new \RuntimeException('Unable to create test directory for plugin test-one'); + } + if (!file_put_contents( + $projectDir . '/wp-content/plugins/test-one/plugin.php', + <<< PHP +config = [ + 'wpRootFolder' => $projectDir, + 'dbUrl' => $installationDb->getDbUrl(), + 'tablePrefix' => 'test_', + 'plugins' => [ + 'test-one/plugin.php', + 'test-two/plugin.php', + ] + ]; + $wpLoader = $this->module(); + $projectDirname = basename($projectDir); + + $this->assertInIsolation( + static function () use ($wpLoader, $projectDir) { + chdir($projectDir); + + $wpLoader->_initialize(); + + Assert::assertTrue(function_exists('test_one_loaded')); + Assert::assertTrue(function_exists('test_two_loaded')); + } + ); + } + /** * It should allow loading a plugin from an arbitrary path *