Better handling of plugin incompatibility

If a PHP is raised while executing plugin hook, Shaarli will display an error instead of rendering the error page (or just ending in fatal error for default hooks).
Also added phpErrorHandler which is handled differently that regular errorHandler by Slim.:

diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php
index 4a1a6ea7896830f56de47a015f6c1cb151f207b0..58067c9945319d7d7bb664d37c0589fe71b8d377 100644 (file)
--- a/application/container/ContainerBuilder.php
+++ b/application/container/ContainerBuilder.php
@@ -152,6 +152,9 @@ class ContainerBuilder
         $container['errorHandler'] = function (ShaarliContainer $container): ErrorController {
             return new ErrorController($container);
         };
+        $container['phpErrorHandler'] = function (ShaarliContainer $container): ErrorController {
+            return new ErrorController($container);
+        };
 
         return $container;
     }

diff --git a/application/container/ShaarliContainer.php b/application/container/ShaarliContainer.php
index c4fe753e5eec3b015d360f67b7bf77c18ac6fcd0..9a9a974a6785bce8077e818607dc75e5dbee7e63 100644 (file)
--- a/application/container/ShaarliContainer.php
+++ b/application/container/ShaarliContainer.php
@@ -4,7 +4,6 @@ declare(strict_types=1);
 
 namespace Shaarli\Container;
 
-use http\Cookie;
 use Shaarli\Bookmark\BookmarkServiceInterface;
 use Shaarli\Config\ConfigManager;
 use Shaarli\Feed\FeedBuilder;
@@ -30,7 +29,7 @@ use Slim\Container;
  * @property CookieManager            $cookieManager
  * @property ConfigManager            $conf
  * @property mixed[]                  $environment          $_SERVER automatically injected by Slim
- * @property callable                 $errorHandler         Overrides default Slim error display
+ * @property callable                 $errorHandler         Overrides default Slim exception display
  * @property FeedBuilder              $feedBuilder
  * @property FormatterFactory         $formatterFactory
  * @property History                  $history
@@ -39,6 +38,7 @@ use Slim\Container;
  * @property NetscapeBookmarkUtils    $netscapeBookmarkUtils
  * @property PageBuilder              $pageBuilder
  * @property PageCacheManager         $pageCacheManager
+ * @property callable                 $phpErrorHandler      Overrides default Slim PHP error display
  * @property PluginManager            $pluginManager
  * @property SessionManager           $sessionManager
  * @property Thumbnailer              $thumbnailer

diff --git a/application/front/controller/visitor/ShaarliVisitorController.php b/application/front/controller/visitor/ShaarliVisitorController.php
index 47057d97664920a08e25af6590491a3f2cbaa7a3..f17c8ed37936550304cfdfbe59735e98e677c4e4 100644 (file)
--- a/application/front/controller/visitor/ShaarliVisitorController.php
+++ b/application/front/controller/visitor/ShaarliVisitorController.php
@@ -58,10 +58,11 @@ abstract class ShaarliVisitorController
     {
         $this->assignView('linkcount', $this->container->bookmarkService->count(BookmarkFilter::$ALL));
         $this->assignView('privateLinkcount', $this->container->bookmarkService->count(BookmarkFilter::$PRIVATE));
-        $this->assignView('plugin_errors', $this->container->pluginManager->getErrors());
 
         $this->executeDefaultHooks($template);
 
+        $this->assignView('plugin_errors', $this->container->pluginManager->getErrors());
+
         return $this->container->pageBuilder->render($template, $this->container->basePath);
     }
 

diff --git a/application/plugin/PluginManager.php b/application/plugin/PluginManager.php
index b3e8b2f8e28c0ebdd9def9e8d7f8df00c7f4e903..2d93cb3a2173c58bada5803b8ca4fd522819991f 100644 (file)
--- a/application/plugin/PluginManager.php
+++ b/application/plugin/PluginManager.php
@@ -116,7 +116,12 @@ class PluginManager
             $hookFunction = $this->buildHookName($hook, $plugin);
 
             if (function_exists($hookFunction)) {
-                $data = call_user_func($hookFunction, $data, $this->conf);
+                try {
+                    $data = call_user_func($hookFunction, $data, $this->conf);
+                } catch (\Throwable $e) {
+                    $error = $plugin . t(' [plugin incompatibility]: ') . $e->getMessage();
+                    $this->errors = array_unique(array_merge($this->errors, [$error]));
+                }
             }
         }
     }

