Bug 30997 - "CGI::param called in list context" warning in detail.pl flooding error log
Summary: "CGI::param called in list context" warning in detail.pl flooding error log
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Peter Vashchuk
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25790
  Show dependency treegraph
 
Reported: 2022-06-20 13:55 UTC by Peter Vashchuk
Modified: 2022-06-21 21:40 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the cause of "CGI::param called in list context from" warning messages that appear in the log files when viewing record detail pages in the staff interface.
Version(s) released in:
22.11.00


Attachments
Bug 30997: Fix "CGI::param called in list context" warning in detail.pl (1.38 KB, patch)
2022-06-20 14:25 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 30997: Fix "CGI::param called in list context" warning in detail.pl (1.43 KB, patch)
2022-06-20 17:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 30997: Fix "CGI::param called in list context" warning in detail.pl (1.51 KB, patch)
2022-06-21 07:15 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Vashchuk 2022-06-20 13:55:00 UTC

    
Comment 1 Peter Vashchuk 2022-06-20 14:25:04 UTC
Created attachment 136374 [details] [review]
Bug 30997: Fix "CGI::param called in list context" warning in detail.pl

CGI param found1 should be explicitly scalar,
or else error log gets flooded with this warning:

CGI::param called in list context from
/usr/share/koha/intranet/cgi-bin/catalogue/detail.pl line 622

This patch fixes it by working with it in a scalar context.
The functionality still remains the same but warning doesn't flood 
error log.

To reproduce:
1. Open any biblio (details.pl) page.
2. Check the error log and find the upper mentioned warning, 
check the timestamp to ensure that it was added when you loaded the page.
3. Apply the patch.
4. Load the page again, ensure that the same warning doesn't get added
to the log file again.
Comment 2 David Nind 2022-06-20 17:47:56 UTC
Created attachment 136377 [details] [review]
Bug 30997: Fix "CGI::param called in list context" warning in detail.pl

CGI param found1 should be explicitly scalar,
or else error log gets flooded with this warning:

CGI::param called in list context from
/usr/share/koha/intranet/cgi-bin/catalogue/detail.pl line 622

This patch fixes it by working with it in a scalar context.
The functionality still remains the same but warning doesn't flood
error log.

To reproduce:
1. Open any biblio (details.pl) page.
2. Check the error log and find the upper mentioned warning,
check the timestamp to ensure that it was added when you loaded the page.
3. Apply the patch.
4. Load the page again, ensure that the same warning doesn't get added
to the log file again.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2022-06-20 18:19:05 UTC
Testing notes (using koha-testing-docker):

1. For me, the error only appeared twice in the logs for the first two records accessed. After a flush_memcached and restart_all it appears again twice.

2. Search/records used: either perl or cat, then paged through several results.

3. Also tried closing browser (set so cache is cleared and history isn't remembered), but required a restart_all for the error to start appearing in the logs again.
Comment 4 Jonathan Druart 2022-06-21 07:15:26 UTC
Created attachment 136386 [details] [review]
Bug 30997: Fix "CGI::param called in list context" warning in detail.pl

CGI param found1 should be explicitly scalar,
or else error log gets flooded with this warning:

CGI::param called in list context from
/usr/share/koha/intranet/cgi-bin/catalogue/detail.pl line 622

This patch fixes it by working with it in a scalar context.
The functionality still remains the same but warning doesn't flood
error log.

To reproduce:
1. Open any biblio (details.pl) page.
2. Check the error log and find the upper mentioned warning,
check the timestamp to ensure that it was added when you loaded the page.
3. Apply the patch.
4. Load the page again, ensure that the same warning doesn't get added
to the log file again.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Andrew Nugged 2022-06-21 08:55:48 UTC
sidenote to David:

authors of CGI.pm decided to "warn only once" that's why Plack restart helps: it starts from again "$LIST_CONTEXT_WARN == 1".

from CGI.pm source in sub param:

        if ( wantarray && $LIST_CONTEXT_WARN == 1 ) {
                my ( $package, $filename, $line ) = caller;
                if ( $package ne 'CGI' ) {
                        $LIST_CONTEXT_WARN++; # only warn once
                        warn "CGI::param called in list context from $filename line $line, this can lead to vulnerabilities. "
                                . 'See the warning in "Fetching the value or values of a single named parameter"';
                }
        }

Petro: Maybe the commit explanation might be expanded with the note "on freshly started plack because this wards suppressed later by CGI.pm"
Comment 6 David Nind 2022-06-21 09:43:42 UTC
(In reply to Andrew Nugged from comment #5)
> sidenote to David:
> 
> authors of CGI.pm decided to "warn only once" that's why Plack restart
> helps: it starts from again "$LIST_CONTEXT_WARN == 1".

Thanks Andrew! 

That explains things.

David
Comment 7 Tomás Cohen Arazi 2022-06-21 18:29:35 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!