Fix Windows proc_open pipe error using Runner interface pattern

.github/workflows/ci.yml

@@ -10,7 +10,7 @@ jobs:
 
     strategy:
       matrix:
-        operating-system: [ubuntu-latest, windows-latest]
+        operating-system: [ubuntu-latest, windows-latest, macos-latest]
         php-versions: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5']
 
     runs-on: ${{ matrix.operating-system }}

src/Exceptions/TempFileException.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace donatj\MockWebServer\Exceptions;
+
+class TempFileException extends RuntimeException {
+
+}

src/MockWebServer.php

@@ -23,18 +23,11 @@ class MockWebServer {
 	private $tmpDir;
 
 	/**
-	 * Contain link to opened process resource
+	 * Platform-specific process runner
 	 *
-	 * @var resource
+	 * @var ProcessRunnerInterface
 	 */
-	private $process;
-
-	/**
-	 * Contains the descriptors for the process after it has been started
-	 *
-	 * @var resource[]
-	 */
-	private $descriptors = [];
+	private $processRunner;
 
 	/**
 	 * TestWebServer constructor.
@@ -50,6 +43,7 @@ public function __construct( int $port = 0, string $host = '127.0.0.1' ) {
 		}
 
 		$this->tmpDir = $this->getTmpDir();
+		$this->processRunner = $this->createProcessRunner();
 	}
 
 	/**
@@ -62,26 +56,11 @@ public function start() : void {
 
 		$script = __DIR__ . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR . 'server' . DIRECTORY_SEPARATOR . 'server.php';
 
-		$stdout = tempnam(sys_get_temp_dir(), 'mockserv-stdout-');
-		$cmd    = sprintf("%s -S %s:%d %s",
-			escapeshellarg(PHP_BINARY),
-			escapeshellarg($this->host),
-			$this->port,
-			escapeshellarg($script)
-		);
-
-		if( !putenv(self::TMP_ENV . '=' . $this->tmpDir) ) {
-			throw new Exceptions\RuntimeException('Unable to put environmental variable');
-		}
-
-		$fullCmd = sprintf('%s > %s 2>&1',
-			$cmd,
-			escapeshellarg($stdout)
-		);
-
 		InternalServer::incrementRequestCounter($this->tmpDir, 0);
 
-		[ $this->process, $this->descriptors ] = $this->startServer($fullCmd);
+		$env = [ self::TMP_ENV => $this->tmpDir ];
+
+		$this->processRunner->startProcess(PHP_BINARY, $this->host, $this->port, $script, $env);
 
 		for( $i = 0; $i <= 20; $i++ ) {
 			usleep(100000);
@@ -96,53 +75,21 @@ public function start() : void {
 		if( !$this->isRunning() ) {
 			throw new Exceptions\ServerException("Failed to start server. Is something already running on port {$this->port}?");
 		}
-
-		register_shutdown_function(function () {
-			if( $this->isRunning() ) {
-				$this->stop();
-			}
-		});
 	}
 
 	/**
 	 * Is the Web Server currently running?
+	 * @phpstan-impure
 	 */
 	public function isRunning() : bool {
-		if( !is_resource($this->process) ) {
-			return false;
-		}
-
-		$processStatus = proc_get_status($this->process);
-
-		if( !$processStatus ) {
-			return false;
-		}
-
-		return $processStatus['running'];
+		return $this->processRunner->isRunning();
 	}
 
 	/**
 	 * Stop the Web Server
 	 */
 	public function stop() : void {
-		if( $this->isRunning() ) {
-			proc_terminate($this->process);
-
-			$attempts = 0;
-			while( $this->isRunning() ) {
-				if( ++$attempts > 1000 ) {
-					throw new Exceptions\ServerException('Failed to stop server.');
-				}
-
-				usleep(10000);
-			}
-		}
-
-		foreach( $this->descriptors as $descriptor ) {
-			@fclose($descriptor);
-		}
-
-		$this->descriptors = [];
+		$this->processRunner->stop();
 	}
 
 	/**
@@ -315,55 +262,14 @@ private function isWindowsPlatform() : bool {
 	}
 
 	/**
-	 * @return array{resource,array{resource,resource,resource}}
+	 * Create the appropriate process runner for the current platform
 	 */
-	private function startServer( string $fullCmd ) : array {
-		if( !$this->isWindowsPlatform() ) {
-			// We need to prefix exec to get the correct process http://php.net/manual/ru/function.proc-get-status.php#93382
-			$fullCmd = 'exec ' . $fullCmd;
-		}
-
-		$pipes = [];
-		$env   = null;
-		$cwd   = null;
-
-		$stdoutf = tempnam(sys_get_temp_dir(), 'MockWebServer.stdout');
-		if( $stdoutf === false ) {
-			throw new RuntimeException('error creating stdout temp file');
-		}
-
-		$stderrf = tempnam(sys_get_temp_dir(), 'MockWebServer.stderr');
-		if( $stderrf === false ) {
-			throw new RuntimeException('error creating stderr temp file');
-		}
-
-		$stdin = fopen('php://stdin', 'rb');
-		if( $stdin === false ) {
-			throw new RuntimeException('error opening stdin');
-		}
-
-		$stdout = fopen($stdoutf, 'ab');
-		if( $stdout === false ) {
-			throw new RuntimeException('error opening stdout');
-		}
-
-		$stderr = fopen($stderrf, 'ab');
-		if( $stderr === false ) {
-			throw new RuntimeException('error opening stderr');
-		}
-
-		$descriptorSpec = [ $stdin, $stdout, $stderr ];
-
-		$process = proc_open($fullCmd, $descriptorSpec, $pipes, $cwd, $env, [
-			'suppress_errors' => false,
-			'bypass_shell'    => true,
-		]);
-
-		if( $process === false ) {
-			throw new Exceptions\ServerException('Error starting server');
+	private function createProcessRunner() : ProcessRunnerInterface {
+		if( $this->isWindowsPlatform() ) {
+			return new ProcessRunners\WindowsProcessRunner;
 		}
 
-		return [ $process, $descriptorSpec ];
+		return new ProcessRunners\PosixProcessRunner;
 	}
 
 }

src/ProcessRunnerInterface.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace donatj\MockWebServer;
+
+/**
+ * Interface for platform-specific process execution
+ */
+interface ProcessRunnerInterface {
+
+	/**
+	 * Start a PHP built-in server process
+	 *
+	 * @param string               $phpBinary Path to the PHP binary
+	 * @param string               $host      Hostname to listen on
+	 * @param int                  $port      Port to listen on
+	 * @param string               $script    Path to the server script
+	 * @param array<string,string> $env       Environment variables to pass to the process
+	 * @throws \donatj\MockWebServer\Exceptions\ServerException If the process fails to start
+	 * @throws \donatj\MockWebServer\Exceptions\RuntimeException If temp file or stream operations fail
+	 * @return resource The process resource
+	 */
+	public function startProcess( string $phpBinary, string $host, int $port, string $script, array $env = [] );
+
+	/**
+	 * Is the process currently running?
+	 */
+	public function isRunning() : bool;
+
+	/**
+	 * Stop the running process
+	 *
+	 * @throws \donatj\MockWebServer\Exceptions\ServerException If the process fails to stop
+	 */
+	public function stop() : void;
+
+}

src/ProcessRunners/AbstractProcessRunner.php

@@ -0,0 +1,79 @@
+<?php
+
+namespace donatj\MockWebServer\ProcessRunners;
+
+use donatj\MockWebServer\Exceptions\ServerException;
+use donatj\MockWebServer\Exceptions\TempFileException;
+use donatj\MockWebServer\ProcessRunnerInterface;
+
+/**
+ * Abstract base for platform-specific process runners.
+ *
+ * Provides shared temporary-file tracking and cleanup logic.
+ */
+abstract class AbstractProcessRunner implements ProcessRunnerInterface {
+
+	public const STDOUT_PREFIX = 'MockWebServer.stdout';
+	public const STDERR_PREFIX = 'MockWebServer.stderr';
+
+	/** @var resource|null */
+	protected $process;
+
+	/**
+	 * Ensure the process is stopped when the object is destroyed
+	 */
+	public function __destruct() {
+		$this->stop();
+	}
+
+	/**
+	 * @phpstan-assert-if-true resource $this->process
+	 */
+	public function isRunning() : bool {
+		if( !is_resource($this->process) ) {
+			return false;
+		}
+
+		$processStatus = proc_get_status($this->process);
+
+		if( !$processStatus ) {
+			return false;
+		}
+
+		return $processStatus['running'];
+	}
+
+	public function stop() : void {
+		if( !$this->isRunning() ) {
+			return;
+		}
+
+		proc_terminate($this->process);
+
+		$attempts = 0;
+		while( $this->isRunning() ) {
+			if( ++$attempts > 1000 ) {
+				throw new ServerException('Failed to stop server.');
+			}
+
+			usleep(10000);
+		}
+	}
+
+	/**
+	 * Create a temporary file
+	 *
+	 * @param string $prefix Prefix for the temp file name
+	 * @throws TempFileException If temp file creation fails
+	 * @return string Path to the created temp file
+	 */
+	protected function createTempFile( string $prefix ) : string {
+		$tempFile = tempnam(sys_get_temp_dir(), $prefix);
+		if( $tempFile === false ) {
+			throw new TempFileException("error creating temp file with prefix '{$prefix}'");
+		}
+
+		return $tempFile;
+	}
+
+}