Bug 18771 - CGI.pm: Subroutine multi_param redefined
Summary: CGI.pm: Subroutine multi_param redefined
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: M. Tompsett
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-09 17:36 UTC by M. Tompsett
Modified: 2017-07-31 12:00 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18771: CGI.pm: Subroutine multi_param redefined (2.74 KB, patch)
2017-06-09 17:37 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 18771: CGI.pm: Subroutine multi_param redefined (1.31 KB, patch)
2017-06-09 17:39 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 18771: CGI.pm: Subroutine multi_param redefined (2.79 KB, patch)
2017-06-29 21:59 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 18771: CGI.pm: Subroutine multi_param redefined (1.36 KB, patch)
2017-06-29 21:59 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 18771: CGI.pm: Subroutine multi_param redefined (2.89 KB, patch)
2017-06-30 10:23 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18771: CGI.pm: Subroutine multi_param redefined (1.46 KB, patch)
2017-06-30 10:23 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18771: CGI.pm: Subroutine multi_param redefined (2.01 KB, patch)
2017-06-30 14:34 UTC, M. Tompsett
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description M. Tompsett 2017-06-09 17:36:04 UTC
Without a require CGI, the $CGI::VERSION checks in C4::Context are checking against undefined. I have a series of 2 patches to demonstrate and then fix the missing require.
Comment 1 M. Tompsett 2017-06-09 17:37:34 UTC
Created attachment 64165 [details] [review]
Bug 18771: CGI.pm: Subroutine multi_param redefined

The commands in the test plan are examples, and may need varying
depending on your installation. This was created as a result
of attempting to clean the installation process up. However,
I believe the redefine might exist normally too. I just didn't
check. This is tested on a Debian 8 box sudo apt-get update'd
fully.

TEST PLAN
---------
empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

drop and recreate and empty db
> drop database koha_library;
> create database koha_library;
> quit

run the web installer, but DO NOT LOG IN!
*opening chrome to Staff Client URL*

check the error log
$ less ~/koha-dev/var/log/koha-error_log
...
[Fri Jun 09 13:08:52.793627 2017] [cgi:error] [pid 5802] [client 192.168.71.101:58169] AH01215: [Fri Jun  9 13:08:52 2017] CGI.pm: Subroutine multi_param redefined at /usr/share/perl5/CGI.pm line 419.
...

apply first patch

empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

refresh the installation login page

recheck the error log
$ less ~/koha-dev/var/log/koha-error_log

[Fri Jun 09 12:59:06.705954 2017] [cgi:error] [pid 5692] [client 192.168.71.101:57887] AH01215: Low version
[Fri Jun 09 12:59:06.723323 2017] [cgi:error] [pid 5692] [client 192.168.71.101:57887] AH01215: Is actually: 4.09
...

notice that 4.09 is bigger than the 4.08 condition, and that
the code was triggered because $CGI::VERSION is undefined,
since there was no require to set the VERSION value.

apply second patch

empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

refresh the installation login page

recheck the error log
$ less ~/koha-dev/var/log/koha-error_log

notice no reference to "Subroutine multi_param redefined"

run koha qa test tools
Comment 2 M. Tompsett 2017-06-09 17:39:35 UTC
Created attachment 64166 [details] [review]
Bug 18771: CGI.pm: Subroutine multi_param redefined

TEST PLAN
---------
Follow the test plan on the first patch.

Notice that with both patches applied, it is just a require CGI;
and comment added.
Comment 3 M. Tompsett 2017-06-13 13:15:41 UTC
I figured a side trip to a fresh web install after
> drop database koha_library;
> create database koha_library;
would kind of tie into the fixing tests that we are doing with making tests more robust. Hope you don't mind being added, Lee and Jonathan. :)
Comment 4 Lee Jamison 2017-06-13 13:26:26 UTC
Not at all! I'll take a look at it this morning
Comment 5 Lee Jamison 2017-06-29 21:59:11 UTC
Created attachment 64729 [details] [review]
Bug 18771: CGI.pm: Subroutine multi_param redefined

The commands in the test plan are examples, and may need varying
depending on your installation. This was created as a result
of attempting to clean the installation process up. However,
I believe the redefine might exist normally too. I just didn't
check. This is tested on a Debian 8 box sudo apt-get update'd
fully.

TEST PLAN
---------
empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

drop and recreate and empty db
> drop database koha_library;
> create database koha_library;
> quit

run the web installer, but DO NOT LOG IN!
*opening chrome to Staff Client URL*

check the error log
$ less ~/koha-dev/var/log/koha-error_log
...
[Fri Jun 09 13:08:52.793627 2017] [cgi:error] [pid 5802] [client 192.168.71.101:58169] AH01215: [Fri Jun  9 13:08:52 2017] CGI.pm: Subroutine multi_param redefined at /usr/share/perl5/CGI.pm line 419.
...

apply first patch

empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

refresh the installation login page

recheck the error log
$ less ~/koha-dev/var/log/koha-error_log

[Fri Jun 09 12:59:06.705954 2017] [cgi:error] [pid 5692] [client 192.168.71.101:57887] AH01215: Low version
[Fri Jun 09 12:59:06.723323 2017] [cgi:error] [pid 5692] [client 192.168.71.101:57887] AH01215: Is actually: 4.09
...

notice that 4.09 is bigger than the 4.08 condition, and that
the code was triggered because $CGI::VERSION is undefined,
since there was no require to set the VERSION value.

