Bug 8861

Summary: Undefined variables in batchMod.pl trigger error logs
Product: Koha Reporter: Mark Tompsett <mtompset>
Component: ToolsAssignee: Mark Tompsett <mtompset>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: minor    
Priority: P5 - low CC: chris, jonathan.druart, kyle, paul.poulain
Version: 3.10   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Fix inconsequential error loggling in batchMod.pl
Fix inconsequential error loggling in batchMod.pl (3.8, 3.6)
[SIGNED-OFF] Bug 8861 - Undefined variables in batchMod.pl trigger error logs
[SIGNED-OFF] Bug 8861 - Undefined variables in batchMod.pl trigger error logs (3.8, 3.6)

Description Mark Tompsett 2012-10-02 12:56:09 UTC
This module was looked at because of:
[Tue Oct 02 17:00:56 2012] [error] [client 192.168.100.2] [Tue Oct  2 17:00:56 2012] batchMod.pl: Problem = a value of 1 has been passed to param without key at /usr/share/koha/lib/C4/Templates.pm line 187.

There are two sections that match, though this was likely the $op case, not the $error case:

$template->param(
    op      => $nextop,
    $op => 1,
);

foreach my $error (@errors) {
    $template->param($error => 1);
}

I would normally change:
    my $op           = $input->param('op');
into
    my $op           = $input->param('op') || q{};
However there was code that read:
    if (!defined $op) {
So, I looked to see how $op was used after that:
    if ($op eq "action") {
    if ($op eq "show"){
Of course, the above error'ing code, and lastly.
    if ($op eq "action") {
As such, I decided to define $op = q{} inside the if statement, after it ran it's !defined check. And I still moved the $op => 1 to an external, conditional template param call.

Additionally, I looked for where @errors was used in the file, but only the declaration, and this foreach had a @errors reference. And as an array could be filled with undef's, I err'd on the side of caution by adding "if $error" on the template parameter call.

This was found in 3.8.5. Looks like it is master too.
Patch to follow shortly.
Comment 1 Mark Tompsett 2012-10-02 13:45:41 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2012-10-02 13:47:37 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2012-10-05 13:38:10 UTC
Created attachment 12708 [details] [review]
[SIGNED-OFF] Bug 8861 - Undefined variables in batchMod.pl trigger error logs

Initialized $op, and changed lines like "$op => 1" and
"$error => 1" into separate, conditional template param calls.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Kyle M Hall 2012-10-05 13:40:11 UTC
Created attachment 12709 [details] [review]
[SIGNED-OFF] Bug 8861 - Undefined variables in batchMod.pl trigger error logs (3.8, 3.6)

Initialized $op, and changed lines like "$op => 1" and
"$error => 1" into separate, conditional template param calls.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Jonathan Druart 2012-10-18 14:17:32 UTC
Looks good, marked as Passed QA.
Comment 6 Paul Poulain 2012-10-19 15:48:47 UTC
Patch pushed to master
Comment 7 Chris Cormack 2013-04-21 00:51:07 UTC
Pushed to 3.8.x will be in 3.8.12