From 375a9197b1170b92c9cd27e809bb0dcfce050517 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 27 Sep 2021 08:24:53 -0300 Subject: [PATCH] Bug 29121: Catch errors in ->install and ->upgrade calls on plugins This patch adds a try/catch block when instantiating plugins. Calling ->new on a plugin eventually triggers a call to ->install (this has always been like this since bug 7804). If the ->install method is somehow borked, then the process dies. We need to prevent that, and report back some error took place. That's what this patch does. The same happens to the ->upgrade. To test: 1. Install any plugin you like 2. Restart plack (just in case) => SUCCESS: All good 3. Manually change its install method to: sub install { die "plugin, die!"; } 4. Run: $ koha-mysql kohadev > DELETE FROM plugin_data; (to make sure there's no __INSTALLED__ entry, do on a safe to delete DB). 5. Point your browser to the plugins-home.pl page => FAIL: Boom 6. Apply up to the regression tests 7. Run: $ kshell k$ prove t/db_dependent/Koha/Plugins/Plugins.t \ t/Koha/Exceptions.t => FAIL: Tests fail! 8. Apply this patch 9. Repeat 2 => SUCCESS: Tests pass! 10. Run: $ restart_all 11. Repeat 5 => SUCCESS: The page is not broken 12. Sign off :-D Note: I used $ kshell k$ perl misc/devel/install_plugins.pl to test as well. Signed-off-by: Tomas Cohen Arazi Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart --- Koha/Plugins.pm | 39 ++++++++++++++++++++++++++++++++------- Koha/Plugins/Base.pm | 33 +++++++++++++++++++++++---------- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm index b8f9f86a33..fa4d235287 100644 --- a/Koha/Plugins.pm +++ b/Koha/Plugins.pm @@ -25,10 +25,12 @@ use List::MoreUtils qw( any ); use Module::Load::Conditional qw( can_load ); use Module::Load; use Module::Pluggable search_path => ['Koha::Plugin'], except => qr/::Edifact(|::Line|::Message|::Order|::Segment|::Transport)$/; - +use Try::Tiny; use C4::Context; use C4::Output; + +use Koha::Exceptions::Plugin; use Koha::Plugins::Methods; BEGIN { @@ -120,11 +122,23 @@ sub GetPlugins { while ( my $plugin_class = $plugin_classes->next ) { if ( can_load( modules => { $plugin_class => undef }, nocache => 1 ) ) { - my $plugin = $plugin_class->new({ - enable_plugins => $self->{'enable_plugins'} - # loads even if plugins are disabled - # FIXME: is this for testing without bothering to mock config? - }); + + my $plugin; + my $failed_instantiation; + + try { + $plugin = $plugin_class->new({ + enable_plugins => $self->{'enable_plugins'} + # loads even if plugins are disabled + # FIXME: is this for testing without bothering to mock config? + }); + } + catch { + warn "$_"; + $failed_instantiation = 1; + }; + + next if $failed_instantiation; next unless $plugin->is_enabled or defined($params->{all}) && $params->{all}; @@ -169,7 +183,18 @@ sub InstallPlugins { if ( can_load( modules => { $plugin_class => undef }, nocache => 1 ) ) { next unless $plugin_class->isa('Koha::Plugins::Base'); - my $plugin = $plugin_class->new({ enable_plugins => $self->{'enable_plugins'} }); + my $plugin; + my $failed_instantiation; + + try { + $plugin = $plugin_class->new({ enable_plugins => $self->{'enable_plugins'} }); + } + catch { + warn "$_"; + $failed_instantiation = 1; + }; + + next if $failed_instantiation; Koha::Plugins::Methods->search({ plugin_class => $plugin_class })->delete(); diff --git a/Koha/Plugins/Base.pm b/Koha/Plugins/Base.pm index 10c66d7486..92ce7714bd 100644 --- a/Koha/Plugins/Base.pm +++ b/Koha/Plugins/Base.pm @@ -21,12 +21,15 @@ use Modern::Perl; use Cwd qw( abs_path ); use List::Util qw( max ); +use Try::Tiny; use base qw{Module::Bundled::Files}; use C4::Context; use C4::Output qw( output_with_http_headers ); +use Koha::Exceptions::Plugin; + =head1 NAME Koha::Plugins::Base - Base Module for plugins @@ -48,21 +51,31 @@ sub new { ## Run the installation method if it exists and hasn't been run before if ( $self->can('install') && !$self->retrieve_data('__INSTALLED__') ) { - if ( $self->install() ) { - $self->store_data( { '__INSTALLED__' => 1, '__ENABLED__' => 1 } ); - if ( my $version = $plugin_version ) { - $self->store_data({ '__INSTALLED_VERSION__' => $version }); + try { + if ( $self->install() ) { + $self->store_data( { '__INSTALLED__' => 1, '__ENABLED__' => 1 } ); + if ( my $version = $plugin_version ) { + $self->store_data({ '__INSTALLED_VERSION__' => $version }); + } + } else { + warn "Plugin $class failed during installation!"; } - } else { - warn "Plugin $class failed during installation!"; } + catch { + Koha::Exceptions::Plugin::InstallDied->throw( plugin_class => $class ); + }; } elsif ( $self->can('upgrade') ) { if ( _version_compare( $plugin_version, $database_version ) == 1 ) { - if ( $self->upgrade() ) { - $self->store_data({ '__INSTALLED_VERSION__' => $plugin_version }); - } else { - warn "Plugin $class failed during upgrade!"; + try { + if ( $self->upgrade() ) { + $self->store_data({ '__INSTALLED_VERSION__' => $plugin_version }); + } else { + warn "Plugin $class failed during upgrade!"; + } } + catch { + Koha::Exceptions::Plugin::UpgradeDied->throw( plugin_class => $class ); + }; } } elsif ( $plugin_version ne $database_version ) { $self->store_data({ '__INSTALLED_VERSION__' => $plugin_version }); -- 2.39.5