apply second patch

empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

refresh the installation login page

recheck the error log
$ less ~/koha-dev/var/log/koha-error_log

notice no reference to "Subroutine multi_param redefined"

run koha qa test tools

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 6 Lee Jamison 2017-06-29 21:59:21 UTC
Created attachment 64730 [details] [review]
Bug 18771: CGI.pm: Subroutine multi_param redefined

TEST PLAN
---------
Follow the test plan on the first patch.

Notice that with both patches applied, it is just a require CGI;
and comment added.

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 7 M. Tompsett 2017-06-29 22:40:31 UTC
Comment on attachment 64729 [details] [review]
Bug 18771: CGI.pm: Subroutine multi_param redefined

Needed to make the second patch apply. Patches were split so testing could be done to prove why the final result is the final result.
Comment 8 Marcel de Rooy 2017-06-30 10:23:56 UTC
Created attachment 64753 [details] [review]
Bug 18771: CGI.pm: Subroutine multi_param redefined

The commands in the test plan are examples, and may need varying
depending on your installation. This was created as a result
of attempting to clean the installation process up. However,
I believe the redefine might exist normally too. I just didn't
check. This is tested on a Debian 8 box sudo apt-get update'd
fully.

TEST PLAN
---------
empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

drop and recreate and empty db
> drop database koha_library;
> create database koha_library;
> quit

run the web installer, but DO NOT LOG IN!
*opening chrome to Staff Client URL*

check the error log
$ less ~/koha-dev/var/log/koha-error_log
...
[Fri Jun 09 13:08:52.793627 2017] [cgi:error] [pid 5802] [client 192.168.71.101:58169] AH01215: [Fri Jun  9 13:08:52 2017] CGI.pm: Subroutine multi_param redefined at /usr/share/perl5/CGI.pm line 419.
...

apply first patch

empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

refresh the installation login page

recheck the error log
$ less ~/koha-dev/var/log/koha-error_log

[Fri Jun 09 12:59:06.705954 2017] [cgi:error] [pid 5692] [client 192.168.71.101:57887] AH01215: Low version
[Fri Jun 09 12:59:06.723323 2017] [cgi:error] [pid 5692] [client 192.168.71.101:57887] AH01215: Is actually: 4.09
...

notice that 4.09 is bigger than the 4.08 condition, and that
the code was triggered because $CGI::VERSION is undefined,
since there was no require to set the VERSION value.

apply second patch

empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

refresh the installation login page

recheck the error log
$ less ~/koha-dev/var/log/koha-error_log

notice no reference to "Subroutine multi_param redefined"

run koha qa test tools

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2017-06-30 10:23:59 UTC
Created attachment 64754 [details] [review]
Bug 18771: CGI.pm: Subroutine multi_param redefined

TEST PLAN
---------
Follow the test plan on the first patch.

Notice that with both patches applied, it is just a require CGI;
and comment added.

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2017-06-30 10:24:55 UTC
Would certainly not oppose to squash both patches and reduce the verbosity of the commit message now. The point has been made very clear. Thx Mark!
Comment 11 M. Tompsett 2017-06-30 14:23:59 UTC
(In reply to Marcel de Rooy from comment #10)
> Would certainly not oppose to squash both patches and reduce the verbosity
> of the commit message now. The point has been made very clear. Thx Mark!

Sometimes during the course of doing a patch, you need to show the intermediate work. Since this has passed QA, I concur that a squash is in order.
Comment 12 M. Tompsett 2017-06-30 14:34:11 UTC
Created attachment 64755 [details] [review]
Bug 18771: CGI.pm: Subroutine multi_param redefined

The commands in the test plan are examples, and may need varying
depending on your installation. This was created as a result
of attempting to clean the installation process up. However,
I believe the redefine might exist normally too. I just didn't
check. This is tested on a Debian 8 box sudo apt-get update'd
fully.

TEST PLAN
---------
empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

drop and recreate and empty db
> drop database koha_library;
> create database koha_library;
> quit

run the web installer, but DO NOT LOG IN!
*opening chrome to Staff Client URL*

check the error log
$ less ~/koha-dev/var/log/koha-error_log
...
[Fri Jun 09 13:08:52.793627 2017] [cgi:error] [pid 5802] [client 192.168.71.101:58169] AH01215: [Fri Jun  9 13:08:52 2017] CGI.pm: Subroutine multi_param redefined at /usr/share/perl5/CGI.pm line 419.
...

apply patch

empty error log
$ echo > ~/koha-dev/var/log/koha-error_log

refresh the installation login page

recheck the error log
$ less ~/koha-dev/var/log/koha-error_log

notice no reference to "Subroutine multi_param redefined"

run koha qa test tools

Notice that it is just a require CGI; and comment added.

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 M. Tompsett 2017-06-30 14:35:31 UTC
Squashed and shortened commit message as per comment #10.
Comment 14 Jonathan Druart 2017-07-06 17:44:10 UTC
Note: This only affects installations without plack running
Comment 15 Jonathan Druart 2017-07-06 18:51:16 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 16 Fridolin SOMERS 2017-07-12 08:09:48 UTC
Pushed to 17.05.x, will be in 17.05.02
Comment 17 Katrin Fischer 2017-07-16 11:43:31 UTC
This patch has been pushed to 16.11.x and will be in 16.11.10.
Comment 18 Mason James 2017-07-31 12:00:45 UTC
Pushed to 16.05.x, for 16.05.15 release