Bug 14487 - Warns from returns.pl during checkin
Summary: Warns from returns.pl during checkin
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Indranil Das Gupta
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-02 20:31 UTC by Indranil Das Gupta
Modified: 2016-12-05 21:23 UTC (History)
8 users (show)

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


Attachments
Bug 14487 - silence warns during checkin (1.22 KB, patch)
2015-07-03 08:11 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 14487 - silence warns during checkin (1.12 KB, patch)
2015-07-03 19:55 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 14487 - silence warns during checkin (1.29 KB, patch)
2015-07-03 21:05 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14487: Noise best detected under Debian 8 (1.86 KB, patch)
2015-07-03 22:10 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14487: Noise best detected under Debian 8 (1.94 KB, patch)
2015-07-04 05:29 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 14487 - silence warns during checkin (1.34 KB, patch)
2015-07-18 17:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14487: Noise best detected under Debian 8 (1.99 KB, patch)
2015-07-18 17:33 UTC, Nick Clemens
Details | Diff | Splinter Review
[PASSED QA] Bug 14487 - silence warns during checkin (1.43 KB, patch)
2015-07-24 16:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 14487: Noise best detected under Debian 8 (2.06 KB, patch)
2015-07-24 16:08 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Indranil Das Gupta 2015-07-02 20:31:04 UTC
Suppress these warns during returning an item

returns.pl: Use of uninitialized value $holdingBranch in string eq at /home/l2c2technologies/koha/circ/returns.pl line 623., referer: http://kohamaster.l2c2.co.in/cgi-bin/koha/members/readingrec.pl?borrowernumber=51

returns.pl: Use of uninitialized value $collectionBranch in string eq at /home/l2c2technologies/koha/circ/returns.pl line 623., referer: http://kohamaster.l2c2.co.in/cgi-bin/koha/members/readingrec.pl?borrowernumber=51
Comment 1 Indranil Das Gupta 2015-07-03 07:41:09 UTC
(In reply to Indranil Das Gupta from comment #0)
> Suppress these warns during returning an item

The warns have stopped for some reason. I need to investigate this a bit more.
Comment 2 Indranil Das Gupta 2015-07-03 08:11:03 UTC Comment hidden (obsolete)
Comment 3 Indranil Das Gupta 2015-07-03 08:12:07 UTC
(In reply to Indranil Das Gupta from comment #1)
> (In reply to Indranil Das Gupta from comment #0)
> > Suppress these warns during returning an item
> 
> The warns have stopped for some reason. I need to investigate this a bit
> more.

please ignore comment #1
Comment 4 Indranil Das Gupta 2015-07-03 19:55:27 UTC Comment hidden (obsolete)
Comment 5 Mark Tompsett 2015-07-03 21:05:52 UTC Comment hidden (obsolete)
Comment 6 Mark Tompsett 2015-07-03 21:08:39 UTC
[Fri Jul 03 17:00:54.327367 2015] [cgi:error] [pid 2960] [client 192.168.45.37:62532] AH01215: [Fri Jul  3 17:00:54 2015] returns.pl: Use of uninitialized value in hash element at /home/mtompset/kohaclone/circ/returns.pl line 469., referer: http://library-admin.debiankoha.ca/cgi-bin/koha/circ/returns.pl

This probably applies to more than just Debian Jessie.

[Fri Jul 03 17:00:54.334787 2015] [cgi:error] [pid 2960] [client 192.168.45.37:62532] AH01215: [Fri Jul  3 17:00:54 2015] returns.pl: CGI::param called in list context from package main line 620, 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/circ/returns.pl

This probably applies to only Debian Jessie context.

Moving this back to ASSIGNED, since I believe 469 should be fixed at least based on the bug description given.
Comment 7 Mark Tompsett 2015-07-03 21:11:32 UTC
(In reply to M. Tompsett from comment #6)
> [Fri Jul 03 17:00:54.327367 2015] [cgi:error] [pid 2960] [client
> 192.168.45.37:62532] AH01215: [Fri Jul  3 17:00:54 2015] returns.pl: Use of
> uninitialized value in hash element at
> /home/mtompset/kohaclone/circ/returns.pl line 469., referer:
> http://library-admin.debiankoha.ca/cgi-bin/koha/circ/returns.pl

Trigger case: check in something that isn't checked out.
Comment 8 Mark Tompsett 2015-07-03 22:10:07 UTC Comment hidden (obsolete)
Comment 9 Indranil Das Gupta 2015-07-04 05:29:13 UTC Comment hidden (obsolete)
Comment 10 Nick Clemens 2015-07-18 14:20:04 UTC
All patches appear to be signed off, marking as such
Comment 11 Mark Tompsett 2015-07-18 17:10:11 UTC
(In reply to Nick Clemens from comment #10)
> All patches appear to be signed off, marking as such

I happen to know that Indranil did not have a Debian 8 VM, so I don't think he accurately triggered the noise, though he did confirm that it still worked as expected. That is why it was still needs sign off.

Like I said, this needs a Debian 8 (not Jessie release candidate) updated to test the second patch fully (because otherwise you won't likely see the messages as in comment #6. That's why I'm sticking it back to Needs Sign off.
Comment 12 Nick Clemens 2015-07-18 17:33:43 UTC Comment hidden (obsolete)
Comment 13 Nick Clemens 2015-07-18 17:33:50 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall 2015-07-24 16:07:52 UTC
Created attachment 41173 [details] [review]
[PASSED QA] Bug 14487 - silence warns during checkin

Test plan
=========

1/ check out an item and then check it back in.
1/ check the logs after the check-in to see the warns from
   returns.pl line 623 of :
    (a) Use of uninitialized value $holdingBranch
    (b) Use of uninitialized value $collectionBranch
2/ apply patch
3/ check out and check-in again. no warns are recorded this time.

NOTE: Under Debian Jessie, there are other messages.
      Additionally, this only corrects the line 623 ones.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 15 Kyle M Hall 2015-07-24 16:08:01 UTC
Created attachment 41174 [details] [review]
[PASSED QA] Bug 14487: Noise best detected under Debian 8

The first error is caused by the fact that
$messages->{'IsPermanent'} is undefined.

The second error is caused by querying the CGI
parameter 'barcode' inside a function call. This is not required.
There is a variable $barcode set with the parameter. Changed to
use the variable.

TEST PLAN
----------
1) Test first patch.
2) Clear the log
3) Put in a barcode which is not checked out.
4) Check the log.
   -- should be two errors. One about a hash,
      the other will only be detectable under Debian 8.
5) Apply this second patch
6) Clear the log
7) Put in a barcode which is not checked out.
8) Check the log.
   -- should be empty.
9) run koha qa test tools

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 16 Tomás Cohen Arazi 2015-07-28 13:37:29 UTC
Patches pushed to master.

Thanks Indranil and Mark!
Comment 17 Chris Cormack 2015-08-10 21:05:49 UTC
Pushed to 3.20.x will be in 3.20.3
Comment 18 Liz Rea 2015-08-11 02:05:33 UTC
Pushed to 3.18.x, will be in 3.18.10.
Comment 19 Liz Rea 2015-08-11 02:06:15 UTC
Pushed to 3.18.x, will be in 3.18.10.
Comment 20 Mason James 2015-08-28 06:04:31 UTC
Pushed to 3.16.x, will be in 3.16.14