Bug 30997

Summary: "CGI::param called in list context" warning in detail.pl flooding error log
Product: Koha Reporter: Peter Vashchuk <stalkernoid>
Component: CatalogingAssignee: Peter Vashchuk <stalkernoid>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: arthur.suzuki, david, lucas, m.de.rooy, nugged, victor
Version: unspecified   
Hardware: All   
OS: All   
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, 22.05.03, 21.11.11
Circulation function:
Bug Depends on:    
Bug Blocks: 25790    
Attachments: Bug 30997: Fix "CGI::param called in list context" warning in detail.pl
Bug 30997: Fix "CGI::param called in list context" warning in detail.pl
Bug 30997: Fix "CGI::param called in list context" warning in detail.pl

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 Andrii 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 (tcohen) 2022-06-21 18:29:35 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 8 Lucas Gass (lukeg) 2022-07-15 17:51:25 UTC
Backported to 22.05.x for 22.05.03
Comment 9 Arthur Suzuki 2022-07-21 14:28:10 UTC
pushed to 21.11.x for 21.11.11
Comment 10 Victor Grousset/tuxayo 2022-07-24 22:01:25 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.