diff --git a/inc/languages/fr/LC_MESSAGES/shaarli.po b/inc/languages/fr/LC_MESSAGES/shaarli.po
index fda0e62f634a04f7ea34d3a8b848ffc7a1e98f03..fbb2fe64df4eb37b859c9a7896f8210ea84c20de 100644 (file)
--- a/inc/languages/fr/LC_MESSAGES/shaarli.po
+++ b/inc/languages/fr/LC_MESSAGES/shaarli.po
@@ -1,15 +1,15 @@
 msgid ""
 msgstr ""
 "Project-Id-Version: Shaarli\n"
-"POT-Creation-Date: 2020-07-23 17:11+0200\n"
-"PO-Revision-Date: 2020-07-23 17:12+0200\n"
+"POT-Creation-Date: 2020-08-27 12:01+0200\n"
+"PO-Revision-Date: 2020-08-27 12:02+0200\n"
 "Last-Translator: \n"
 "Language-Team: Shaarli\n"
 "Language: fr_FR\n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
-"X-Generator: Poedit 2.0.6\n"
+"X-Generator: Poedit 2.3\n"
 "X-Poedit-Basepath: ../../../..\n"
 "Plural-Forms: nplurals=2; plural=(n > 1);\n"
 "X-Poedit-SourceCharset: UTF-8\n"
@@ -103,36 +103,36 @@ msgstr "Mo"
 msgid "GiB"
 msgstr "Go"
 
-#: application/bookmark/BookmarkFileService.php:161
-#: application/bookmark/BookmarkFileService.php:186
-#: application/bookmark/BookmarkFileService.php:211
-#: application/bookmark/BookmarkFileService.php:228
+#: application/bookmark/BookmarkFileService.php:165
+#: application/bookmark/BookmarkFileService.php:190
+#: application/bookmark/BookmarkFileService.php:215
+#: application/bookmark/BookmarkFileService.php:232
 msgid "You're not authorized to alter the datastore"
 msgstr "Vous n'êtes pas autorisé à modifier les données"
 
-#: application/bookmark/BookmarkFileService.php:164
-#: application/bookmark/BookmarkFileService.php:189
-#: application/bookmark/BookmarkFileService.php:231
+#: application/bookmark/BookmarkFileService.php:168
+#: application/bookmark/BookmarkFileService.php:193
+#: application/bookmark/BookmarkFileService.php:235
 msgid "Provided data is invalid"
 msgstr "Les informations fournies ne sont pas valides"
 
-#: application/bookmark/BookmarkFileService.php:192
+#: application/bookmark/BookmarkFileService.php:196
 msgid "This bookmarks already exists"
 msgstr "Ce marque-page existe déjà."
 
-#: application/bookmark/BookmarkInitializer.php:40
+#: application/bookmark/BookmarkInitializer.php:37
 #: application/legacy/LegacyLinkDB.php:266
 msgid "My secret stuff... - Pastebin.com"
 msgstr "Mes trucs secrets... - Pastebin.com"
 
-#: application/bookmark/BookmarkInitializer.php:42
+#: application/bookmark/BookmarkInitializer.php:39
 #: application/legacy/LegacyLinkDB.php:268
 msgid "Shhhh! I'm a private link only YOU can see. You can delete me too."
 msgstr ""
 "Pssst ! Je suis un lien privé que VOUS êtes le seul à voir. Vous pouvez me "
 "supprimer aussi."
 
-#: application/bookmark/BookmarkInitializer.php:48
+#: application/bookmark/BookmarkInitializer.php:45
 #: application/legacy/LegacyLinkDB.php:246
 #: tmp/page.footer.b91ef64efc3688266305ea9b42e5017e.rtpl.php:15
 #: tmp/page.footer.b91ef64efc3688266305ea9b42e5017e.rtpl.php:49
