The use of the CGI TT plugin in catalogue/itemsearch.tt floods logs with: itemsearch.pl: CGI::param called in list context from package Template::Document line 118, 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://xxx/cgi-bin/koha/about.pl
Created attachment 64123 [details] [review] Bug 18633: Remove useless use of CGI TT plugin The form is sent using AJAX so there is never a query string in the URL (no POST parameters either)
Test plan: 1. Verify that item search works exactly the same with or without the patch
And 2. Verify that "CGI::param" warns are gone
I still get a warning in the log: itemsearch.pl: CGI::param called in list context from package Template::Document line 3, 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.
Created attachment 64193 [details] [review] Bug 18633: Remove remaining use of CGI.param in template
Thanks for testing Owen. With this new patch the last warning should disappear
Merge conflict on clean up to date branch Applying: Bug 18633: Remove useless use of CGI TT plugin Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt Failed to merge in the changes. Patch failed at 0001 Bug 18633: Remove useless use of CGI TT plugin The copy of the patch that failed is found in: /home/vagrant/kohaclone/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-18633-Remove-useless-use-of-CGI-TT-plugin-W31_uJ.patch
Created attachment 64709 [details] [review] Bug 18633: Remove useless use of CGI TT plugin The form is sent using AJAX so there is never a query string in the URL (no POST parameters either)
Created attachment 64710 [details] [review] Bug 18633: Remove remaining use of CGI.param in template
Resurrected. Check for side-effects related to previous values being filled in. I think this might break something like that, but I have not tested. I merely made the patches apply cleanly.
Created attachment 64721 [details] [review] Bug 18633: Remove useless use of CGI TT plugin The form is sent using AJAX so there is never a query string in the URL (no POST parameters either) Tested both patches together, item search behaves as before, log entries are gone Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 64722 [details] [review] Bug 18633: Remove remaining use of CGI.param in template Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 64900 [details] [review] Bug 18633: Remove useless use of CGI TT plugin The form is sent using AJAX so there is never a query string in the URL (no POST parameters either) Tested both patches together, item search behaves as before, log entries are gone Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 64901 [details] [review] Bug 18633: Remove remaining use of CGI.param in template Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, will be in 17.05.02
Doesn't apply cleanly on 16.11.x - please rebase!
Created attachment 65502 [details] [review] Bug 18633: Remove useless use of CGI TT plugin [16.11.x] The form is sent using AJAX so there is never a query string in the URL (no POST parameters either) Tested both patches together, item search behaves as before, log entries are gone Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 65503 [details] [review] Bug 18633: Remove remaining use of CGI.param in template [16.11.x] Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
I believe I have rebased this for 16.11.x correctly. However, I did not test. I probably should have removed the sign offs for the 16.11.x
Hi Mark, thx for the patches. I tested and it seems they break itemsearch on 16.11.x. Please take another look, if you can.
Closing.