Bug 28052 - keys omitted in check for system preference override
Summary: keys omitted in check for system preference override
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Colin Campbell
QA Contact: Testopia
URL:
Keywords:
: 25993 (view as bug list)
Depends on: 20954
Blocks:
  Show dependency treegraph
 
Reported: 2021-03-30 11:12 UTC by Colin Campbell
Modified: 2021-04-24 17:17 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.05,20.05.11


Attachments
patch to extract keys only for loop (1.07 KB, patch)
2021-03-30 11:28 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 28052 Do not check SYSENV values only keys in loop (1.13 KB, patch)
2021-03-30 12:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28052 Do not check SYSENV values only keys in loop (1.70 KB, patch)
2021-03-30 14:06 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2021-03-30 11:12:30 UTC
loop should look at the keys of of system environment settings, it does not and treats the hash as an array - it can cause misleading errors as the contents of some some keys are undefined.
Comment 1 Colin Campbell 2021-03-30 11:28:38 UTC
Created attachment 118990 [details] [review]
patch to extract keys only for loop

Very trivial patch. loop body only concerned with keys, should not be looking at values
Comment 2 Colin Campbell 2021-03-30 11:30:52 UTC
omission of keys was probably a typo in original patch
Comment 3 Martin Renvoize 2021-03-30 12:21:37 UTC
*** Bug 25993 has been marked as a duplicate of this bug. ***
Comment 4 Martin Renvoize 2021-03-30 12:22:59 UTC
Created attachment 119000 [details] [review]
Bug 28052 Do not check SYSENV values only keys in loop

Loop is only interested in key values, because 'keys' is omitted
it is checking values as well. This can lead to misleading errors
being logged (e.g. key is undefined)
Extract keys only to loop array as in the other loops below this

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Nick Clemens 2021-03-30 14:06:13 UTC
Created attachment 119010 [details] [review]
Bug 28052 Do not check SYSENV values only keys in loop

Loop is only interested in key values, because 'keys' is omitted
it is checking values as well. This can lead to misleading errors
being logged (e.g. key is undefined)
Extract keys only to loop array as in the other loops below this

Testing:
1 - tail /var/lib/koha/kohadev/sip-output.log
2 - in another tab use the sip cli tester:
   perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m checkin --item CHESS1
3 - Note errors:
Use of uninitialized value $key in index at /kohadevbox/koha/C4/SIP/Sip/MsgType.pm line 337, <STDIN> line 1.
Use of uninitialized value $key in index at /kohadevbox/koha/C4/SIP/Sip/MsgType.pm line 337, <STDIN> line 2.
4 - Apply patch and restart all the things
5 - Repeat 2
6 - Errors mentioned are gone

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Jonathan Druart 2021-04-01 16:55:46 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 7 Fridolin Somers 2021-04-15 10:37:33 UTC
Pushed to 20.11.x for 20.11.05
Comment 8 Andrew Fuerste-Henry 2021-04-24 16:41:11 UTC
Pushed to 20.05.x for 20.05.11
Comment 9 Victor Grousset/tuxayo 2021-04-24 17:17:18 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.