Merge 'minor-next' into 'major-next'

Automatic merge performed by: https://github.com/pmmp/RestrictedActions/actions/runs/11944832380

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] . " <command token seed>" . 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<int, string> $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.

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");

src/console/ConsoleReaderChildProcessUtils.php

@@ -0,0 +1,71 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\console;
+
+use function hash;
+use function strlen;
+use function strrpos;
+use function substr;
+
+final class ConsoleReaderChildProcessUtils{
+	public const TOKEN_DELIMITER = ":";
+	public const TOKEN_HASH_ALGO = "xxh3";
+
+	private function __construct(){
+
+	}
+
+	/**
+	 * Creates an IPC message to transmit a user's input command to the parent process.
+	 *
+	 * Unfortunately we can't currently provide IPC pipes other than stdout/stderr to subprocesses on Windows, so this
+	 * adds a hash of the user input (with a counter as salt) to prevent unintended process output (like error messages)
+	 * from being treated as user input.
+	 */
+	public static function createMessage(string $line, int &$counter) : string{
+		$token = hash(self::TOKEN_HASH_ALGO, $line, options: ['seed' => $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;
+	}
+}

tests/phpunit/console/ConsoleReaderChildProcessUtilsTest.php

@@ -0,0 +1,92 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\console;
+
+use PHPUnit\Framework\TestCase;
+use function mt_rand;
+use function str_repeat;
+
+final class ConsoleReaderChildProcessUtilsTest extends TestCase{
+
+	/**
+	 * @phpstan-return \Generator<int, array{string}, void, void>
+	 */
+	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<int, array{string, bool}, void, void>
+	 */
+	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");
+		}
+	}
+}

tests/phpunit/event/EventTest.php

@@ -0,0 +1,79 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\event;
+
+use PHPUnit\Framework\TestCase;
+use pocketmine\event\fixtures\TestChildEvent;
+use pocketmine\event\fixtures\TestGrandchildEvent;
+use pocketmine\event\fixtures\TestParentEvent;
+use pocketmine\plugin\Plugin;
+use pocketmine\plugin\PluginManager;
+use pocketmine\Server;
+
+final class EventTest extends TestCase{
+
+	private Plugin $mockPlugin;
+	private PluginManager $pluginManager;
+
+	protected function setUp() : void{
+		HandlerListManager::global()->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");
+	}
+}

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{
 

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

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{
 

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{
 
 }

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{
 

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{
 

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{
 

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{
 

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{
 
 }

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{
 
 }

tests/plugins/TesterPlugin/src/EventHandlerInheritanceTest.php

@@ -1,88 +0,0 @@
-<?php
-
-/*
- *
- *  ____            _        _   __  __ _                  __  __ ____
- * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
- * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
- * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
- * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * @author PocketMine Team
- * @link http://www.pocketmine.net/
- *
- *
- */
-
-declare(strict_types=1);
-
-namespace pmmp\TesterPlugin;
-
-use pmmp\TesterPlugin\event\ChildEvent;
-use pmmp\TesterPlugin\event\GrandchildEvent;
-use pmmp\TesterPlugin\event\ParentEvent;
-use pocketmine\event\EventPriority;
-use function implode;
-
-final class EventHandlerInheritanceTest extends Test{
-
-	private const EXPECTED_ORDER = [
-		GrandchildEvent::class,
-		ChildEvent::class,
-		ParentEvent::class
-	];
-
-	/** @var string[] */
-	private array $callOrder = [];
-
-	public function getName() : string{
-		return "Event Handler Inheritance Test";
-	}
-
-	public function getDescription() : string{
-		return "Tests that child events are correctly passed to parent event handlers";
-	}
-
-	public function run() : void{
-		$plugin = $this->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);
-		}
-	}
-}

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
 		];
 	}