Bug 16809 - Silence CGI param warnings from C4::Biblio::TransformHtmlToMarc
Summary: Silence CGI param warnings from C4::Biblio::TransformHtmlToMarc
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Mark Tompsett
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-24 16:07 UTC by Mark Tompsett
Modified: 2017-12-07 22:17 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16809: Silence authority warnings C4::Biblio::TransformHtmlToMarc (3.63 KB, patch)
2016-06-24 16:12 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16809: Silence authority warnings C4::Biblio::TransformHtmlToMarc (3.84 KB, patch)
2016-08-17 13:43 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16809: Follow-up for scalarizing biblionumber (1.71 KB, patch)
2016-08-17 13:43 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16809: Silence authority warnings C4::Biblio::TransformHtmlToMarc (3.87 KB, patch)
2016-08-19 15:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16809: Follow-up for scalarizing biblionumber (1.74 KB, patch)
2016-08-19 15:47 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 Mark 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 Mark 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 Mark 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