@@ -144,7 +144,7 @@ msgstr ""
 "Le gestionnaire de marque-pages personnel, minimaliste, et sans base de "
 "données"
 
-#: application/bookmark/BookmarkInitializer.php:51
+#: application/bookmark/BookmarkInitializer.php:48
 #: application/legacy/LegacyLinkDB.php:249
 msgid ""
 "Welcome to Shaarli! This is your first public bookmark. To edit or delete "
@@ -210,10 +210,6 @@ msgstr "Liens directs"
 msgid "Permalink"
 msgstr "Permalien"
 
-#: application/front/ShaarliMiddleware.php:78
-msgid "An unexpected error occurred."
-msgstr "Une erreur inattendue s'est produite."
-
 #: application/front/controller/admin/ConfigureController.php:54
 #: tmp/configure.b91ef64efc3688266305ea9b42e5017e.rtpl.php:24
 msgid "Configure"
@@ -230,7 +226,7 @@ msgid "Please synchronize them."
 msgstr "Merci de les synchroniser."
 
 #: application/front/controller/admin/ConfigureController.php:113
-#: application/front/controller/visitor/InstallController.php:137
+#: application/front/controller/visitor/InstallController.php:136
 msgid "Error while writing config file after configuration update."
 msgstr ""
 "Une erreur s'est produite lors de la sauvegarde du fichier de configuration."
@@ -373,7 +369,7 @@ msgstr "Mise à jour des miniatures"
 msgid "Tools"
 msgstr "Outils"
 
-#: application/front/controller/visitor/BookmarkListController.php:114
+#: application/front/controller/visitor/BookmarkListController.php:115
 msgid "Search: "
 msgstr "Recherche : "
 
@@ -385,13 +381,17 @@ msgstr "Aujourd'hui"
 msgid "Yesterday"
 msgstr "Hier"
 
-#: application/front/controller/visitor/DailyController.php:86
+#: application/front/controller/visitor/DailyController.php:85
 #: tmp/page.header.b91ef64efc3688266305ea9b42e5017e.rtpl.php:48
 #: tmp/page.header.cedf684561d925457130839629000a81.rtpl.php:48
 msgid "Daily"
 msgstr "Quotidien"
 
-#: application/front/controller/visitor/InstallController.php:74
+#: application/front/controller/visitor/ErrorController.php:36
+msgid "An unexpected error occurred."
+msgstr "Une erreur inattendue s'est produite."
+
+#: application/front/controller/visitor/InstallController.php:73
 #, php-format
 msgid ""
 "<pre>Sessions do not seem to work correctly on your server.<br>Make sure the "
@@ -410,14 +410,14 @@ msgstr ""
 "des cookies. Nous vous recommandons d'accéder à votre serveur depuis son "
 "adresse IP ou un <em>Fully Qualified Domain Name</em>.<br>"
 
-#: application/front/controller/visitor/InstallController.php:149
+#: application/front/controller/visitor/InstallController.php:144
 msgid ""
 "Shaarli is now configured. Please login and start shaaring your bookmarks!"
 msgstr ""
 "Shaarli est maintenant configuré. Vous pouvez vous connecter et commencez à "
 "shaare vos liens !"
 
-#: application/front/controller/visitor/InstallController.php:163
+#: application/front/controller/visitor/InstallController.php:158
 msgid "Insufficient permissions:"
 msgstr "Permissions insuffisantes :"
 
@@ -441,7 +441,7 @@ msgstr "Nom d'utilisateur ou mot de passe incorrect(s)."
 msgid "Picture wall"
 msgstr "Mur d'images"
 
-#: application/front/controller/visitor/TagCloudController.php:82
+#: application/front/controller/visitor/TagCloudController.php:80
 #, fuzzy
 #| msgid "Tag list"
 msgid "Tag "
@@ -519,6 +519,10 @@ msgstr ""
 "a été importé avec succès en %d secondes : %d liens importés, %d liens "
 "écrasés, %d liens ignorés."
 
+#: application/plugin/PluginManager.php:122
+msgid " [plugin incompatibility]: "
+msgstr " [incompatibilité de l'extension] : "
+
 #: application/plugin/exception/PluginFileNotFoundException.php:21
 #, php-format
 msgid "Plugin \"%s\" files not found."
