Bug: eval of use does not protect from error messages

if a plugin that is listed in INSTALLEDPLUGINS, the .pm but is not actually in Plugins you get lots of errors in the browser

Test case

goto http://twiki.org/cgi-bin/view/Codev.IncludeUrlScrewsUpAbsoluteUrls?_foo=1.2

Software error:
Can't locate TWiki/Plugins/SpreadSheetPlugin.pm in @INC (@INC contains: ../lib . /usr/lib/perl5/5.005/i386-linux /usr/lib/perl5/5.005 /usr/local/lib/site_perl/i386-linux /usr/local/lib/site_perl /usr/lib/perl5) at (eval 16) line 1.

For help, please send mail to the webmaster (staff@sourceforge.net), giving this error message and the time and date of the error. Content-type: text/html
Software error:
[Sat Jul 24 04:24:19 2004] view: Can't locate TWiki/Plugins/SpreadSheetPlugin.pm in @INC (@INC contains: ../lib . /usr/lib/perl5/5.005/i386-linux /usr/lib/perl5/5.005 /usr/local/lib/site_perl/i386-linux /usr/local/lib/site_perl /usr/lib/perl5) at (eval 16) line 1. BEGIN failed--compilation aborted at (eval 16) line 1.

For help, please send mail to the webmaster (staff@sourceforge.net), giving this error message and the time and date of the error. Content-type: text/html
Software error:
Can't locate TWiki/Plugins/SpreadSheetPlugin.pm in @INC (@INC contains: ../lib . /usr/lib/perl5/5.005/i386-linux /usr/lib/perl5/5.005 /usr/local/lib/site_perl/i386-linux /usr/local/lib/site_perl /usr/lib/perl5) at (eval 20) line 1.

For help, please send mail to the webmaster (staff@sourceforge.net), giving this error message and the time and date of the error. Content-type: text/html
Software error:
[Sat Jul 24 04:24:20 2004] view: Can't locate TWiki/Plugins/SpreadSheetPlugin.pm in @INC (@INC contains: ../lib . /usr/lib/perl5/5.005/i386-linux /usr/lib/perl5/5.005 /usr/local/lib/site_perl/i386-linux /usr/local/lib/site_perl /usr/lib/perl5) at (eval 20) line 1. BEGIN failed--compilation aborted at (eval 20) line 1.

For help, please send mail to the webmaster (staff@sourceforge.net), giving this error message and the time and date of the error. Content-Type: text/html; charset=ISO-8859-1

Environment

-- SvenDowideit - 24 Jul 2004

Follow up

there are 2 issues here

1. we try to "eval use $p;" plugins that don't have any perl code
2. we report those errors to the browser

Fix record

to fix issue 1, i propose the following patch

Index: lib/TWiki/Plugins.pm
===================================================================
--- lib/TWiki/Plugins.pm        (revision 1594)
+++ lib/TWiki/Plugins.pm        (working copy)
@@ -289,19 +289,27 @@
     # Get INSTALLEDPLUGINS and DISABLEDPLUGINS variables
     my $plugin = &TWiki::Prefs::getPreferencesValue( "INSTALLEDPLUGINS" ) || "";
     $plugin =~ s/[\n\t\s\r]+/ /go;
-    @instPlugins = grep { /^.+Plugin$/ } split( /,?\s+/ , $plugin );
+    my @setInstPlugins = grep { /^.+Plugin$/ } split( /,?\s+/ , $plugin );
     $plugin = &TWiki::Prefs::getPreferencesValue( "DISABLEDPLUGINS" ) || "";
     $plugin =~ s/[\n\t\s\r]+/ /go;
     @disabledPlugins = map{ s/^.*\.(.*)$/$1/o; $_ }
                        grep { /^.+Plugin$/ }
                        split( /,?\s+/ , $plugin );
 
+    my @discoveredPlugins = discoverPluginPerlModules();
+    my $p = "";
+    foreach $plugin ( @setInstPlugins ) {
+        $p = $plugin;
+        $p =~ s/^.*\.(.*)$/$1/o; # cut web
+        unless( grep { /^$p$/ } @disabledPlugins ) {
+            push( @instPlugins, $plugins );
+        }
+    }
     # append discovered plugin modules to installed plugin list
