[Bug 19200] New: Warns when exporting a basket
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Bug ID: 19200 Summary: Warns when exporting a basket Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: aleisha@catalyst.net.nz QA Contact: testopia@bugs.koha-community.org When exporting a basket with the default CSV profile: Use of uninitialized value in string eq at /home/vagrant/kohaclone/acqui/basket.pl line 173. CGI::param called in list context from package CGI::Compile::ROOT::home_vagrant_kohaclone_acqui_basket_2epl line 177, 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. When exporting a basket with some other CSV profile: CGI::param called in list context from package CGI::Compile::ROOT::home_vagrant_kohaclone_acqui_basket_2epl line 177, 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. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 --- Comment #1 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Created attachment 66602 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=66602&action=edit Bug 19200: Preventing warns when exporting a basket To test: 1) Go to Tools -> CSV profiles -> New CSV Profile 2) Create a new CSV profile with any name of SQL fields. Ensure profile type: SQL and usage: basket export in acquisition 3) Go to Acquisitions -> Find or create a vendor -> Use or create a basket 4) Click the dropdown menu next to the 'Export as CSV' button. There should be a 'Default' option and your new CSV profile (at least) 5) Click the 'Default' option. Notice warns 6) Click the 'Export as CSV' button. Notice warns 7) Click your new CSV profile option. Notice warns 8) Apply patch and refresh page 9) Repeat steps 5-7, confirm that warns do not show 10) Confirm export still works as expected Sponsored-by: Catalyst IT -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |aleisha@catalyst.net.nz |ity.org | Change sponsored?|--- |Sponsored -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #66602|0 |1 is obsolete| | --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 67799 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67799&action=edit Bug 19200: Preventing warns when exporting a basket To test: 1) Go to Tools -> CSV profiles -> New CSV Profile 2) Create a new CSV profile with any name of SQL fields. Ensure profile type: SQL and usage: basket export in acquisition 3) Go to Acquisitions -> Find or create a vendor -> Use or create a basket 4) Click the dropdown menu next to the 'Export as CSV' button. There should be a 'Default' option and your new CSV profile (at least) 5) Click the 'Default' option. Notice warns 6) Click the 'Export as CSV' button. Notice warns 7) Click your new CSV profile option. Notice warns 8) Apply patch and refresh page 9) Repeat steps 5-7, confirm that warns do not show 10) Confirm export still works as expected Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Version|unspecified |master Severity|enhancement |minor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Failed QA --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- This looks like a change in behavior. When the cgi param is default, no profile id was passed. I guess what you wanted to do was just if( $param && $param eq ... ) ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #4 from Aleisha Amohia <aleisha@catalyst.net.nz> --- (In reply to Marcel de Rooy from comment #3)
This looks like a change in behavior. When the cgi param is default, no profile id was passed. I guess what you wanted to do was just if( $param && $param eq ... ) ?
Hi Marcel, there is no change in behaviour in my patch. It looks like there's originally a bug here, because the original code says "if $query->param('csv_profile') eq 'default'", but the word 'default' is never passed through that param, based on acqui/basket.tt. this means that when the Default option was selected, the $query->param('csv_profile') is not actually set to 'default' so an uninitialised value is sent to the GetBasketAsCSV method and the 'if' condition is never true. (I hope that makes sense. If you put a warn inside the if statement under where $query->param('csv_profile') eq default, you'll see that it never enters here and never prints the warn.) my patch checks if the $query->param('csv_profile') has been defined. if it has, then it uses one of the library-made csv profiles. if it hasn't been defined then it must be using the default profile and no ID is passed, as intended, with no warns. setting back to needs signoff as I believe this is still a valid patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Jon Knight <J.P.Knight@lboro.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Jon Knight <J.P.Knight@lboro.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67799|0 |1 is obsolete| | --- Comment #5 from Jon Knight <J.P.Knight@lboro.ac.uk> --- Created attachment 69959 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69959&action=edit Bug 19200: Preventing warns when exporting a basket To test: 1) Go to Tools -> CSV profiles -> New CSV Profile 2) Create a new CSV profile with any name of SQL fields. Ensure profile type: SQL and usage: basket export in acquisition 3) Go to Acquisitions -> Find or create a vendor -> Use or create a basket 4) Click the dropdown menu next to the 'Export as CSV' button. There should be a 'Default' option and your new CSV profile (at least) 5) Click the 'Default' option. Notice warns 6) Click the 'Export as CSV' button. Notice warns 7) Click your new CSV profile option. Notice warns 8) Apply patch and refresh page 9) Repeat steps 5-7, confirm that warns do not show 10) Confirm export still works as expected Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |alex.arnaud@biblibre.com |y.org | CC| |alex.arnaud@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 --- Comment #6 from Alex Arnaud <alex.arnaud@biblibre.com> --- Is the if() else() statement still usefull? Why not just one call to GetBasketAsCSV? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Resuming here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|alex.arnaud@biblibre.com |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Aleisha Amohia from comment #4)
(In reply to Marcel de Rooy from comment #3)
This looks like a change in behavior. When the cgi param is default, no profile id was passed. I guess what you wanted to do was just if( $param && $param eq ... ) ?
Hi Marcel,
there is no change in behaviour in my patch. It looks like there's originally a bug here, because the original code says "if $query->param('csv_profile') eq 'default'", but the word 'default' is never passed through that param, based on acqui/basket.tt. this means that when the Default option was selected, the $query->param('csv_profile') is not actually set to 'default' so an uninitialised value is sent to the GetBasketAsCSV method and the 'if' condition is never true. (I hope that makes sense. If you put a warn inside the if statement under where $query->param('csv_profile') eq default, you'll see that it never enters here and never prints the warn.)
my patch checks if the $query->param('csv_profile') has been defined. if it has, then it uses one of the library-made csv profiles. if it hasn't been defined then it must be using the default profile and no ID is passed, as intended, with no warns.
setting back to needs signoff as I believe this is still a valid patch.
You are right that the compare with 'default' makes no sense. It may have been a leftover from the past. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Alex Arnaud from comment #6)
Is the if() else() statement still usefull? Why not just one call to GetBasketAsCSV?
Yes. Adding a follow-up for that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #69959|0 |1 is obsolete| | --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 70113 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70113&action=edit Bug 19200: Preventing warns when exporting a basket To test: 1) Go to Tools -> CSV profiles -> New CSV Profile 2) Create a new CSV profile with any name of SQL fields. Ensure profile type: SQL and usage: basket export in acquisition 3) Go to Acquisitions -> Find or create a vendor -> Use or create a basket 4) Click the dropdown menu next to the 'Export as CSV' button. There should be a 'Default' option and your new CSV profile (at least) 5) Click the 'Default' option. Notice warns 6) Click the 'Export as CSV' button. Notice warns 7) Click your new CSV profile option. Notice warns 8) Apply patch and refresh page 9) Repeat steps 5-7, confirm that warns do not show 10) Confirm export still works as expected Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 70114 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70114&action=edit Bug 19200: (QA follow-up) Simplify call to GetBasketAsCSV If no profile_id is passed, GetBasketAsCSV will fallback to default itself. No need to make the distinction in basket.pl. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 18.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Pushed to Master |Pushed to Stable --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all, pushed to stable for 17.11.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |8612 CC| |fridolin.somers@biblibre.co | |m Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8612 [Bug 8612] CSV export profile to have custom fields in export csv basket -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19200 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Stable |RESOLVED Resolution|--- |FIXED --- Comment #14 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Depends on Bug 8612 that is not in 17.05.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org