Bug 23978

Summary: Notes field in saved reports should allow for (scrubbed) HTML
Product: Koha Reporter: Theodoros Theodoropoulos <theod>
Component: ReportsAssignee: Theodoros Theodoropoulos <theod>
Status: Needs documenting --- QA Contact: David Cook <dcook>
Severity: enhancement    
Priority: P5 - low CC: baptiste.wojtkowski, david, dcook, magnus, martin.renvoize, mirjam.vantieghem, severine.queune
Version: MainKeywords: rel_25_11_candidate
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22938
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22702
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22501
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23194
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23683
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19613
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40079
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
This enhancement to the notes field for saved SQL reports now allows the use of selected HTML in the field.
Version(s) released in:
25.11.00,25.05.01
Circulation function:
Attachments: Allow HTML in saved reports note field
Allow HTML in saved reports note field
Bug 23978: Expose HTML in Reports
Bug 23978: Expose HTML in Reports
Bug 23978: (QA follow-up) Use html_scrubber
Bug 23978: (QA follow-up) Use html_scrubber
Bug 23978: Expose HTML in Reports
Bug 23978: (QA follow-up) Use html_scrubber

Description Theodoros Theodoropoulos 2019-11-06 13:48:58 UTC
In recent Koha 18.11, parsing of HTML tags was removed from many fields for security reasons. However, as also seen in 22938, 22702, 22501, 23194, 23683 there are places where HTML (or at leas line breaks!) are required, or the output is too messy.

The 'notes' field in the "Guided reports wizard › Saved reports" displays the data that is previously entered in a textarea. However, the guided_reports_start.tt does not parse the HTML (nor line breaks), so the output in the resulting table is all in one line!

As with the other patches, I propose that we allow HTML in report notes, so one can use strong/italics/colors/urls in notes, and most importantly, line breaks!
Comment 1 Theodoros Theodoropoulos 2019-11-06 14:08:34 UTC
Created attachment 95107 [details] [review]
Allow HTML in saved reports note field
Comment 2 Theodoros Theodoropoulos 2019-11-06 14:25:51 UTC
Created attachment 95108 [details] [review]
Allow HTML in saved reports note field

patch based on Koha 18.11.06 code
Comment 3 Katrin Fischer 2019-11-24 13:54:34 UTC
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!
Comment 4 Mirjam Vantieghem 2024-01-30 11:02:31 UTC
+1 We are currently using a custom patch to achieve this.
Comment 5 Martin Renvoize (ashimema) 2024-02-01 06:46:29 UTC
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>
Comment 6 Martin Renvoize (ashimema) 2024-02-01 06:48:50 UTC
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?
Comment 7 David Cook 2024-02-06 22:35:35 UTC
(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.
Comment 8 Magnus Enger 2024-02-16 14:39:22 UTC
(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. :-)
Comment 9 Martin Renvoize (ashimema) 2025-05-19 15:51:38 UTC
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>
Comment 10 Martin Renvoize (ashimema) 2025-05-19 15:51:41 UTC
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`.
Comment 11 Martin Renvoize (ashimema) 2025-05-19 15:53:37 UTC
I added a QA follow-up here which should hopefully resolve the security side of it.
Comment 12 David Cook 2025-05-20 02:21:32 UTC
I'm guessing one of the rebase scripts was this and it's altered the authorship. I'll look at fixing that...
Comment 13 David Cook 2025-05-20 02:59:57 UTC Comment hidden (obsolete)
Comment 14 David Cook 2025-05-20 03:01:08 UTC
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>
Comment 15 David Cook 2025-05-20 03:01:11 UTC
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>
Comment 16 Lucas Gass (lukeg) 2025-06-04 22:08:05 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 17 Paul Derscheid 2025-06-22 21:46:47 UTC
Nice work everyone!

Pushed to 25.05.x for 25.05.03
Comment 18 David Nind 2025-07-01 20:57:49 UTC
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).
Comment 19 Baptiste Wojtkowski (bwoj) 2025-07-09 13:33:39 UTC
Enhancement, not backported to 24.11.x