-    push( @instPlugins, discoverPluginPerlModules() );
-
+    push( @instPlugins, @discoveredPlugins );
+    
     # for efficiency we register all possible handlers at once
     my $user = "";
-    my $p = "";
     my $posUser = "";
     foreach $plugin ( @instPlugins ) {
         $p = $plugin;

-- SvenDowideit - 24 Jul 2004

Looks like a sensible fix to make Plugins registration more robust.

-- PeterThoeny - 25 Jul 2004

I have commited the above patch

*For DakarRelease : * we should look toward making the error reporting mechanism consistently log to one place only (configurably the debug.txt file, the browser, or the apachelogs)

-- SvenDowideit - 25 Jul 2004

Follow Up

There is a small typo in the patch above (push variable $plugins instead of $plugin in the loop) leading to perl error messages in the httpd error log saying "Use of uninitialized value ..."

• in the last foreach loop in initialize1 and
• to error log messages saying "couldn't register , no plugin topic" in registerPlugin.

In addition I have included more tests against empty plugin names to make it more robust.

Fix Record

Changes to TWiki20040729beta:

• activated "use strict" (except for "refs")
• added a test for empty plugin name in function registerPlugin to make it more robust
• fixed typo of variable name $plugins
• added more test for empty plugin name in function initialize1 to make it more robust

--- lib/TWiki/Plugins.pm_ORIG   Mon Jul 26 00:01:59 2004
+++ lib/TWiki/Plugins.pm   Fri Aug  6 10:45:02 2004
@@ -25,7 +25,8 @@
 
 package TWiki::Plugins;
 
-##use strict;
+use strict;
+no strict 'refs';
 
 use vars qw(
         @activePluginWebs @activePluginTopics @instPlugins @disabledPlugins
@@ -150,6 +151,12 @@
     #   3 Plugins.plugin
     #   4 thisweb.plugin
 
+    # Ignore an empty plugin name (should not happen, fix the calling function!).
+    if ( ! $plugin ) {
+      initialisationError( "Plugins: undefined or empty plugin name" );
+      return;
+    }
+
     my $installWeb = '';
     # first check for fully specified plugin
     if ( $plugin =~ m/^(.+)\.([^\.]+Plugin)$/ ) {
@@ -299,10 +306,11 @@
     my @discoveredPlugins = discoverPluginPerlModules();
     my $p = "";
     foreach $plugin ( @setInstPlugins ) {
+        next if (! $plugin );  # ignore an empty entry
         $p = $plugin;
         $p =~ s/^.*\.(.*)$/$1/o; # cut web
         unless( grep { /^$p$/ } @disabledPlugins ) {
-            push( @instPlugins, $plugins );
+            push( @instPlugins, $plugin );
         }
     }
     # append discovered plugin modules to installed plugin list
@@ -312,6 +320,7 @@
     my $user = "";
     my $posUser = "";
     foreach $plugin ( @instPlugins ) {
+        next if (! $plugin );  # ignore an empty entry
         $p = $plugin;
         $p =~ s/^.*\.(.*)$/$1/o; # cut web
         unless( grep { /^$p$/ } @disabledPlugins ) {
@@ -346,6 +355,7 @@
     my $p = "";
     my $plugin = "";
     foreach $plugin ( @instPlugins ) {
+        next if (! $plugin);  # ignore an empty entry
         $p = $plugin;
         $p =~ s/^.*\.(.*)$/$1/o; # cut web
         unless( grep { /^$p$/ } @disabledPlugins ) {

-- BerndRaichle - 06 Aug 2004

Bernd, not all of the tests you added are required. I took the bulk of your patch and merged it with another patch I was preparing for performance. The attached patch addresses both. (My fixes are to remove RE searches of arrays and replace them with associative arrays. A simple fix that gives a measurable improvement in plugins discovery)

-- CrawfordCurrie - 06 Aug 2004

thankyou to both of you! I have applied this obviously superior code to SVN.

-- SvenDowideit - 06 Aug 2004