Bug 35278 - CGI::param called in list context from /usr/share/koha/admin/columns_settings.pl line 76
Summary: CGI::param called in list context from /usr/share/koha/admin/columns_settings...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Marcel de Rooy
QA Contact: Emily Lamancusa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-07 13:33 UTC by Marcel de Rooy
Modified: 2023-11-14 15:02 UTC (History)
3 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:
23.11.00,23.05.06,22.11.12


Attachments
Bug 35278: Fix warn from column_settings.pl (3.08 KB, patch)
2023-11-07 15:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35278: Fix warn from column_settings.pl (3.08 KB, patch)
2023-11-07 21:52 UTC, David Nind
Details | Diff | Splinter Review
Bug 35278: Fix warn from column_settings.pl (3.15 KB, patch)
2023-11-09 17:20 UTC, Emily Lamancusa
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2023-11-07 13:33:50 UTC

    
Comment 1 Marcel de Rooy 2023-11-07 15:12:33 UTC
Created attachment 158605 [details] [review]
Bug 35278: Fix warn from column_settings.pl

We need to add scalar to the calls. These parameters are not meant
to be used as multi_param.

Test plan:
Try With and without this patch on staff:

Try: /cgi-bin/koha/admin/columns_settings.pl
=> WITHOUT: No warnings
=> WITH: Same. No change.
Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket
=> WITHOUT: Opens module, scrolls down. CGI::param called in list context in logfile
=> WITH: Opens module, scrolls down.
Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket&table=orders
=> WITHOUT: Opens module, scrolls down. CGI::param called in list context in logfile
=> WITH: Opens module, scrolls down.

Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin
* Hash like: panel => acqui, admin => page, undef  => table, undef => modules, $modules (SHIFTED)
=> WITHOUT: Acqui expanded showing: There is no page using the table configuration in this module.
=> CGI::param warning in log.
=> And: Problem = a value of table has been passed to param without key
=> And: Problem = a value of modules has been passed to param without key
=> WITH: Only the first module (Acqui) should be expanded.

Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin&page=basket&table=orders
* Hash like: panel => acqui, admin => page, basket => table, orders => modules, $modules hash (SHIFTED)
=> WITHOUT: Acqui expanded showing: There is no page using the table configuration in this module.
=> And: CGI param log warning.
=> WITH:: Opens Acqui. Second module ignored. Scrolling.

