From e25c03eec16f8fbe0e69f7c8c852ca46c525f390 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Tue, 5 Oct 2021 20:28:43 +0100
Subject: [PATCH] Gracefully handle errors loading plugin manifest this isn't
 perfect, but it covers the common cases. Now, the server won't spam
 crashdumps just because some plugin declared nested permissions.

---
 src/lang/KnownTranslationFactory.php |  6 ++++++
 src/lang/KnownTranslationKeys.php    |  1 +
 src/plugin/PluginDescription.php     | 15 ++++++++++-----
 src/plugin/PluginLoader.php          |  1 +
 src/plugin/PluginManager.php         |  7 +++++++
 5 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/lang/KnownTranslationFactory.php b/src/lang/KnownTranslationFactory.php
index e4aea7df6..756d623de 100644
--- a/src/lang/KnownTranslationFactory.php
+++ b/src/lang/KnownTranslationFactory.php
@@ -1746,6 +1746,12 @@ final class KnownTranslationFactory{
 		]);
 	}
 
+	public static function pocketmine_plugin_invalidManifest(Translatable|string $details) : Translatable{
+		return new Translatable(KnownTranslationKeys::POCKETMINE_PLUGIN_INVALIDMANIFEST, [
+			"details" => $details,
+		]);
+	}
+
 	public static function pocketmine_plugin_load(Translatable|string $param0) : Translatable{
 		return new Translatable(KnownTranslationKeys::POCKETMINE_PLUGIN_LOAD, [
 			0 => $param0,
diff --git a/src/lang/KnownTranslationKeys.php b/src/lang/KnownTranslationKeys.php
index 4b8d87c64..7f1eb2658 100644
--- a/src/lang/KnownTranslationKeys.php
+++ b/src/lang/KnownTranslationKeys.php
@@ -364,6 +364,7 @@ final class KnownTranslationKeys{
 	public const POCKETMINE_PLUGIN_INCOMPATIBLEPHPVERSION = "pocketmine.plugin.incompatiblePhpVersion";
 	public const POCKETMINE_PLUGIN_INCOMPATIBLEPROTOCOL = "pocketmine.plugin.incompatibleProtocol";
 	public const POCKETMINE_PLUGIN_INVALIDAPI = "pocketmine.plugin.invalidAPI";
+	public const POCKETMINE_PLUGIN_INVALIDMANIFEST = "pocketmine.plugin.invalidManifest";
 	public const POCKETMINE_PLUGIN_LOAD = "pocketmine.plugin.load";
 	public const POCKETMINE_PLUGIN_LOADERROR = "pocketmine.plugin.loadError";
 	public const POCKETMINE_PLUGIN_RESTRICTEDNAME = "pocketmine.plugin.restrictedName";
diff --git a/src/plugin/PluginDescription.php b/src/plugin/PluginDescription.php
index 9ae463093..090be61b1 100644
--- a/src/plugin/PluginDescription.php
+++ b/src/plugin/PluginDescription.php
@@ -25,6 +25,7 @@ namespace pocketmine\plugin;
 
 use pocketmine\permission\Permission;
 use pocketmine\permission\PermissionParser;
+use pocketmine\permission\PermissionParserException;
 use function array_map;
 use function array_values;
 use function is_array;
@@ -99,20 +100,20 @@ class PluginDescription{
 
 	/**
 	 * @param mixed[] $plugin
-	 * @throws PluginException
+	 * @throws PluginDescriptionParseException
 	 */
 	private function loadMap(array $plugin) : void{
 		$this->map = $plugin;
 
 		$this->name = $plugin["name"];
 		if(preg_match('/^[A-Za-z0-9 _.-]+$/', $this->name) === 0){
-			throw new PluginException("Invalid Plugin name");
+			throw new PluginDescriptionParseException("Invalid Plugin name");
 		}
 		$this->name = str_replace(" ", "_", $this->name);
 		$this->version = (string) $plugin["version"];
 		$this->main = $plugin["main"];
 		if(stripos($this->main, "pocketmine\\") === 0){
-			throw new PluginException("Invalid Plugin main, cannot start within the PocketMine namespace");
+			throw new PluginDescriptionParseException("Invalid Plugin main, cannot start within the PocketMine namespace");
 		}
 
 		$this->srcNamespacePrefix = $plugin["src-namespace-prefix"] ?? "";
@@ -153,7 +154,7 @@ class PluginDescription{
 		if(isset($plugin["load"])){
 			$order = PluginEnableOrder::fromString($plugin["load"]);
 			if($order === null){
-				throw new PluginException("Invalid Plugin \"load\"");
+				throw new PluginDescriptionParseException("Invalid Plugin \"load\"");
 			}
 			$this->order = $order;
 		}else{
@@ -175,7 +176,11 @@ class PluginDescription{
 		}
 
 		if(isset($plugin["permissions"])){
-			$this->permissions = PermissionParser::loadPermissions($plugin["permissions"]);
+			try{
+				$this->permissions = PermissionParser::loadPermissions($plugin["permissions"]);
+			}catch(PermissionParserException $e){
+				throw new PluginDescriptionParseException("Invalid Plugin \"permissions\": " . $e->getMessage(), 0, $e);
+			}
 		}
 	}
 
diff --git a/src/plugin/PluginLoader.php b/src/plugin/PluginLoader.php
index 2bf603dde..7e4787fa6 100644
--- a/src/plugin/PluginLoader.php
+++ b/src/plugin/PluginLoader.php
@@ -40,6 +40,7 @@ interface PluginLoader{
 
 	/**
 	 * Gets the PluginDescription from the file
+	 * @throws PluginDescriptionParseException
 	 */
 	public function getPluginDescription(string $file) : ?PluginDescription;
 
diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php
index 2ab687525..3a58e3bc4 100644
--- a/src/plugin/PluginManager.php
+++ b/src/plugin/PluginManager.php
@@ -246,6 +246,13 @@ class PluginManager{
 				}
 				try{
 					$description = $loader->getPluginDescription($file);
+				}catch(PluginDescriptionParseException $e){
+					$this->server->getLogger()->error($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_fileError(
+						$file,
+						$directory,
+						KnownTranslationFactory::pocketmine_plugin_invalidManifest($e->getMessage())
+					)));
+					continue;
 				}catch(\RuntimeException $e){ //TODO: more specific exception handling
 					$this->server->getLogger()->error($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_fileError($file, $directory, $e->getMessage())));
 					$this->server->getLogger()->logException($e);