| Summary: | Report params not escaped when downloading | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> | 
| Component: | Reports | Assignee: | Nick Clemens (kidclamp) <nick> | 
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> | 
| Severity: | normal | ||
| Priority: | P5 - low | CC: | fridolin.somers, jonathan.druart, josef.moravec, martin.renvoize | 
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Crowdfunding goal: | 0 | 
| Patch complexity: | Trivial patch | Documentation contact: | |
| Documentation submission: | Text to go in the release notes: | ||
| Version(s) released in: | Circulation function: | ||
| Bug Depends on: | 18497 | ||
| Bug Blocks: | |||
| Attachments: | Bug 20945: Escape SQL parameters when constructing download links Bug 20945: Escape SQL parameters when constructing download links Bug 20945: Escape SQL parameters when constructing download links Bug 20945: (RM follow-up) Escape param_name too | ||
| 
        
          Description
        
        
          Nick Clemens (kidclamp)
        
        
        
        
          2018-06-14 14:20:29 UTC
        
       Created attachment 76056 [details] [review] Bug 20945: Escape SQL parameters when constructing download links TO test: 1 - Create a report that takes a parameter 2 - Enter a parameter containing '%' 3 - Attempt to download report, note link is misconstructed 4 - Apply patch 5 - Reload 6 - Note URL is now correct I've been testing with the following SQL: select * from borrowers where firstname like <<TEST|Asort1>> and surname like <<surname>> Asort1 has a % in my test. But I can download fine in all formats and the URL looks like this: cgi-bin/koha/reports/guided_reports.pl?reports=2&phase=Run+this+report¶m_name=TEST|Asort1&sql_params=%25¶m_name=surname&sql_params=%25 Can you explain more about the problem? What am I missing? Ah, I don't see it in Chrome, but I do in Firefox, what browser are you using? Firefox - maybe a different version? But it trying to be smart could explain. (In reply to Katrin Fischer from comment #4) > Firefox - maybe a different version? But it trying to be smart could explain. My version: 61.0b13 Just to check, that URL you pasted is the report url, I meant the URL of the download links. To download as tab I get: http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Export&format=tab&report_id=13&reportname=Test&sql_params=%25¶m_name=Enter|Asort1 Versus the report URL: http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?reports=13&phase=Run+this+report¶m_name=Enter%7CAsort1&sql_params=%2525# Notice the param changes from: %2525 in the report URL to %25 in the download URL This has the effect of running the report with param as '%' when downloading (because it translates %25 as an html code) The results will download, however, they will include all the barcodes, instead of the limited set it should be Created attachment 76423 [details] [review] Bug 20945: Escape SQL parameters when constructing download links TO test: 1 - Create a report that takes a parameter 2 - Enter a parameter containing '%' 3 - Attempt to download report, note link is misconstructed 4 - Apply patch 5 - Reload 6 - Note URL is now correct Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 76558 [details] [review] Bug 20945: Escape SQL parameters when constructing download links TO test: 1 - Create a report that takes a parameter 2 - Enter a parameter containing '%' 3 - Attempt to download report, note link is misconstructed 4 - Apply patch 5 - Reload 6 - Note URL is now correct Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Trivial fix that works as expected, Passed QA :) Any good reasons to not escape param_name as well? Created attachment 76587 [details] [review] Bug 20945: (RM follow-up) Escape param_name too Awesome work all! Pushed to master for 18.11 Pushed to 18.05.x for 18.05.02 Pushed to 17.11.x for 17.11.08. Follow-up not needed, it depends on Bug 9634 not in 17.11.x Depends on Bug 18497 not in 17.05.x |