Bug 16809

Summary: Silence CGI param warnings from C4::Biblio::TransformHtmlToMarc
Product: Koha Reporter: M. Tompsett <mtompset>
Component: Staff ClientAssignee: M. Tompsett <mtompset>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: gmcharlt, julian.maurice, kyle, liz, m.de.rooy
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16746
Change sponsored?: --- Patch complexity: Trivial patch
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
Attachments: Bug 16809: Silence authority warnings C4::Biblio::TransformHtmlToMarc
Bug 16809: Silence authority warnings C4::Biblio::TransformHtmlToMarc
Bug 16809: Follow-up for scalarizing biblionumber
Bug 16809: Silence authority warnings C4::Biblio::TransformHtmlToMarc
Bug 16809: Follow-up for scalarizing biblionumber

Description M. Tompsett 2016-06-24 16:07:09 UTC
While trying to test another bug. I was annoyed by the floody nature of TransformHtmlToMarc in C4/Biblio.pm

To reproduce:
 1) Back up your intranet error log
    -- for example:
       cp ~/koha-dev/var/log/koha-error-log ~/koha-error-log.backup
 2) Blank your intranet error log
    -- for example:
       echo > ~/koha-dev/var/log/koha-error-log
 3) Log into your staff client
 4) Click 'Authorities'
 5) Click 'New from Z39.50'
 5) Type 'Seuss' into 'Name (any):' and press enter
 6) Click 'Import' beside the first link
 7) Click 'Save'
 8) Check your koha-error-log
    -- floody!
 9) Apply patch
10) repeat steps 2-8
    -- blank!
11) restore your intranet error log
   -- for example:
      mv ~/koha-error-log.backup ~/koha-dev/var/log/koha-error-log
12) run koha qa test tools
Comment 1 M. Tompsett 2016-06-24 16:12:04 UTC Comment hidden (obsolete)
Comment 2 Liz Rea 2016-07-22 04:42:38 UTC
Hi Mark,

I was unable to replicate the flooding, but the patch doesn't seem to hurt anything. 

Not super into change for the sake of it, can you still replicate this on current master?

Cheers,
Liz
Comment 3 M. Tompsett 2016-08-07 15:19:44 UTC
mtompset@debian:~/kohaclone$ echo > ~/koha-dev/var/log/koha-error_log
mtompset@debian:~/kohaclone$ cat ~/koha-dev/var/log/koha-error_log | cut -f12- -d" " | sort -u

[Sun Aug  7 10:40:24 2016] authorities.pl: CGI::param called in list context from package C4::Biblio line 2580, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436., referer: http://library-admin.debiankoha.ca/cgi-bin/koha/authorities/authorities.pl?breedingid=115&authtypecode=PERSO_NAME&authid=0
[Sun Aug  7 10:40:24 2016] authorities.pl: CGI::param called in list context from package C4::Biblio line 2588, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436., referer: http://library-admin.debiankoha.ca/cgi-bin/koha/authorities/authorities.pl?breedingid=115&authtypecode=PERSO_NAME&authid=0
[Sun Aug  7 10:40:24 2016] authorities.pl: CGI::param called in list context from package C4::Biblio line 2605, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436., referer: http://library-admin.debiankoha.ca/cgi-bin/koha/authorities/authorities.pl?breedingid=115&authtypecode=PERSO_NAME&authid=0
[Sun Aug  7 10:40:24 2016] authorities.pl: CGI::param called in list context from package C4::Biblio line 2608, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436., referer: http://library-admin.debiankoha.ca/cgi-bin/koha/authorities/authorities.pl?breedingid=115&authtypecode=PERSO_NAME&authid=0

$ uname -a
Linux debian 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt25-2+deb8u3 (2016-07-02) x86_64 GNU/Linux
$ sudo apache2 -v
Server version: Apache/2.4.10 (Debian)
Server built:   Jul 20 2016 06:48:18

Yes, Liz. I do still get it.
Comment 4 Marcel de Rooy 2016-08-17 13:43:52 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2016-08-17 13:43:56 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-08-19 15:47:02 UTC
Created attachment 54649 [details] [review]
Bug 16809: Silence authority warnings C4::Biblio::TransformHtmlToMarc

Most of the floodiness is caused by accessing the cgi parameters
in a context which is hard to determine. By purposefully saving
the value to a scalar variable and using the variable, the issue
disappears, and it will likely be a tiny tad faster as variable
access is faster than multiple function calls.

TEST PLAN
---------
 1) Back up your intranet error log
    -- for example:
       cp ~/koha-dev/var/log/koha-error-log ~/koha-error-log.backup
 2) Blank your intranet error log
    -- for example:
       echo > ~/koha-dev/var/log/koha-error-log
 3) Log into your staff client
 4) Click 'Authorities'
 5) Click 'New from Z39.50'
 5) Type 'Seuss' into 'Name (any):' and press enter
 6) Click 'Import' beside the first link
 7) Click 'Save'
 8) Check your koha-error-log
    -- floody!
 9) Apply patch
10) repeat steps 2-8
    -- blank!
11) restore your intranet error log
   -- for example:
      mv ~/koha-error-log.backup ~/koha-dev/var/log/koha-error-log
12) run koha qa test tools

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with addbiblio.pl. I would have preferred the scalar option in terms
of simpler code, but this works too.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2016-08-19 15:47:23 UTC
Created attachment 54650 [details] [review]
Bug 16809: Follow-up for scalarizing biblionumber

Still resolves another multi_param warning.

Test plan:
Look at your logs before and after this patch when saving a biblio
record (you may have to start plack again).
If your biblionumber is mapped to 999c, you should no longer have a warn
about line 2563 (disclaimer: line numbers are subject to change).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Kyle M Hall 2016-09-02 16:49:58 UTC
Pushed to master for 16.11, thanks Marcel, Mark!
Comment 9 Julian Maurice 2016-09-16 08:00:16 UTC
Pushed to 3.22.x, will be in 3.22.11