Summary: | Acquisitions statistics fails when filling only the To date | ||
---|---|---|---|
Product: | Koha | Reporter: | Adolfo Rodríguez Taboada <adolfo.rodriguez> |
Component: | Reports | Assignee: | Adolfo Rodríguez Taboada <adolfo.rodriguez> |
Status: | Pushed to stable --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | minor | ||
Priority: | P5 - low | CC: | david, fridolin.somers, jonathan.druart, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: |
This fixes an internal 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.
|
|
Version(s) released in: |
25.11.00,25.05.01
|
Circulation function: | |
Attachments: |
Bug 39866: Acquisitions statistics fails when filling only the To date
Bug 39866: Acquisitions statistics fails when filling only the To date Bug 39866: Acquisitions statistics fails when filling only the To date Bug 39866: (QA follow-up) Rudimentary validation of line/column |
Description
Adolfo Rodríguez Taboada
2025-05-09 08:58:47 UTC
Created attachment 182160 [details] [review] 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 Created attachment 182227 [details] [review] 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> 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` Looking here Created attachment 182854 [details] [review] 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. Created attachment 182855 [details] [review] 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> 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. Nice work everyone! Pushed to main for 25.11 I see in 25.05.x Does not apply easy on 24.11.x Please provide a rebased patch if needed. |