Bug 19088 - plugins-upload.pl causes uninitialized value noise
Summary: plugins-upload.pl causes uninitialized value noise
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Lee Jamison
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-11 13:36 UTC by Lee Jamison
Modified: 2017-08-12 15:23 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Test Plugin KPZ (1.25 KB, application/zip)
2017-08-11 19:38 UTC, Lee Jamison
Details
Bug 19088: plugins-upload causes error log noise (1.40 KB, patch)
2017-08-11 19:44 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 19088: plugins-upload causes error log noise (1.45 KB, patch)
2017-08-12 15:21 UTC, M. Tompsett
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lee Jamison 2017-08-11 13:36:29 UTC
After uploading a plugin, plack-error.log indicates the message

Use of uninitialized value $op in string eq at /home/vagrant/kohaclone/plugins/plugins-upload.pl line 57.
Use of uninitialized value $op in string eq at /home/vagrant/kohaclone/plugins/plugins-upload.pl line 92.
Comment 1 Lee Jamison 2017-08-11 19:38:26 UTC
Created attachment 65926 [details]
Test Plugin KPZ

Test plugin KPZ file to use to test this bug
Comment 2 Lee Jamison 2017-08-11 19:44:31 UTC
Created attachment 65928 [details] [review]
Bug 19088: plugins-upload causes error log noise

After uploading a plugin the error log indicates
use of uninitialized value in $op. This patch
silences the noise.

To test:
1) Set <enable_plugins> to 1 (one) in koha-conf.xml.
2) Set the UseKohaPlugins system preference to 'Enable'.
3) Navigate to Administration -> Manage plugins.
4) Install the test plugin KPZ file attached to this bug.
5) Notice the uninitialized value noise in the error log.
6) Uninstall the plugin (plack restart may be required if plack is
    enabled).
7) Apply patch.
8) Install the plugin again.
9) Notice no noise in the error log.
10) Run qa tools.
11) Run prove t/db_dependent/Plugins.t
Comment 3 M. Tompsett 2017-08-12 14:50:34 UTC
Comment on attachment 65928 [details] [review]
Bug 19088: plugins-upload causes error log noise

Review of attachment 65928 [details] [review]:
-----------------------------------------------------------------

::: plugins/plugins-upload.pl
@@ +47,4 @@
>  
>  my $uploadfilename = $input->param('uploadfile');
>  my $uploadfile     = $input->upload('uploadfile');
> +my $op             = $input->param('op') || q{};

// is better.
if 'op' is '0', then this would set $op to ''.
|| evaluates truthy only.
// is a defined check.
http://perldoc.perl.org/perlop.html#Logical-Defined-Or
Comment 4 M. Tompsett 2017-08-12 15:21:04 UTC
Don't even need the plugin. Using the breadcrumbs to go back you can trigger the error over and over. :)
Comment 5 M. Tompsett 2017-08-12 15:21:34 UTC
Created attachment 65936 [details] [review]
Bug 19088: plugins-upload causes error log noise

After uploading a plugin the error log indicates
use of uninitialized value in $op. This patch
silences the noise.

To test:
1) Set <enable_plugins> to 1 (one) in koha-conf.xml.
2) Set the UseKohaPlugins system preference to 'Enable'.
3) Navigate to Administration -> Manage plugins.
4) Install the test plugin KPZ file attached to this bug.
5) Notice the uninitialized value noise in the error log.
6) Uninstall the plugin (plack restart may be required if plack is
    enabled).
7) Apply patch.
8) Install the plugin again.
9) Notice no noise in the error log.
10) Run qa tools.
11) Run prove t/db_dependent/Plugins.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 6 M. Tompsett 2017-08-12 15:23:02 UTC
In an old school git install, it still triggers noise, just in a different log. Leave it up to the tester to know which log it should be in.