@@ -550,11 +554,11 @@ msgstr "Shaare"
 msgid "Adds the addlink input on the linklist page."
 msgstr "Ajoute le formulaire d'ajout de liens sur la page principale."
 
-#: plugins/archiveorg/archiveorg.php:25
+#: plugins/archiveorg/archiveorg.php:26
 msgid "View on archive.org"
 msgstr "Voir sur archive.org"
 
-#: plugins/archiveorg/archiveorg.php:38
+#: plugins/archiveorg/archiveorg.php:39
 msgid "For each link, add an Archive.org icon."
 msgstr "Pour chaque lien, ajoute une icône pour Archive.org."
 
@@ -674,7 +678,7 @@ msgstr "Mauvaise réponse du hub %s"
 msgid "Enable PubSubHubbub feed publishing."
 msgstr "Active la publication de flux vers PubSubHubbub."
 
-#: plugins/qrcode/qrcode.php:72 plugins/wallabag/wallabag.php:68
+#: plugins/qrcode/qrcode.php:73 plugins/wallabag/wallabag.php:70
 msgid "For each link, add a QRCode icon."
 msgstr "Pour chaque lien, ajouter une icône de QRCode."
 
@@ -690,11 +694,11 @@ msgstr ""
 msgid "Save to wallabag"
 msgstr "Sauvegarder dans Wallabag"
 
-#: plugins/wallabag/wallabag.php:69
+#: plugins/wallabag/wallabag.php:71
 msgid "Wallabag API URL"
 msgstr "URL de l'API Wallabag"
 
-#: plugins/wallabag/wallabag.php:70
+#: plugins/wallabag/wallabag.php:72
 msgid "Wallabag API version (1 or 2)"
 msgstr "Version de l'API Wallabag (1 ou 2)"
 

diff --git a/tests/PluginManagerTest.php b/tests/PluginManagerTest.php
index 195d959c2fc4c3ab3e30d4a07b9b1ce57105a1f1..a5d5dbe988ea22ad525ed74a71b5c3d421c2d7ec 100644 (file)
--- a/tests/PluginManagerTest.php
+++ b/tests/PluginManagerTest.php
@@ -25,7 +25,7 @@ class PluginManagerTest extends \PHPUnit\Framework\TestCase
      */
     protected $pluginManager;
 
