diff --git a/src/console/ConsoleReaderChildProcess.php b/src/console/ConsoleReaderChildProcess.php index 0d5a30fdc..c0c4f0048 100644 --- a/src/console/ConsoleReaderChildProcess.php +++ b/src/console/ConsoleReaderChildProcess.php @@ -29,23 +29,21 @@ use pocketmine\utils\Process; use function cli_set_process_title; use function count; use function dirname; -use function feof; use function fwrite; -use function stream_socket_client; +use function is_numeric; +use const PHP_EOL; +use const STDOUT; + +if(count($argv) !== 2 || !is_numeric($argv[1])){ + echo "Usage: " . $argv[0] . " " . PHP_EOL; + exit(1); +} + +$commandTokenSeed = (int) $argv[1]; require dirname(__DIR__, 2) . '/vendor/autoload.php'; -if(count($argv) !== 2){ - die("Please provide a server to connect to"); -} - @cli_set_process_title('PocketMine-MP Console Reader'); -$errCode = null; -$errMessage = null; -$socket = stream_socket_client($argv[1], $errCode, $errMessage, 15.0); -if($socket === false){ - throw new \RuntimeException("Failed to connect to server process ($errCode): $errMessage"); -} /** @phpstan-var ThreadSafeArray $channel */ $channel = new ThreadSafeArray(); @@ -75,15 +73,15 @@ $thread = new class($channel) extends NativeThread{ }; $thread->start(NativeThread::INHERIT_NONE); -while(!feof($socket)){ +while(true){ $line = $channel->synchronized(function() use ($channel) : ?string{ if(count($channel) === 0){ $channel->wait(1_000_000); } - $line = $channel->shift(); - return $line; + return $channel->shift(); }); - if(@fwrite($socket, ($line ?? "") . "\n") === false){ + $message = $line !== null ? ConsoleReaderChildProcessUtils::createMessage($line, $commandTokenSeed) : ""; + if(@fwrite(STDOUT, $message . "\n") === false){ //Always send even if there's no line, to check if the parent is alive //If the parent process was terminated forcibly, it won't close the connection properly, so feof() will return //false even though the connection is actually broken. However, fwrite() will fail. diff --git a/src/console/ConsoleReaderChildProcessDaemon.php b/src/console/ConsoleReaderChildProcessDaemon.php index 138559f06..f7300b7a5 100644 --- a/src/console/ConsoleReaderChildProcessDaemon.php +++ b/src/console/ConsoleReaderChildProcessDaemon.php @@ -29,19 +29,16 @@ use Symfony\Component\Filesystem\Path; use function base64_encode; use function fgets; use function fopen; +use function mt_rand; use function preg_replace; use function proc_close; use function proc_open; use function proc_terminate; +use function rtrim; use function sprintf; use function stream_select; -use function stream_socket_accept; -use function stream_socket_get_name; -use function stream_socket_server; -use function stream_socket_shutdown; use function trim; use const PHP_BINARY; -use const STREAM_SHUT_RDWR; /** * This pile of shit exists because PHP on Windows is broken, and can't handle stream_select() on stdin or pipes @@ -58,44 +55,44 @@ use const STREAM_SHUT_RDWR; * communication. */ final class ConsoleReaderChildProcessDaemon{ + public const TOKEN_DELIMITER = ":"; + public const TOKEN_HASH_ALGO = "xxh3"; + private \PrefixedLogger $logger; /** @var resource */ private $subprocess; /** @var resource */ private $socket; + private int $commandTokenSeed; public function __construct( \Logger $logger ){ $this->logger = new \PrefixedLogger($logger, "Console Reader Daemon"); + $this->commandTokenSeed = mt_rand(); $this->prepareSubprocess(); } private function prepareSubprocess() : void{ - $server = stream_socket_server("tcp://127.0.0.1:0"); - if($server === false){ - throw new \RuntimeException("Failed to open console reader socket server"); - } - $address = Utils::assumeNotFalse(stream_socket_get_name($server, false), "stream_socket_get_name() shouldn't return false here"); - //Windows sucks, and likes to corrupt UTF-8 file paths when they travel to the subprocess, so we base64 encode //the path to avoid the problem. This is an abysmally shitty hack, but here we are :( $sub = Utils::assumeNotFalse(proc_open( - [PHP_BINARY, '-dopcache.enable_cli=0', '-r', sprintf('require base64_decode("%s", true);', base64_encode(Path::join(__DIR__, 'ConsoleReaderChildProcess.php'))), $address], [ + PHP_BINARY, + '-dopcache.enable_cli=0', + '-r', + sprintf('require base64_decode("%s", true);', base64_encode(Path::join(__DIR__, 'ConsoleReaderChildProcess.php'))), + (string) $this->commandTokenSeed + ], + [ + 1 => ['socket'], 2 => fopen("php://stderr", "w"), ], $pipes ), "Something has gone horribly wrong"); - $client = stream_socket_accept($server, 15); - if($client === false){ - throw new AssumptionFailedError("stream_socket_accept() returned false"); - } - stream_socket_shutdown($server, STREAM_SHUT_RDWR); - $this->subprocess = $sub; - $this->socket = $client; + $this->socket = $pipes[1]; } private function shutdownSubprocess() : void{ @@ -104,7 +101,6 @@ final class ConsoleReaderChildProcessDaemon{ //the first place). proc_terminate($this->subprocess); proc_close($this->subprocess); - stream_socket_shutdown($this->socket, STREAM_SHUT_RDWR); } public function readLine() : ?string{ @@ -112,13 +108,27 @@ final class ConsoleReaderChildProcessDaemon{ $w = null; $e = null; if(stream_select($r, $w, $e, 0, 0) === 1){ - $command = fgets($this->socket); - if($command === false){ + $line = fgets($this->socket); + if($line === false){ $this->logger->debug("Lost connection to subprocess, restarting (maybe the child process was killed from outside?)"); $this->shutdownSubprocess(); $this->prepareSubprocess(); return null; } + $line = rtrim($line, "\n"); + + if($line === ""){ + //keepalive + return null; + } + + $command = ConsoleReaderChildProcessUtils::parseMessage($line, $this->commandTokenSeed); + if($command === null){ + //this is not a command - it may be some kind of error output from the subprocess + //write it directly to the console + $this->logger->warning("Unexpected output from child process: $line"); + return null; + } $command = preg_replace("#\\x1b\\x5b([^\\x1b]*\\x7e|[\\x40-\\x50])#", "", trim($command)) ?? throw new AssumptionFailedError("This regex is assumed to be valid"); $command = preg_replace('/[[:cntrl:]]/', '', $command) ?? throw new AssumptionFailedError("This regex is assumed to be valid"); diff --git a/src/console/ConsoleReaderChildProcessUtils.php b/src/console/ConsoleReaderChildProcessUtils.php new file mode 100644 index 000000000..661e9b0f7 --- /dev/null +++ b/src/console/ConsoleReaderChildProcessUtils.php @@ -0,0 +1,71 @@ + $counter]); + $counter++; + return $line . self::TOKEN_DELIMITER . $token; + } + + /** + * Extracts a command from an IPC message from the console reader subprocess. + * Returns the user's input command, or null if this isn't a user input. + */ + public static function parseMessage(string $message, int &$counter) : ?string{ + $delimiterPos = strrpos($message, self::TOKEN_DELIMITER); + if($delimiterPos !== false){ + $left = substr($message, 0, $delimiterPos); + $right = substr($message, $delimiterPos + strlen(self::TOKEN_DELIMITER)); + $expectedToken = hash(self::TOKEN_HASH_ALGO, $left, options: ['seed' => $counter]); + + if($expectedToken === $right){ + $counter++; + return $left; + } + } + + return null; + } +} diff --git a/tests/phpunit/console/ConsoleReaderChildProcessUtilsTest.php b/tests/phpunit/console/ConsoleReaderChildProcessUtilsTest.php new file mode 100644 index 000000000..31ae2e27a --- /dev/null +++ b/tests/phpunit/console/ConsoleReaderChildProcessUtilsTest.php @@ -0,0 +1,92 @@ + + */ + public static function commandStringProvider() : \Generator{ + yield ["stop"]; + yield ["pocketmine:status"]; + yield [str_repeat("s", 1000)]; + yield ["time set day"]; + yield ["give \"Steve\" golden_apple"]; + } + + /** + * @dataProvider commandStringProvider + */ + public function testCreateParseSymmetry(string $input) : void{ + $counterCreate = $counterParse = mt_rand(); + $message = ConsoleReaderChildProcessUtils::createMessage($input, $counterCreate); + $parsedInput = ConsoleReaderChildProcessUtils::parseMessage($message, $counterParse); + + self::assertSame($input, $parsedInput); + } + + public function testCreateMessage() : void{ + $counter = 0; + + ConsoleReaderChildProcessUtils::createMessage("", $counter); + self::assertSame(1, $counter, "createMessage should always bump the counter"); + } + + /** + * @phpstan-return \Generator + */ + public static function parseMessageProvider() : \Generator{ + $counter = 0; + yield [ConsoleReaderChildProcessUtils::createMessage("", $counter), true]; + + yield ["", false]; //keepalive message, doesn't bump counter + + $counter = 1; + yield [ConsoleReaderChildProcessUtils::createMessage("", $counter), false]; //mismatched counter + + $counter = 0; + yield ["a" . ConsoleReaderChildProcessUtils::TOKEN_DELIMITER . "b", false]; //message with delimiter but not a valid IPC message + } + + /** + * @dataProvider parseMessageProvider + */ + public static function testParseMessage(string $message, bool $valid) : void{ + $counter = $oldCounter = 0; + + $input = ConsoleReaderChildProcessUtils::parseMessage($message, $counter); + if(!$valid){ + self::assertNull($input, "Result should be null on invalid message"); + self::assertSame($oldCounter, $counter, "Counter shouldn't be bumped on invalid message"); + }else{ + self::assertNotNull($input, "This was a valid message, expected a result"); + self::assertSame($oldCounter + 1, $counter, "Counter should be bumped on valid message parse"); + } + } +}