Summary: | Add multi_param call and add divider in cache key in svc/report and opac counterpart | ||
---|---|---|---|
Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
Component: | Architecture, internals, and plumbing | Assignee: | Marcel de Rooy <m.de.rooy> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | trivial | ||
Priority: | P5 - low | CC: | fridolin.somers, martin.renvoize, nick |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 21115: Add multi_param call and add divider in cache key in svc/report and opac counterpart
Bug 21115: Add multi_param call and add divider in cache key in svc/report and opac counterpart Bug 21115: Add multi_param call and add divider in cache key in svc/report and opac counterpart |
Description
Marcel de Rooy
2018-07-25 12:37:43 UTC
Created attachment 77256 [details] [review] Bug 21115: Add multi_param call and add divider in cache key in svc/report and opac counterpart Resolve things like: CGI::param called in list context from package CGI::Compile::ROOT::usr_share_koha_prodclone_opac_svc_report line 42, 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. The cache key in both script looks like: opac:report:id:602018 but should for consistency be: opac:report:id:60:2018 Note: The 2018 here is part of the sql_params and should not be concatenated to the report id. Test plan: Do not yet apply this patch. Make a report public, set cache to 300 secs. Check its output with opac/svc/report. Check for the warn in your log. Apply the patch, restart Plack and flush cache. Check opac/svc/report. Modify your report; e.g. add a simple string to the SELECT. Check opac/svc/report. You should still see cached output. Flush the cache. Check opac/svc/report. You should now see the added text. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested also by clearing individual keys with $cache->clear_from_cache. Note for QA: Looks like we should remove the opac and intranet key from the cache when modifying a report. But that is not the topic of this report. So I use this phenomenon in my test plan. Created attachment 80365 [details] [review] Bug 21115: Add multi_param call and add divider in cache key in svc/report and opac counterpart Resolve things like: CGI::param called in list context from package CGI::Compile::ROOT::usr_share_koha_prodclone_opac_svc_report line 42, 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. The cache key in both script looks like: opac:report:id:602018 but should for consistency be: opac:report:id:60:2018 Note: The 2018 here is part of the sql_params and should not be concatenated to the report id. Test plan: Do not yet apply this patch. Make a report public, set cache to 300 secs. Check its output with opac/svc/report. Check for the warn in your log. Apply the patch, restart Plack and flush cache. Check opac/svc/report. Modify your report; e.g. add a simple string to the SELECT. Check opac/svc/report. You should still see cached output. Flush the cache. Check opac/svc/report. You should now see the added text. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested also by clearing individual keys with $cache->clear_from_cache. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> (In reply to M. Tompsett from comment #3) > Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Thx Created attachment 80441 [details] [review] Bug 21115: Add multi_param call and add divider in cache key in svc/report and opac counterpart Resolve things like: CGI::param called in list context from package CGI::Compile::ROOT::usr_share_koha_prodclone_opac_svc_report line 42, 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. The cache key in both script looks like: opac:report:id:602018 but should for consistency be: opac:report:id:60:2018 Note: The 2018 here is part of the sql_params and should not be concatenated to the report id. Test plan: Do not yet apply this patch. Make a report public, set cache to 300 secs. Check its output with opac/svc/report. Check for the warn in your log. Apply the patch, restart Plack and flush cache. Check opac/svc/report. Modify your report; e.g. add a simple string to the SELECT. Check opac/svc/report. You should still see cached output. Flush the cache. Check opac/svc/report. You should now see the added text. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested also by clearing individual keys with $cache->clear_from_cache. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Awesome work all! Pushed to master for 18.11 Pushed to 18.05.x for 18.05.05 Pushed to 17.11.x for 17.11.12 |