[Bug 39866] New: Acquisitions statistics fails when filling only the To date
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 Bug ID: 39866 Summary: Acquisitions statistics fails when filling only the To date Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Reports Assignee: koha-bugs@lists.koha-community.org Reporter: adolfo.rodriguez@xercode.es QA Contact: testopia@bugs.koha-community.org The Acquisitions statistics returns an Internal Server Error if you fill only the To date. The error happens with both "Placed on" and "Received on". When executing the query, the script checks that the To date is filled but it doesn't check if the From date is also filled. if ( (@linefilter) and ( $linefilter[1] ) ) { $sth->execute( $linefilter[0], $linefilter[1] ); However, it checks both when building the SQL if (@linefilter) { if ( $linefilter[1] ) { if ( $linefilter[0] ) { $strsth .= " AND $line BETWEEN ? AND ? "; } else { $strsth .= " AND $line <= ? "; } } elsif ( ( $linefilter[0] ) and ( ( $line =~ /closedate/ ) or ( $line =~ /received/ ) ) ) { $strsth .= " AND $line >= ? "; } elsif ( $linefilter[0] ) { $linefilter[0] =~ s/\*/%/g; $strsth .= " AND $line LIKE ? "; } } This discrepancy is what creates the error, as it passes the SQL more variables than it expects. -- 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=39866 --- Comment #1 from Adolfo Rodríguez Taboada <adolfo.rodriguez@xercode.es> --- Created attachment 182160 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182160&action=edit Bug 39866: Acquisitions statistics fails when filling only the To date This patch fixes the exection of the report Acquisitions statistics if you fill only the To date Test plan: 1 Execute the report Acquisitions statistics filling only To date filter in Placed on or Received on (/cgi-bin/koha/reports/acquisitions_stats.pl) Koha returns an Internal Server Error 2 Apply patch, restart services 3 Repeat step 1, now the report returns the results page as expected -- 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=39866 Adolfo Rodríguez Taboada <adolfo.rodriguez@xercode.es> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=39866 Roman Dolny <roman.dolny@jezuici.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- 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=39866 Roman Dolny <roman.dolny@jezuici.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #182160|0 |1 is obsolete| | --- Comment #2 from Roman Dolny <roman.dolny@jezuici.pl> --- Created attachment 182227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182227&action=edit Bug 39866: Acquisitions statistics fails when filling only the To date This patch fixes the exection of the report Acquisitions statistics if you fill only the To date Test plan: 1 Execute the report Acquisitions statistics filling only To date filter in Placed on or Received on (/cgi-bin/koha/reports/acquisitions_stats.pl) Koha returns an Internal Server Error 2 Apply patch, restart services 3 Repeat step 1, now the report returns the results page as expected Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> -- 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=39866 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor -- 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=39866 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com Status|Signed Off |Failed QA --- Comment #3 from Jonathan Druart <jonathan.druart@gmail.com> --- Please setup the pre-commit git hook and do not forget to run the QA script. FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt` -- 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=39866 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- 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=39866 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |adolfo.rodriguez@xercode.es |ity.org | -- 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=39866 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | CC| |m.de.rooy@rijksmuseum.nl --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #182227|0 |1 is obsolete| | --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 182854 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182854&action=edit Bug 39866: Acquisitions statistics fails when filling only the To date This patch fixes the exection of the report Acquisitions statistics if you fill only the To date Test plan: 1 Execute the report Acquisitions statistics filling only To date filter in Placed on or Received on (/cgi-bin/koha/reports/acquisitions_stats.pl) Koha returns an Internal Server Error 2 Apply patch, restart services 3 Repeat step 1, now the report returns the results page as expected Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Removed superfluous COALESCEs. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 182855 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182855&action=edit Bug 39866: (QA follow-up) Rudimentary validation of line/column Prevent SQL injection. Note: output_error in Output.pm needs further attention, but out of scope here. (The error passed gets replaced by 404 here too.) Test plan: Pass something else than table.field in Line or Column URL param. (Manipulate URL.) You should be redirected to 404 error instead of 500. 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=39866 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: Thx for patching this! This script is not optimal indeed. my $line = $input->param("Line"); WHERE $line IS NOT NULL This script has issues with parameter validation all together. We can post stuff to it without using the template. And it does not expect that. We should be sure that this is not some 'nice' SQL injection thing. Same for $column btw. Prevent 500s like CGI::Compile::ROOT::usr_share_koha_reports_acquisitions_stats_2epl::calculate(): DBI Exception: DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '; FROM aqorders LEFT JOIN aqbasket ON (aqorders.basketno = ...' at line 1 at /usr/share/koha/reports/acquisitions_stats.pl line 70 => Adding a simple bailout in follow-up. (Did not add checking the actual field in the regex here.) - WHERE $line IS NOT NULL AND $line <> '' "; + WHERE $line IS NOT NULL AND COALESCE($line,'') <> '' "; Same for $column $column is a database field now. If it is NULL the first part is false. If it is not NULL, the second part is the same as $line <> "". No need to change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 --- Comment #8 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|25.11.00 |25.11.00,25.05.01 released in| | CC| |fridolin.somers@biblibre.co | |m Status|Pushed to main |Pushed to stable --- Comment #9 from Fridolin Somers <fridolin.somers@biblibre.com> --- I see in 25.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 --- Comment #10 from Fridolin Somers <fridolin.somers@biblibre.com> --- Does not apply easy on 24.11.x Please provide a rebased patch if needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fixes an internal release notes| |server error when using the | |acquisitions statistics | |wizard report. The error | |was generated when only the | |"To" date was filled in for | |either the "Placed on" and | |"Received on" options. CC| |david@davidnind.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- Lets skip ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39866 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED --- Comment #12 from David Nind <david@davidnind.com> --- Bug fix, no changes to the manual required. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org