Stop the server if any plugin failed to load or enable (#4951)

closes #3080 

If plugins fail to load for some reason, it's highly likely that some critical functionality of the server is compromised. For example:
- if an NPC plugin fails to load, all custom entities added by that plugin will be deleted from worlds
- if a world protection plugin fails, players will be able to grief your otherwise immutable lobby map
- if a worldgen plugin fails, worlds using custom generators won't load
- if a permission plugin fails, players might have access to commands and features they aren't supposed to have
- the list goes on...

This change makes the server commit graceful suicide if any plugin fails to load for error-related reasons, including (but not limited to):
- Incompatible API version
- Missing dependencies
- Invalid plugin.yml
- Invalid main class

Plugins prevented from loading by `plugin_list.yml` are not considered errors and **are not** included in this change. If a plugin is disallowed from loading due to the `plugin_list`, the server will continue to run as if the plugin was not present.
This commit is contained in:
Dylan T 2022-05-11 20:43:38 +01:00 committed by GitHub
parent 3b7e274c34
commit 52e74296de
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 56 additions and 19 deletions

View File

@ -1003,14 +1003,29 @@ class Server{
register_shutdown_function([$this, "crashDump"]); register_shutdown_function([$this, "crashDump"]);
$this->pluginManager->loadPlugins($this->pluginPath); $loadErrorCount = 0;
$this->enablePlugins(PluginEnableOrder::STARTUP()); $this->pluginManager->loadPlugins($this->pluginPath, $loadErrorCount);
if($loadErrorCount > 0){
$this->logger->emergency($this->language->translate(KnownTranslationFactory::pocketmine_plugin_someLoadErrors()));
$this->forceShutdown();
return;
}
if(!$this->enablePlugins(PluginEnableOrder::STARTUP())){
$this->logger->emergency($this->language->translate(KnownTranslationFactory::pocketmine_plugin_someEnableErrors()));
$this->forceShutdown();
return;
}
if(!$this->startupPrepareWorlds()){ if(!$this->startupPrepareWorlds()){
$this->forceShutdown(); $this->forceShutdown();
return; return;
} }
$this->enablePlugins(PluginEnableOrder::POSTWORLD());
if(!$this->enablePlugins(PluginEnableOrder::POSTWORLD())){
$this->logger->emergency($this->language->translate(KnownTranslationFactory::pocketmine_plugin_someEnableErrors()));
$this->forceShutdown();
return;
}
if(!$this->startupPrepareNetworkInterfaces()){ if(!$this->startupPrepareNetworkInterfaces()){
$this->forceShutdown(); $this->forceShutdown();
@ -1395,16 +1410,21 @@ class Server{
} }
} }
public function enablePlugins(PluginEnableOrder $type) : void{ public function enablePlugins(PluginEnableOrder $type) : bool{
$allSuccess = true;
foreach($this->pluginManager->getPlugins() as $plugin){ foreach($this->pluginManager->getPlugins() as $plugin){
if(!$plugin->isEnabled() && $plugin->getDescription()->getOrder()->equals($type)){ if(!$plugin->isEnabled() && $plugin->getDescription()->getOrder()->equals($type)){
$this->pluginManager->enablePlugin($plugin); if(!$this->pluginManager->enablePlugin($plugin)){
$allSuccess = false;
}
} }
} }
if($type->equals(PluginEnableOrder::POSTWORLD())){ if($type->equals(PluginEnableOrder::POSTWORLD())){
$this->commandMap->registerServerAliases(); $this->commandMap->registerServerAliases();
} }
return $allSuccess;
} }
/** /**

View File

@ -138,7 +138,7 @@ class PluginManager{
$dataFolder = $this->getDataDirectory($path, $description->getName()); $dataFolder = $this->getDataDirectory($path, $description->getName());
if(file_exists($dataFolder) && !is_dir($dataFolder)){ if(file_exists($dataFolder) && !is_dir($dataFolder)){
$this->server->getLogger()->error($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError( $this->server->getLogger()->critical($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError(
$description->getName(), $description->getName(),
KnownTranslationFactory::pocketmine_plugin_badDataFolder($dataFolder) KnownTranslationFactory::pocketmine_plugin_badDataFolder($dataFolder)
))); )));
@ -153,14 +153,14 @@ class PluginManager{
$mainClass = $description->getMain(); $mainClass = $description->getMain();
if(!class_exists($mainClass, true)){ if(!class_exists($mainClass, true)){
$this->server->getLogger()->error($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError( $this->server->getLogger()->critical($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError(
$description->getName(), $description->getName(),
KnownTranslationFactory::pocketmine_plugin_mainClassNotFound() KnownTranslationFactory::pocketmine_plugin_mainClassNotFound()
))); )));
return null; return null;
} }
if(!is_a($mainClass, Plugin::class, true)){ if(!is_a($mainClass, Plugin::class, true)){
$this->server->getLogger()->error($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError( $this->server->getLogger()->critical($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError(
$description->getName(), $description->getName(),
KnownTranslationFactory::pocketmine_plugin_mainClassWrongType(Plugin::class) KnownTranslationFactory::pocketmine_plugin_mainClassWrongType(Plugin::class)
))); )));
@ -168,7 +168,7 @@ class PluginManager{
} }
$reflect = new \ReflectionClass($mainClass); //this shouldn't throw; we already checked that it exists $reflect = new \ReflectionClass($mainClass); //this shouldn't throw; we already checked that it exists
if(!$reflect->isInstantiable()){ if(!$reflect->isInstantiable()){
$this->server->getLogger()->error($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError( $this->server->getLogger()->critical($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError(
$description->getName(), $description->getName(),
KnownTranslationFactory::pocketmine_plugin_mainClassAbstract() KnownTranslationFactory::pocketmine_plugin_mainClassAbstract()
))); )));
@ -179,7 +179,7 @@ class PluginManager{
foreach($description->getPermissions() as $permsGroup){ foreach($description->getPermissions() as $permsGroup){
foreach($permsGroup as $perm){ foreach($permsGroup as $perm){
if($permManager->getPermission($perm->getName()) !== null){ if($permManager->getPermission($perm->getName()) !== null){
$this->server->getLogger()->error($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError( $this->server->getLogger()->critical($language->translate(KnownTranslationFactory::pocketmine_plugin_loadError(
$description->getName(), $description->getName(),
KnownTranslationFactory::pocketmine_plugin_duplicatePermissionError($perm->getName()) KnownTranslationFactory::pocketmine_plugin_duplicatePermissionError($perm->getName())
))); )));
@ -229,7 +229,7 @@ class PluginManager{
* @param string[]|null $newLoaders * @param string[]|null $newLoaders
* @phpstan-param list<class-string<PluginLoader>> $newLoaders * @phpstan-param list<class-string<PluginLoader>> $newLoaders
*/ */
private function triagePlugins(string $path, PluginLoadTriage $triage, ?array $newLoaders = null) : void{ private function triagePlugins(string $path, PluginLoadTriage $triage, int &$loadErrorCount, ?array $newLoaders = null) : void{
if(is_array($newLoaders)){ if(is_array($newLoaders)){
$loaders = []; $loaders = [];
foreach($newLoaders as $key){ foreach($newLoaders as $key){
@ -261,14 +261,16 @@ class PluginManager{
try{ try{
$description = $loader->getPluginDescription($file); $description = $loader->getPluginDescription($file);
}catch(PluginDescriptionParseException $e){ }catch(PluginDescriptionParseException $e){
$this->server->getLogger()->error($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_loadError( $this->server->getLogger()->critical($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_loadError(
$file, $file,
KnownTranslationFactory::pocketmine_plugin_invalidManifest($e->getMessage()) KnownTranslationFactory::pocketmine_plugin_invalidManifest($e->getMessage())
))); )));
$loadErrorCount++;
continue; continue;
}catch(\RuntimeException $e){ //TODO: more specific exception handling }catch(\RuntimeException $e){ //TODO: more specific exception handling
$this->server->getLogger()->error($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_loadError($file, $e->getMessage()))); $this->server->getLogger()->critical($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_loadError($file, $e->getMessage())));
$this->server->getLogger()->logException($e); $this->server->getLogger()->logException($e);
$loadErrorCount++;
continue; continue;
} }
if($description === null){ if($description === null){
@ -278,12 +280,14 @@ class PluginManager{
$name = $description->getName(); $name = $description->getName();
if(($loadabilityError = $loadabilityChecker->check($description)) !== null){ if(($loadabilityError = $loadabilityChecker->check($description)) !== null){
$this->server->getLogger()->error($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_loadError($name, $loadabilityError))); $this->server->getLogger()->critical($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_loadError($name, $loadabilityError)));
$loadErrorCount++;
continue; continue;
} }
if(isset($triage->plugins[$name]) || $this->getPlugin($name) instanceof Plugin){ if(isset($triage->plugins[$name]) || $this->getPlugin($name) instanceof Plugin){
$this->server->getLogger()->error($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_duplicateError($name))); $this->server->getLogger()->critical($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_duplicateError($name)));
$loadErrorCount++;
continue; continue;
} }
@ -296,6 +300,9 @@ class PluginManager{
$name, $name,
$this->graylist->isWhitelist() ? KnownTranslationFactory::pocketmine_plugin_disallowedByWhitelist() : KnownTranslationFactory::pocketmine_plugin_disallowedByBlacklist() $this->graylist->isWhitelist() ? KnownTranslationFactory::pocketmine_plugin_disallowedByWhitelist() : KnownTranslationFactory::pocketmine_plugin_disallowedByBlacklist()
))); )));
//this does NOT increment loadErrorCount, because using the graylist to prevent a plugin from
//loading is not considered accidental; this is the same as if the plugin were manually removed
//this means that the server will continue to boot even if some plugins were blocked by graylist
continue; continue;
} }
@ -339,14 +346,14 @@ class PluginManager{
/** /**
* @return Plugin[] * @return Plugin[]
*/ */
public function loadPlugins(string $path) : array{ public function loadPlugins(string $path, int &$loadErrorCount = 0) : array{
if($this->loadPluginsGuard){ if($this->loadPluginsGuard){
throw new \LogicException(__METHOD__ . "() cannot be called from within itself"); throw new \LogicException(__METHOD__ . "() cannot be called from within itself");
} }
$this->loadPluginsGuard = true; $this->loadPluginsGuard = true;
$triage = new PluginLoadTriage(); $triage = new PluginLoadTriage();
$this->triagePlugins($path, $triage); $this->triagePlugins($path, $triage, $loadErrorCount);
$loadedPlugins = []; $loadedPlugins = [];
@ -372,10 +379,12 @@ class PluginManager{
if(count($diffLoaders) !== 0){ if(count($diffLoaders) !== 0){
$this->server->getLogger()->debug("Plugin $name registered a new plugin loader during load, scanning for new plugins"); $this->server->getLogger()->debug("Plugin $name registered a new plugin loader during load, scanning for new plugins");
$plugins = $triage->plugins; $plugins = $triage->plugins;
$this->triagePlugins($path, $triage, $diffLoaders); $this->triagePlugins($path, $triage, $loadErrorCount, $diffLoaders);
$diffPlugins = array_diff_key($triage->plugins, $plugins); $diffPlugins = array_diff_key($triage->plugins, $plugins);
$this->server->getLogger()->debug("Re-triage found plugins: " . implode(", ", array_keys($diffPlugins))); $this->server->getLogger()->debug("Re-triage found plugins: " . implode(", ", array_keys($diffPlugins)));
} }
}else{
$loadErrorCount++;
} }
} }
} }
@ -418,12 +427,14 @@ class PluginManager{
KnownTranslationFactory::pocketmine_plugin_unknownDependency(implode(", ", $unknownDependencies)) KnownTranslationFactory::pocketmine_plugin_unknownDependency(implode(", ", $unknownDependencies))
))); )));
unset($triage->plugins[$name]); unset($triage->plugins[$name]);
$loadErrorCount++;
} }
} }
} }
foreach(Utils::stringifyKeys($triage->plugins) as $name => $file){ foreach(Utils::stringifyKeys($triage->plugins) as $name => $file){
$this->server->getLogger()->critical($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_loadError($name, KnownTranslationFactory::pocketmine_plugin_circularDependency()))); $this->server->getLogger()->critical($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_loadError($name, KnownTranslationFactory::pocketmine_plugin_circularDependency())));
$loadErrorCount++;
} }
break; break;
} }
@ -437,7 +448,7 @@ class PluginManager{
return isset($this->plugins[$plugin->getDescription()->getName()]) && $plugin->isEnabled(); return isset($this->plugins[$plugin->getDescription()->getName()]) && $plugin->isEnabled();
} }
public function enablePlugin(Plugin $plugin) : void{ public function enablePlugin(Plugin $plugin) : bool{
if(!$plugin->isEnabled()){ if(!$plugin->isEnabled()){
$this->server->getLogger()->info($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_enable($plugin->getDescription()->getFullName()))); $this->server->getLogger()->info($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_enable($plugin->getDescription()->getFullName())));
@ -447,6 +458,8 @@ class PluginManager{
$this->enabledPlugins[$plugin->getDescription()->getName()] = $plugin; $this->enabledPlugins[$plugin->getDescription()->getName()] = $plugin;
(new PluginEnableEvent($plugin))->call(); (new PluginEnableEvent($plugin))->call();
return true;
}else{ }else{
$this->server->getLogger()->critical($this->server->getLanguage()->translate( $this->server->getLogger()->critical($this->server->getLanguage()->translate(
KnownTranslationFactory::pocketmine_plugin_enableError( KnownTranslationFactory::pocketmine_plugin_enableError(
@ -454,8 +467,12 @@ class PluginManager{
KnownTranslationFactory::pocketmine_plugin_suicide() KnownTranslationFactory::pocketmine_plugin_suicide()
) )
)); ));
return false;
} }
} }
return true; //TODO: maybe this should be an error?
} }
public function disablePlugins() : void{ public function disablePlugins() : void{