-    public function setUp()
+    public function setUp(): void
     {
         $conf = new ConfigManager('');
         $this->pluginManager = new PluginManager($conf);
@@ -33,10 +33,8 @@ class PluginManagerTest extends \PHPUnit\Framework\TestCase
 
     /**
      * Test plugin loading and hook execution.
-     *
-     * @return void
      */
-    public function testPlugin()
+    public function testPlugin(): void
     {
         PluginManager::$PLUGINS_PATH = self::$pluginPath;
         $this->pluginManager->load(array(self::$pluginName));
@@ -56,10 +54,29 @@ class PluginManagerTest extends \PHPUnit\Framework\TestCase
         $this->assertEquals('loggedin', $data[1]);
     }
 
+    /**
+     * Test plugin loading and hook execution with an error: raise an incompatibility error.
+     */
+    public function testPluginWithPhpError(): void
+    {
+        PluginManager::$PLUGINS_PATH = self::$pluginPath;
+        $this->pluginManager->load(array(self::$pluginName));
+
+        $this->assertTrue(function_exists('hook_test_error'));
+
+        $data = [];
+        $this->pluginManager->executeHooks('error', $data);
+
+        $this->assertSame(
+            'test [plugin incompatibility]: Class \'Unknown\' not found',
+            $this->pluginManager->getErrors()[0]
+        );
+    }
+
     /**
      * Test missing plugin loading.
      */
-    public function testPluginNotFound()
+    public function testPluginNotFound(): void
     {
         $this->pluginManager->load(array());
         $this->pluginManager->load(array('nope', 'renope'));
@@ -69,7 +86,7 @@ class PluginManagerTest extends \PHPUnit\Framework\TestCase
     /**
      * Test plugin metadata loading.
      */
-    public function testGetPluginsMeta()
+    public function testGetPluginsMeta(): void
     {
         PluginManager::$PLUGINS_PATH = self::$pluginPath;
         $this->pluginManager->load(array(self::$pluginName));

diff --git a/tests/container/ContainerBuilderTest.php b/tests/container/ContainerBuilderTest.php
index fa77bf310c361dc363c9d476d8f41c1d4a254060..c08010ae915a778e161a424d0ead26a097b20d5e 100644 (file)
--- a/tests/container/ContainerBuilderTest.php
+++ b/tests/container/ContainerBuilderTest.php
@@ -9,6 +9,7 @@ use Shaarli\Bookmark\BookmarkServiceInterface;
 use Shaarli\Config\ConfigManager;
 use Shaarli\Feed\FeedBuilder;
 use Shaarli\Formatter\FormatterFactory;
+use Shaarli\Front\Controller\Visitor\ErrorController;
 use Shaarli\History;
 use Shaarli\Http\HttpAccess;
 use Shaarli\Netscape\NetscapeBookmarkUtils;
@@ -20,6 +21,7 @@ use Shaarli\Security\LoginManager;
 use Shaarli\Security\SessionManager;
 use Shaarli\Thumbnailer;
 use Shaarli\Updater\Updater;
+use Slim\Http\Environment;
 
 class ContainerBuilderTest extends TestCase
 {
@@ -59,20 +61,23 @@ class ContainerBuilderTest extends TestCase
     {
         $container = $this->containerBuilder->build();
 
-        static::assertInstanceOf(ConfigManager::class, $container->conf);
-        static::assertInstanceOf(SessionManager::class, $container->sessionManager);
+        static::assertInstanceOf(BookmarkServiceInterface::class, $container->bookmarkService);
         static::assertInstanceOf(CookieManager::class, $container->cookieManager);
-        static::assertInstanceOf(LoginManager::class, $container->loginManager);
+        static::assertInstanceOf(ConfigManager::class, $container->conf);
+        static::assertInstanceOf(ErrorController::class, $container->errorHandler);
+        static::assertInstanceOf(Environment::class, $container->environment);
+        static::assertInstanceOf(FeedBuilder::class, $container->feedBuilder);
+        static::assertInstanceOf(FormatterFactory::class, $container->formatterFactory);
         static::assertInstanceOf(History::class, $container->history);
-        static::assertInstanceOf(BookmarkServiceInterface::class, $container->bookmarkService);
+        static::assertInstanceOf(HttpAccess::class, $container->httpAccess);
+        static::assertInstanceOf(LoginManager::class, $container->loginManager);
+        static::assertInstanceOf(NetscapeBookmarkUtils::class, $container->netscapeBookmarkUtils);
         static::assertInstanceOf(PageBuilder::class, $container->pageBuilder);
-        static::assertInstanceOf(PluginManager::class, $container->pluginManager);
-        static::assertInstanceOf(FormatterFactory::class, $container->formatterFactory);
         static::assertInstanceOf(PageCacheManager::class, $container->pageCacheManager);
-        static::assertInstanceOf(FeedBuilder::class, $container->feedBuilder);
+        static::assertInstanceOf(ErrorController::class, $container->phpErrorHandler);
+        static::assertInstanceOf(PluginManager::class, $container->pluginManager);
+        static::assertInstanceOf(SessionManager::class, $container->sessionManager);
         static::assertInstanceOf(Thumbnailer::class, $container->thumbnailer);
-        static::assertInstanceOf(HttpAccess::class, $container->httpAccess);
-        static::assertInstanceOf(NetscapeBookmarkUtils::class, $container->netscapeBookmarkUtils);
         static::assertInstanceOf(Updater::class, $container->updater);
 
         // Set by the middleware

diff --git a/tests/plugins/test/test.php b/tests/plugins/test/test.php
index 2aaf51223e326e108451b0ba77b026db7828b6d0..ae5032dd36a5b19fc999c387c008ff781481c70c 100644 (file)
--- a/tests/plugins/test/test.php
+++ b/tests/plugins/test/test.php
@@ -19,3 +19,8 @@ function hook_test_random($data)
 
     return $data;
 }
+
+function hook_test_error()
+{
+    new Unknown();
+}