Try: /cgi-bin/koha/admin/columns_settings.pl?module=admin&module=acqui&page=basket&table=orders
* Hash like: panel => admin, acqui=> page, basket => table, orders => modules, $modules hash (SHIFTED)
=> WITHOUT: Admin expanded showing: There is no page using the table configuration in this module.
=> And: CGI param log warning.
=> WITH: Admin should be expanded (page and table ignored, not in Admin). No scroll.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2023-11-07 15:14:53 UTC
(In reply to Marcel de Rooy from comment #1)
> * Hash like: panel => acqui, admin => page, undef  => table, undef =>

Actually not a hash, but a hash-like list. That can have odd number.
Comment 3 David Nind 2023-11-07 21:52:51 UTC
Created attachment 158632 [details] [review]
Bug 35278: Fix warn from column_settings.pl

We need to add scalar to the calls. These parameters are not meant
to be used as multi_param.

Test plan:
Try With and without this patch on staff:

Try: /cgi-bin/koha/admin/columns_settings.pl
=> WITHOUT: No warnings
=> WITH: Same. No change.
Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket
=> WITHOUT: Opens module, scrolls down. CGI::param called in list context in logfile
=> WITH: Opens module, scrolls down.
Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket&table=orders
=> WITHOUT: Opens module, scrolls down. CGI::param called in list context in logfile
=> WITH: Opens module, scrolls down.

Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin
* Hash like: panel => acqui, admin => page, undef  => table, undef => modules, $modules (SHIFTED)
=> WITHOUT: Acqui expanded showing: There is no page using the table configuration in this module.
=> CGI::param warning in log.
=> And: Problem = a value of table has been passed to param without key
=> And: Problem = a value of modules has been passed to param without key
=> WITH: Only the first module (Acqui) should be expanded.

Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin&page=basket&table=orders
* Hash like: panel => acqui, admin => page, basket => table, orders => modules, $modules hash (SHIFTED)
=> WITHOUT: Acqui expanded showing: There is no page using the table configuration in this module.
=> And: CGI param log warning.
=> WITH:: Opens Acqui. Second module ignored. Scrolling.

Try: /cgi-bin/koha/admin/columns_settings.pl?module=admin&module=acqui&page=basket&table=orders
* Hash like: panel => admin, acqui=> page, basket => table, orders => modules, $modules hash (SHIFTED)
=> WITHOUT: Admin expanded showing: There is no page using the table configuration in this module.
=> And: CGI param log warning.
=> WITH: Admin should be expanded (page and table ignored, not in Admin). No scroll.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2023-11-07 21:55:04 UTC
Testing notes (using KTD)

1.Before applying the patch, ran through the URLs and noted area accessed and any warnings in the /var/log/koha/kohadev/plack-intranet-error.log log file.

1.1 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl - No error in the log file (as expected).

1.2 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket - Scrolls down to Page: basket (as expected). Error in the log file:

  1 [2023/11/07 21:16:58] [WARN] CGI::param called in list context from /kohadevbox/koha/admin/columns_settings.pl line 76, 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 414.

1.3 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket&table=orders - Scrolls down to Table id: orders (as expected). Error in the log file:

  2 [2023/11/07 21:20:33] [WARN] CGI::param called in list context from /kohadevbox/koha/admin/columns_settings.pl line 76, 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 414.

1.4 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin - Acquisition area expanded showing: "There is no page using the table configuration in this module.". Warnings in the log file: 

  3 [2023/11/07 21:27:19] [WARN] Problem = a value of table has been passed to param without key at /kohadevbox/koha/C4/Templates.pm line 143.
  4 [2023/11/07 21:27:19] [WARN] Problem = a value of modules has been passed to param without key at /kohadevbox/koha/C4/Templates.pm line 143.

1.5 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin&page=basket&table=orders - Acquisition area expanded showing: "There is no page using the table configuration in this module.". No additional warnings added to the log file. 

1.6 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=admin&module=acqui&page=basket&table=orders - Administration area expanded showing: "There is no page using the table configuration in this module.". No additional warnings added to the logfile.


2. Applied patch, flush_memcached, restart_all, fresh browser.

2.1 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl - No error in the log file (as expected).

2.2 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket - Scrolls down to Page: basket (as expected). No error in the log file. (Results as expected.)

2.3 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket&table=orders - Scrolls down to Table id: orders. No error in the log file. (Results as expected.)

2.4 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin - Acquisition area expanded. No error in the log file. (Results as expected.)

2.5 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin&page=basket&table=orders - Acquisition area expanded at Table id: orders. No error in the log file. (Results as expected.)

2.6 http://127.0.0.1:8081/cgi-bin/koha/admin/columns_settings.pl?module=admin&module=acqui&page=basket&table=orders - Administration area expanded, at the top. No error in the log file. (Results as expected.)
Comment 5 Marcel de Rooy 2023-11-08 12:12:56 UTC
Thx David.
Comment 6 Emily Lamancusa 2023-11-09 17:20:17 UTC
Created attachment 158753 [details] [review]
Bug 35278: Fix warn from column_settings.pl

We need to add scalar to the calls. These parameters are not meant
to be used as multi_param.

Test plan:
Try With and without this patch on staff:

Try: /cgi-bin/koha/admin/columns_settings.pl
=> WITHOUT: No warnings
=> WITH: Same. No change.
Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket
=> WITHOUT: Opens module, scrolls down. CGI::param called in list context in logfile
=> WITH: Opens module, scrolls down.
Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&page=basket&table=orders
=> WITHOUT: Opens module, scrolls down. CGI::param called in list context in logfile
=> WITH: Opens module, scrolls down.

Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin
* Hash like: panel => acqui, admin => page, undef  => table, undef => modules, $modules (SHIFTED)
=> WITHOUT: Acqui expanded showing: There is no page using the table configuration in this module.
=> CGI::param warning in log.
=> And: Problem = a value of table has been passed to param without key
=> And: Problem = a value of modules has been passed to param without key
=> WITH: Only the first module (Acqui) should be expanded.

Try: /cgi-bin/koha/admin/columns_settings.pl?module=acqui&module=admin&page=basket&table=orders
* Hash like: panel => acqui, admin => page, basket => table, orders => modules, $modules hash (SHIFTED)
=> WITHOUT: Acqui expanded showing: There is no page using the table configuration in this module.
=> And: CGI param log warning.
=> WITH:: Opens Acqui. Second module ignored. Scrolling.

Try: /cgi-bin/koha/admin/columns_settings.pl?module=admin&module=acqui&page=basket&table=orders
* Hash like: panel => admin, acqui=> page, basket => table, orders => modules, $modules hash (SHIFTED)
=> WITHOUT: Admin expanded showing: There is no page using the table configuration in this module.
=> And: CGI param log warning.
=> WITH: Admin should be expanded (page and table ignored, not in Admin). No scroll.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 7 Tomás Cohen Arazi 2023-11-09 17:44:15 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 8 Fridolin Somers 2023-11-11 07:27:24 UTC
Pushed to 23.05.x for 23.05.06
Comment 9 Pedro Amorim 2023-11-14 15:02:57 UTC
Nice work everyone!

Pushed to 22.11.x for next release