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"); + } + } +} diff --git a/tests/phpunit/event/EventTest.php b/tests/phpunit/event/EventTest.php new file mode 100644 index 000000000..7410cc3bb --- /dev/null +++ b/tests/phpunit/event/EventTest.php @@ -0,0 +1,79 @@ +unregisterAll(); + + //TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field + //we really need to make it possible to register events without a Plugin or Server context + $mockServer = $this->createMock(Server::class); + $this->mockPlugin = self::createStub(Plugin::class); + $this->mockPlugin->method('isEnabled')->willReturn(true); + + $this->pluginManager = new PluginManager($mockServer, null); + } + + public static function tearDownAfterClass() : void{ + HandlerListManager::global()->unregisterAll(); + } + + public function testHandlerInheritance() : void{ + $expectedOrder = [ + TestGrandchildEvent::class, + TestChildEvent::class, + TestParentEvent::class + ]; + $actualOrder = []; + + foreach($expectedOrder as $class){ + $this->pluginManager->registerEvent( + $class, + function(TestParentEvent $event) use (&$actualOrder, $class) : void{ + $actualOrder[] = $class; + }, + EventPriority::NORMAL, + $this->mockPlugin + ); + } + + $event = new TestGrandchildEvent(); + $event->call(); + + self::assertSame($expectedOrder, $actualOrder, "Expected event handlers to be called from most specific to least specific"); + } +} diff --git a/tests/phpunit/event/HandlerListManagerTest.php b/tests/phpunit/event/HandlerListManagerTest.php index edff36639..c61043dab 100644 --- a/tests/phpunit/event/HandlerListManagerTest.php +++ b/tests/phpunit/event/HandlerListManagerTest.php @@ -24,6 +24,12 @@ declare(strict_types=1); namespace pocketmine\event; use PHPUnit\Framework\TestCase; +use pocketmine\event\fixtures\TestAbstractAllowHandleEvent; +use pocketmine\event\fixtures\TestAbstractEvent; +use pocketmine\event\fixtures\TestConcreteEvent; +use pocketmine\event\fixtures\TestConcreteExtendsAbstractEvent; +use pocketmine\event\fixtures\TestConcreteExtendsAllowHandleEvent; +use pocketmine\event\fixtures\TestConcreteExtendsConcreteEvent; class HandlerListManagerTest extends TestCase{ diff --git a/tests/phpunit/event/TestAbstractAllowHandleEvent.php b/tests/phpunit/event/fixtures/TestAbstractAllowHandleEvent.php similarity index 92% rename from tests/phpunit/event/TestAbstractAllowHandleEvent.php rename to tests/phpunit/event/fixtures/TestAbstractAllowHandleEvent.php index 1bac06bbb..383169809 100644 --- a/tests/phpunit/event/TestAbstractAllowHandleEvent.php +++ b/tests/phpunit/event/fixtures/TestAbstractAllowHandleEvent.php @@ -21,7 +21,9 @@ declare(strict_types=1); -namespace pocketmine\event; +namespace pocketmine\event\fixtures; + +use pocketmine\event\Event; /** * @allowHandle diff --git a/tests/phpunit/event/TestAbstractEvent.php b/tests/phpunit/event/fixtures/TestAbstractEvent.php similarity index 92% rename from tests/phpunit/event/TestAbstractEvent.php rename to tests/phpunit/event/fixtures/TestAbstractEvent.php index 92a95363e..b48d8f526 100644 --- a/tests/phpunit/event/TestAbstractEvent.php +++ b/tests/phpunit/event/fixtures/TestAbstractEvent.php @@ -21,7 +21,9 @@ declare(strict_types=1); -namespace pocketmine\event; +namespace pocketmine\event\fixtures; + +use pocketmine\event\Event; abstract class TestAbstractEvent extends Event{ diff --git a/tests/plugins/TesterPlugin/src/event/ChildEvent.php b/tests/phpunit/event/fixtures/TestChildEvent.php similarity index 89% rename from tests/plugins/TesterPlugin/src/event/ChildEvent.php rename to tests/phpunit/event/fixtures/TestChildEvent.php index b71d2627f..569a2c069 100644 --- a/tests/plugins/TesterPlugin/src/event/ChildEvent.php +++ b/tests/phpunit/event/fixtures/TestChildEvent.php @@ -21,8 +21,8 @@ declare(strict_types=1); -namespace pmmp\TesterPlugin\event; +namespace pocketmine\event\fixtures; -class ChildEvent extends ParentEvent{ +class TestChildEvent extends TestParentEvent{ } diff --git a/tests/phpunit/event/TestConcreteEvent.php b/tests/phpunit/event/fixtures/TestConcreteEvent.php similarity index 92% rename from tests/phpunit/event/TestConcreteEvent.php rename to tests/phpunit/event/fixtures/TestConcreteEvent.php index 8b159df91..cf744eb2c 100644 --- a/tests/phpunit/event/TestConcreteEvent.php +++ b/tests/phpunit/event/fixtures/TestConcreteEvent.php @@ -21,7 +21,9 @@ declare(strict_types=1); -namespace pocketmine\event; +namespace pocketmine\event\fixtures; + +use pocketmine\event\Event; class TestConcreteEvent extends Event{ diff --git a/tests/phpunit/event/TestConcreteExtendsAbstractEvent.php b/tests/phpunit/event/fixtures/TestConcreteExtendsAbstractEvent.php similarity index 95% rename from tests/phpunit/event/TestConcreteExtendsAbstractEvent.php rename to tests/phpunit/event/fixtures/TestConcreteExtendsAbstractEvent.php index 3f0fa572f..54ec3dbb1 100644 --- a/tests/phpunit/event/TestConcreteExtendsAbstractEvent.php +++ b/tests/phpunit/event/fixtures/TestConcreteExtendsAbstractEvent.php @@ -21,7 +21,7 @@ declare(strict_types=1); -namespace pocketmine\event; +namespace pocketmine\event\fixtures; class TestConcreteExtendsAbstractEvent extends TestAbstractEvent{ diff --git a/tests/phpunit/event/TestConcreteExtendsAllowHandleEvent.php b/tests/phpunit/event/fixtures/TestConcreteExtendsAllowHandleEvent.php similarity index 95% rename from tests/phpunit/event/TestConcreteExtendsAllowHandleEvent.php rename to tests/phpunit/event/fixtures/TestConcreteExtendsAllowHandleEvent.php index e41465121..362ce693b 100644 --- a/tests/phpunit/event/TestConcreteExtendsAllowHandleEvent.php +++ b/tests/phpunit/event/fixtures/TestConcreteExtendsAllowHandleEvent.php @@ -21,7 +21,7 @@ declare(strict_types=1); -namespace pocketmine\event; +namespace pocketmine\event\fixtures; class TestConcreteExtendsAllowHandleEvent extends TestAbstractAllowHandleEvent{ diff --git a/tests/phpunit/event/TestConcreteExtendsConcreteEvent.php b/tests/phpunit/event/fixtures/TestConcreteExtendsConcreteEvent.php similarity index 95% rename from tests/phpunit/event/TestConcreteExtendsConcreteEvent.php rename to tests/phpunit/event/fixtures/TestConcreteExtendsConcreteEvent.php index cc9558935..3fd3a6cf0 100644 --- a/tests/phpunit/event/TestConcreteExtendsConcreteEvent.php +++ b/tests/phpunit/event/fixtures/TestConcreteExtendsConcreteEvent.php @@ -21,7 +21,7 @@ declare(strict_types=1); -namespace pocketmine\event; +namespace pocketmine\event\fixtures; class TestConcreteExtendsConcreteEvent extends TestConcreteEvent{ diff --git a/tests/plugins/TesterPlugin/src/event/GrandchildEvent.php b/tests/phpunit/event/fixtures/TestGrandchildEvent.php similarity index 89% rename from tests/plugins/TesterPlugin/src/event/GrandchildEvent.php rename to tests/phpunit/event/fixtures/TestGrandchildEvent.php index 40c37c567..bfe50f9f3 100644 --- a/tests/plugins/TesterPlugin/src/event/GrandchildEvent.php +++ b/tests/phpunit/event/fixtures/TestGrandchildEvent.php @@ -21,8 +21,8 @@ declare(strict_types=1); -namespace pmmp\TesterPlugin\event; +namespace pocketmine\event\fixtures; -class GrandchildEvent extends ChildEvent{ +class TestGrandchildEvent extends TestChildEvent{ } diff --git a/tests/plugins/TesterPlugin/src/event/ParentEvent.php b/tests/phpunit/event/fixtures/TestParentEvent.php similarity index 87% rename from tests/plugins/TesterPlugin/src/event/ParentEvent.php rename to tests/phpunit/event/fixtures/TestParentEvent.php index 68f7df630..c20442272 100644 --- a/tests/plugins/TesterPlugin/src/event/ParentEvent.php +++ b/tests/phpunit/event/fixtures/TestParentEvent.php @@ -21,8 +21,10 @@ declare(strict_types=1); -namespace pmmp\TesterPlugin\event; +namespace pocketmine\event\fixtures; -class ParentEvent extends \pocketmine\event\Event{ +use pocketmine\event\Event; + +class TestParentEvent extends Event{ } diff --git a/tests/plugins/TesterPlugin/src/EventHandlerInheritanceTest.php b/tests/plugins/TesterPlugin/src/EventHandlerInheritanceTest.php deleted file mode 100644 index efe20f8d8..000000000 --- a/tests/plugins/TesterPlugin/src/EventHandlerInheritanceTest.php +++ /dev/null @@ -1,88 +0,0 @@ -getPlugin(); - $plugin->getServer()->getPluginManager()->registerEvent( - ParentEvent::class, - function(ParentEvent $event) : void{ - $this->callOrder[] = ParentEvent::class; - }, - EventPriority::NORMAL, - $plugin - ); - $plugin->getServer()->getPluginManager()->registerEvent( - ChildEvent::class, - function(ChildEvent $event) : void{ - $this->callOrder[] = ChildEvent::class; - }, - EventPriority::NORMAL, - $plugin - ); - $plugin->getServer()->getPluginManager()->registerEvent( - GrandchildEvent::class, - function(GrandchildEvent $event) : void{ - $this->callOrder[] = GrandchildEvent::class; - }, - EventPriority::NORMAL, - $plugin - ); - - $event = new GrandchildEvent(); - $event->call(); - - if($this->callOrder === self::EXPECTED_ORDER){ - $this->setResult(Test::RESULT_OK); - }else{ - $plugin->getLogger()->error("Expected order: " . implode(", ", self::EXPECTED_ORDER) . ", got: " . implode(", ", $this->callOrder)); - $this->setResult(Test::RESULT_FAILED); - } - } -} diff --git a/tests/plugins/TesterPlugin/src/Main.php b/tests/plugins/TesterPlugin/src/Main.php index 26d3441f4..08b59dbac 100644 --- a/tests/plugins/TesterPlugin/src/Main.php +++ b/tests/plugins/TesterPlugin/src/Main.php @@ -57,7 +57,7 @@ class Main extends PluginBase implements Listener{ }), 10); $this->waitingTests = [ - new EventHandlerInheritanceTest($this), + //Add test objects here ]; }