Description
Theodoros Theodoropoulos
2019-11-06 13:48:58 UTC
Created attachment 95107 [details] [review] Allow HTML in saved reports note field Created attachment 95108 [details] [review] Allow HTML in saved reports note field patch based on Koha 18.11.06 code Hi Theodoros, all patches must be based on the current master branch and if it's a bug will then be backported to the older supported versions. Also, you need to create a patch, not just a diff - the file is missing some necessary information and can't be applied with our tools. Do you want to have another go at this? Please let us know if we can help! +1 We are currently using a custom patch to achieve this. Created attachment 161711 [details] [review] Bug 23978: Expose HTML in Reports This patch updates the notes field to a $raw filter to prevent html escaping of the data within it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Is there a more secure way of doing this rather than just exposing the raw html.. I feel like we're just undoing a security flaw we fixed for a reason. Is it time to use markdown for rich text or perhaps for linebreaks just outputting the note field in a pre/code block? (In reply to Martin Renvoize from comment #6) > Is there a more secure way of doing this rather than just exposing the raw > html.. I feel like we're just undoing a security flaw we fixed for a reason. Yeah I don't think we can just expose the raw HTML. One option would be to use the HTML scrubber. I think there are quite a few parts of Koha where people want to use HTML, but could be limited to a fairly small subset of elements and attributes. > Is it time to use markdown for rich text or perhaps for linebreaks just > outputting the note field in a pre/code block? For line breaks, the "html_line_break" filter can be useful. For notes, adding that line break filter would make sense. I don't know that any other HTML features would really needed though. If they were to be added, I think we'd have to scrub them first. (In reply to Martin Renvoize from comment #5) > This patch updates the notes field to a $raw filter to prevent html > escaping of the data within it. (In reply to David Cook from comment #7) > Yeah I don't think we can just expose the raw HTML. Sounds like a security problem and a FQA to me. Please set back to NSO if you disagree. :-) Created attachment 182627 [details] [review] Bug 23978: Expose HTML in Reports This patch updates the notes field to a $raw filter to prevent html escaping of the data within it. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Created attachment 182628 [details] [review] Bug 23978: (QA follow-up) Use html_scrubber This followup replaces the $raw filter with `scrub_html type => 'note' | $raw`. I added a QA follow-up here which should hopefully resolve the security side of it. I'm guessing one of the rebase scripts was this and it's altered the authorship. I'll look at fixing that... Created attachment 182642 [details] [review] Bug 23978: (QA follow-up) Use html_scrubber This followup replaces the $raw filter with `scrub_html type => 'note' | $raw`. Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 182643 [details] [review] Bug 23978: Expose HTML in Reports This patch updates the notes field to a $raw filter to prevent html escaping of the data within it. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 182644 [details] [review] Bug 23978: (QA follow-up) Use html_scrubber This followup replaces the $raw filter with `scrub_html type => 'note' | $raw`. Signed-off-by: David Cook <dcook@prosentient.com.au> Nice work everyone! Pushed to main for 25.11 Nice work everyone! Pushed to 25.05.x for 25.05.03 I had a go at a release note, but I'm not sure that I got what this does right. Some things I noticed - should these be separate bugs? 1. In the table listing saved reports, the HTML code is shown in the notes column. 2. When you view the report (list of reports > Actions > View), the notes text is all over the place: - The "Notes" label is indented - None of the note text is aligned properly, even if you use paragraph tags for every line in the note 3. When you run the report, the notes text is formatted correctly above the report results (as expected). Enhancement, not backported to 24.11.x |