Bug 13618 - Add additional template filter methods and a filter presence test to Koha
Summary: Add additional template filter methods and a filter presence test to Koha
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL: https://gitlab.com/joubu/Koha/tree/bu...
Whiteboard:
Keywords:
: 20869 20870 20871 20872 20873 20874 20909 22342 (view as bug list)
Depends on: 14412 14416 14418 14423
Blocks: 21576 22007 22596 22641 14744 15717 15733 15734 15754 20975 21257 21293 21347 21377 21393 21418 21454 21476 21526 21529 21541 21663 21669 21703 21704 21706 21905 21947 22058 22442 22550 22586
  Show dependency treegraph
 
Reported: 2015-01-23 10:40 UTC by Jonathan Druart
Modified: 2019-04-04 13:43 UTC (History)
24 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
RMaint Note: The inclusion of bug 22007 in 18.11.02 means this enhancement bug gets added to the release notes.. It should be removed before release!
Version(s) released in:


Attachments
Bug 13618: [wip] Use Template::Stash::HTML::Entities to use the html filter (2.48 KB, patch)
2015-01-23 12:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Revert "Bug 13609: Cross Site Scripting problem in authority search result list paging" (3.36 KB, patch)
2015-01-26 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Use Template::Stash::AutoEscaping to use the html filter (3.09 KB, patch)
2015-06-22 14:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the OPAC (138.25 KB, patch)
2015-06-22 14:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables (2.46 KB, patch)
2015-06-22 14:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the intranet (311.55 KB, patch)
2015-06-22 15:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables - intra (1.76 KB, patch)
2015-06-22 15:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for pagination_bar (12.92 KB, patch)
2015-06-22 15:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for the ISBD view (1.90 KB, patch)
2015-06-22 15:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Use Template::Stash::AutoEscaping to use the html filter (3.09 KB, patch)
2015-08-04 14:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the OPAC (138.30 KB, patch)
2015-08-04 14:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables (2.46 KB, patch)
2015-08-04 14:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the intranet (312.30 KB, patch)
2015-08-04 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables - intra (1.76 KB, patch)
2015-08-04 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for pagination_bar (12.92 KB, patch)
2015-08-04 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for the ISBD view (1.90 KB, patch)
2015-08-04 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix error 'Not a GLOB reference' (2.75 KB, patch)
2015-08-04 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for IntranetUser* and OPACUser* prefs (5.79 KB, patch)
2015-08-04 15:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for ColumnsSettings (5.95 KB, patch)
2015-08-04 15:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618 - memberentrygen.tt errors Not a GLOB reference (3.24 KB, patch)
2015-08-20 22:55 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 13618 - memberentrygen.tt errors Not a GLOB reference (3.12 KB, patch)
2015-08-20 23:09 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 13618: Specific for other prefs (20.10 KB, patch)
2015-08-21 11:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for Salutation on editing a patron (1011 bytes, patch)
2015-08-27 08:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Use Template::Stash::AutoEscaping to use the html filter (3.23 KB, patch)
2015-08-28 09:54 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Specific for other prefs (20.14 KB, patch)
2015-08-28 12:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for XSLTBloc (2.16 KB, patch)
2015-08-28 12:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for XSLTBloc (2.30 KB, patch)
2015-08-28 12:50 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables - intra (1.91 KB, patch)
2015-08-28 13:11 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Fix escape on sending baskets or shelves by email (26.72 KB, patch)
2015-08-28 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix escape on sending baskets or shelves by email (26.87 KB, patch)
2015-08-28 15:24 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Specific for other prefs (20.29 KB, patch)
2015-08-29 07:34 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Specific for Salutation on editing a patron (1.13 KB, patch)
2015-08-29 07:36 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Specific for the ISBD view (2.04 KB, patch)
2015-08-29 07:37 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Specific for pagination_bar (12.92 KB, patch)
2015-08-29 08:09 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables (2.60 KB, patch)
2015-08-29 08:20 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Specific for ColumnsSettings (6.09 KB, patch)
2015-08-29 08:24 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Specific for pagination_bar (13.06 KB, patch)
2015-08-29 08:37 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the intranet (313.01 KB, patch)
2015-08-29 09:22 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 13618: followup to remove tabs (24.78 KB, patch)
2015-11-25 15:01 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Screenshot opac-shelves.pl (43.94 KB, image/png)
2015-12-17 21:28 UTC, Liz Rea
Details
List pages in OPAC also broken when the list contains items (55.85 KB, image/png)
2015-12-17 21:37 UTC, Liz Rea
Details
Bug 13618: (follow-up) add missing lines for opac-shelves (3.18 KB, patch)
2015-12-17 22:51 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 13618: Use Template::Stash::AutoEscaping to use the html filter (3.36 KB, patch)
2016-01-18 11:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the OPAC (132.41 KB, patch)
2016-01-18 11:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables (2.74 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the intranet (313.78 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables - intra (2.04 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for pagination_bar (11.44 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for the ISBD view (2.17 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix error 'Not a GLOB reference' (3.02 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for IntranetUser* and OPACUser* prefs (5.91 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for ColumnsSettings (6.22 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618 - memberentrygen.tt errors Not a GLOB reference (3.40 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for other prefs (20.42 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for Salutation on editing a patron (1.26 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for XSLTBloc (2.41 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix escape on sending baskets or shelves by email (26.98 KB, patch)
2016-01-18 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for news (6.03 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix last occurrences recently introduced to master (7.28 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: followup to remove tabs (23.78 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for edit biblios and items (22.44 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) Specific for ColumnsSettings (1.18 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) add missing lines for opac-shelves (3.31 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters for newly pushed code (2.15 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for system preference description (1.47 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Do not display   and html tags in item fields content (1.19 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Do not display html tags in patron's notices (1.02 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for debarredcomment and patron messages (8.41 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) Specific for other prefs (1.56 KB, patch)
2016-01-18 11:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for branches.opac_info (4.29 KB, patch)
2016-01-18 11:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Use Template::Stash::AutoEscaping to use the html filter (3.36 KB, patch)
2016-01-18 11:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the OPAC (132.41 KB, patch)
2016-01-18 11:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables (2.74 KB, patch)
2016-01-18 11:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the intranet (313.78 KB, patch)
2016-01-18 11:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables - intra (2.04 KB, patch)
2016-01-18 11:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for pagination_bar (11.44 KB, patch)
2016-01-18 11:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for the ISBD view (2.17 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix error 'Not a GLOB reference' (3.02 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for IntranetUser* and OPACUser* prefs (5.91 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for ColumnsSettings (6.22 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618 - memberentrygen.tt errors Not a GLOB reference (3.40 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for other prefs (20.42 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for Salutation on editing a patron (1.26 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for XSLTBloc (2.41 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix escape on sending baskets or shelves by email (26.98 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for news (6.03 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix last occurrences recently introduced to master (7.28 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: followup to remove tabs (23.78 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for edit biblios and items (22.44 KB, patch)
2016-01-18 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) Specific for ColumnsSettings (1.18 KB, patch)
2016-01-18 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) add missing lines for opac-shelves (3.31 KB, patch)
2016-01-18 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters for newly pushed code (2.15 KB, patch)
2016-01-18 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for system preference description (1.47 KB, patch)
2016-01-18 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Do not display   and html tags in item fields content (1.19 KB, patch)
2016-01-18 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Do not display html tags in patron's notices (1.02 KB, patch)
2016-01-18 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for debarredcomment and patron messages (8.41 KB, patch)
2016-01-18 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) Specific for other prefs (1.56 KB, patch)
2016-01-18 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for branches.opac_info (4.29 KB, patch)
2016-01-18 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Use Template::Stash::AutoEscaping to use the html filter (3.36 KB, patch)
2016-01-29 09:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the OPAC (132.52 KB, patch)
2016-01-29 09:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables (2.74 KB, patch)
2016-01-29 09:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters at the intranet (315.50 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific places where we don't need to escape variables - intra (2.04 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for pagination_bar (10.84 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for the ISBD view (2.17 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix error 'Not a GLOB reference' (3.02 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for IntranetUser* and OPACUser* prefs (5.91 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for ColumnsSettings (6.22 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618 - memberentrygen.tt errors Not a GLOB reference (3.40 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for other prefs (20.29 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for Salutation on editing a patron (1.26 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for XSLTBloc (2.41 KB, patch)
2016-01-29 09:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix escape on sending baskets or shelves by email (26.98 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for news (6.03 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix last occurrences recently introduced to master (7.28 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: followup to remove tabs (22.75 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for edit biblios and items (22.44 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) Specific for ColumnsSettings (1.18 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) add missing lines for opac-shelves (3.31 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filters for newly pushed code (2.15 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for system preference description (1.47 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Do not display   and html tags in item fields content (1.19 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Do not display html tags in patron's notices (1.02 KB, patch)
2016-01-29 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix for debarredcomment and patron messages (8.41 KB, patch)
2016-01-29 09:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) Specific for other prefs (1.56 KB, patch)
2016-01-29 09:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for branches.opac_info (4.29 KB, patch)
2016-01-29 09:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
libtemplate-stash-autoescaping-perl_0.0303-1_all.deb (19.89 KB, application/x-debian-package)
2016-02-04 06:08 UTC, Mirko Tietgen
Details
Bug 13618: Use Template::Stash::AutoEscaping to use the html filter (633.49 KB, patch)
2017-02-07 15:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Use a simplified version of Template::Stash::AutoEscaping (4.20 KB, patch)
2017-02-07 15:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Specific for branches.opac_info (4.33 KB, patch)
2017-02-07 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: [DO NOT PUSH] have fun (3.65 KB, patch)
2018-08-14 21:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 13618: [DO NOT PUSH] have fun (3.65 KB, patch)
2018-08-14 21:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 13618: [DO NOT PUSH] have fun (3.65 KB, patch)
2018-08-14 21:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: [DO NOT PUSH] have fun (3.65 KB, patch)
2018-08-14 21:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: [DO NOT PUSH] Add script to add the html filters (7.55 KB, patch)
2018-08-14 21:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
patches_to_be_pushed.tar.gz (602.32 KB, application/gzip)
2018-08-14 21:21 UTC, Jonathan Druart
Details
Bug 13618: Remove filter when assigning array (5.54 KB, patch)
2018-08-17 18:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Fix xt/tt_valid.t (1.27 KB, patch)
2018-08-17 18:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) Manually replace missing .raw (1.69 KB, patch)
2018-08-24 22:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) Add html filters to all the variables (1.15 KB, patch)
2018-08-29 17:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Do not use html filters with KohaSpan (2.34 KB, patch)
2018-09-04 19:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: (follow-up) Pass opacuser_js from plugins as raw (929 bytes, patch)
2018-09-26 10:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 13618: remove wrong html filter in 'Item search fields' admin (1.33 KB, patch)
2018-10-02 11:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13618: Remove html filter for LAST (8.74 KB, patch)
2018-10-25 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
OPAC MARC view holding detail in 18.05 (20.11 KB, image/png)
2018-12-24 16:53 UTC, Mark Alexander
Details
OPAC MARC view holding detail in 18.11 (20.80 KB, image/png)
2018-12-24 16:54 UTC, Mark Alexander
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2015-01-23 10:40:04 UTC

    
Comment 1 Jonathan Druart 2015-01-23 10:43:29 UTC
Recently, we got a lot of security bugs related to potential XSS attack.
We should think about a global solution instead of escaping everything manually in the templates.

Any thoughs?
Comment 2 Jonathan Druart 2015-01-23 11:18:55 UTC
http://www.perlmonks.org/?node_id=1059961
Comment 3 Jonathan Druart 2015-01-23 12:21:36 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-01-26 08:45:38 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2015-01-26 18:42:31 UTC
Hi Jonathan, is the second patch just for testing or supposed to go with this?
Comment 6 Tomás Cohen Arazi 2015-01-26 19:00:10 UTC
Jonathan, maybe we could think of resurrect some idea from bug 10075 and make it provide a escaped version of the params for direct usage?
Comment 7 Jonathan Druart 2015-01-27 08:18:23 UTC
(In reply to Katrin Fischer from comment #5)
> Hi Jonathan, is the second patch just for testing or supposed to go with
> this?

It is part of the POC, all html filter will become useless in templates.
Comment 8 Jonathan Druart 2015-01-27 08:19:24 UTC
(In reply to Tomás Cohen Arazi from comment #6)
> Jonathan, maybe we could think of resurrect some idea from bug 10075 and
> make it provide a escaped version of the params for direct usage?

I believe the solution I proposed is easier to implement, don't you think?
Comment 9 Fridolin SOMERS 2015-01-30 16:38:02 UTC
Interesting, I think its a good idea.

One should consider the use do vars in JS code.
Comment 10 Jonathan Druart 2015-06-22 09:52:47 UTC
Chris has found Template::Stash::AutoEscape:

http://search.cpan.org/~mala/Template-Stash-AutoEscape-0.03/lib/Template/Stash/AutoEscape.pm
Comment 11 Jonathan Druart 2015-06-22 09:57:51 UTC
and more active:
Template::Stash::AutoEscaping
http://search.cpan.org/~shlomif/Template-Stash-AutoEscaping-0.0303/lib/Template/Stash/AutoEscaping.pm
Comment 12 Chris Cormack 2015-06-22 10:13:05 UTC
Perhaps we could use this module

http://search.cpan.org/~shlomif/Template-Stash-AutoEscaping-0.0303/lib/Template/Stash/AutoEscaping.pm

As it lets us do value.raw  if we need unescaped values.

But either we use it, or the HTML::Entities one, I think we should push it very soon. And we should do it on both the staff and opac sides
Comment 13 Tomás Cohen Arazi 2015-06-22 14:03:48 UTC
(In reply to Chris Cormack from comment #12)
> Perhaps we could use this module
> 
> http://search.cpan.org/~shlomif/Template-Stash-AutoEscaping-0.0303/lib/
> Template/Stash/AutoEscaping.pm
> 
> As it lets us do value.raw  if we need unescaped values.
> 
> But either we use it, or the HTML::Entities one, I think we should push it
> very soon. And we should do it on both the staff and opac sides

+1 for Template::Stash::AutoEscaping
Comment 14 Jonathan Druart 2015-06-22 14:59:43 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2015-06-22 14:59:51 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2015-06-22 14:59:56 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2015-06-22 15:17:05 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2015-06-22 15:17:20 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2015-06-22 15:25:08 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2015-06-22 15:29:23 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2015-06-22 16:07:00 UTC
html code are generated in C4::Reports::Guided::format_results but this subroutine will be removed by bug 14435.
Comment 22 David Cook 2015-06-23 08:11:29 UTC
Neato burrito! This could come in really really handy...
Comment 23 David Cook 2015-07-06 01:58:27 UTC
Jonathan: Is this ready for testing now?
Comment 24 Jonathan Druart 2015-07-08 13:58:15 UTC
(In reply to David Cook from comment #23)
> Jonathan: Is this ready for testing now?

yep :)
Comment 25 Chris Cormack 2015-07-28 22:15:06 UTC
Would be great to get this tested, it will save us working on never ending individual fixes
Comment 26 Tomás Cohen Arazi 2015-07-29 18:45:24 UTC
I'm sorry, could you please rebase. There are several files in conflict right now. I'll give this priority this week, so I'll try to test this ASAP.
Comment 27 Jonathan Druart 2015-08-04 14:22:43 UTC Comment hidden (obsolete)
Comment 28 Jonathan Druart 2015-08-04 14:22:53 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2015-08-04 14:22:59 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2015-08-04 14:23:14 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2015-08-04 14:23:20 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2015-08-04 14:23:27 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2015-08-04 14:23:37 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2015-08-04 14:40:35 UTC
Does not work:
circulation.pl: Template process failed: undef error - Not a GLOB reference at /usr/lib/i386-linux-gnu/perl5/5.20/Template/Provider.pm line 618.
Comment 35 Jonathan Druart 2015-08-04 14:53:40 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2015-08-04 14:58:46 UTC
Found something else, the checkout table is broken on circulation.pl (JS errors).
Comment 37 Jonathan Druart 2015-08-04 15:12:03 UTC Comment hidden (obsolete)
Comment 38 Jonathan Druart 2015-08-04 15:12:10 UTC Comment hidden (obsolete)
Comment 39 Liz Rea 2015-08-20 21:38:29 UTC
This patch seems to break sysprefs such as intranetnav and opacmainuserblock - they show their HTML instead of interpreting it. I haven't checked all of the user configurable areas with this patch but we definitely should check them all.

Off the top of my head (ok, ok, I went and looked through, but I still might have missed some): 

opacmainuserblock
opacnav
opacnavright
opaccredits
opacheader
opacuserjs
opacusercss
opaccustomsearch
opacmysummaryhtml
opacmysummarynote
opacnavbottom
opacnoresultsfound
opacresultssidebar
opacsearchfortitlein
restrictedpagecontent
PatronSelfRegistrationAdditionalInstructions 

intranetmainuserblock
intranetnav
intranetslipprinterjs
intranetusercss
intranetuserjs
Comment 40 Liz Rea 2015-08-20 21:43:05 UTC
Ok, sorry I didn't have the last couple of patches - that's my bad but we still should check all of those sysprefs as part of testing. :)
Comment 41 Liz Rea 2015-08-20 21:43:15 UTC
Ok, sorry I didn't have the last couple of patches - that's my bad but we still should check all of those sysprefs as part of testing. :)
Comment 42 Liz Rea 2015-08-20 21:50:51 UTC
Actually, I'm sure I have all of the patches now, and intranetNav, at least, still has a problem.

Also the news items are displaying their HTML, and they shouldn't be.

LR
Comment 43 HB-NEKLS 2015-08-20 21:52:19 UTC
If it's any sys pref that deals with HTML/JS/CSS, here's a few more to check; what about all the notices/slips templates? Do those need to be checked, too, since they also use HTML/JS/CSS? 

Extra user-edited sys prefs: 
OpacSuppressionMessage
SCOUserCSS
SCOUserJS
SelfCheckHelpMessage
NoLoginInstructions
Comment 44 Liz Rea 2015-08-20 22:55:09 UTC Comment hidden (obsolete)
Comment 45 Liz Rea 2015-08-20 23:04:43 UTC
On memberentrygen.tt, all of the cases for *popup are broken, they show the html we're sending in that variable instead of the dropdown. Most of them are conditional, the main reason I noticed was because of the Patron title: field.

Suspect there will be heaps of stuff like this.
Comment 46 Liz Rea 2015-08-20 23:09:25 UTC Comment hidden (obsolete)
Comment 47 Jonathan Druart 2015-08-21 11:05:51 UTC Comment hidden (obsolete)
Comment 48 Jonathan Druart 2015-08-21 11:06:13 UTC
Thanks for the pref list!
Comment 49 Jonathan Druart 2015-08-27 08:06:00 UTC
See https://github.com/joubu/Koha/tree/bug_13618 for an up-to-date patch set.
Comment 50 Jonathan Druart 2015-08-27 08:17:36 UTC Comment hidden (obsolete)
Comment 51 j.kylmala 2015-08-28 09:54:39 UTC Comment hidden (obsolete)
Comment 52 j.kylmala 2015-08-28 11:31:43 UTC
(In reply to Joonas Kylmälä from comment #51)
> Created attachment 42054 [details] [review] [review]
> Bug 13618: Use Template::Stash::AutoEscaping to use the html filter
> 
> Test plan done and worked. I deleted the commits from 13609, and then
> applied this and it also solved the xss vulnerability.
> 
> Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Thought it worked, but now noticed it broked the html in /cgi-bin/koha/opac-tags.pl?mine=1 (my tags in opac). Under the title column it only shows html as plain text.
Comment 53 Jonathan Druart 2015-08-28 12:01:48 UTC Comment hidden (obsolete)
Comment 54 Jonathan Druart 2015-08-28 12:02:02 UTC Comment hidden (obsolete)
Comment 55 Jonathan Druart 2015-08-28 12:03:18 UTC
(In reply to Joonas Kylmälä from comment #52)
> (In reply to Joonas Kylmälä from comment #51)
> > Created attachment 42054 [details] [review] [review] [review]
> > Bug 13618: Use Template::Stash::AutoEscaping to use the html filter
> > 
> > Test plan done and worked. I deleted the commits from 13609, and then
> > applied this and it also solved the xss vulnerability.
> > 
> > Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>
> 
> Thought it worked, but now noticed it broked the html in
> /cgi-bin/koha/opac-tags.pl?mine=1 (my tags in opac). Under the title column
> it only shows html as plain text.

"Bug 13618: Specific for other prefs" reuploaded, OPACMySummaryNote was not correctly managed.

New patch "Specific for XSLTBloc" should fix your issue
Comment 56 j.kylmala 2015-08-28 12:50:01 UTC Comment hidden (obsolete)
Comment 57 j.kylmala 2015-08-28 13:11:37 UTC Comment hidden (obsolete)
Comment 58 Jonathan Druart 2015-08-28 13:47:52 UTC Comment hidden (obsolete)
Comment 59 j.kylmala 2015-08-28 15:24:41 UTC Comment hidden (obsolete)
Comment 60 j.kylmala 2015-08-29 07:34:51 UTC Comment hidden (obsolete)
Comment 61 j.kylmala 2015-08-29 07:36:07 UTC Comment hidden (obsolete)
Comment 62 j.kylmala 2015-08-29 07:37:07 UTC Comment hidden (obsolete)
Comment 63 j.kylmala 2015-08-29 08:04:10 UTC
(In reply to Jonathan Druart from comment #35)
> Created attachment 41353 [details] [review] [review]
> Bug 13618: Fix error 'Not a GLOB reference'
> 
> The interpolation of a variable on including a file caused an unexpected
> error:
> Template process failed: undef error - Not a GLOB reference at
> /usr/lib/i386-linux-gnu/perl5/5.20/Template/Provider.pm line 619.
> 
> The easier fix is to replace it with a SWITCH.

Did you considered adding .raw in the end of the Koha.Preference( 'AddressFormat' ) ? It would be more generalized code and it works.
Comment 64 j.kylmala 2015-08-29 08:09:06 UTC Comment hidden (obsolete)
Comment 65 j.kylmala 2015-08-29 08:20:51 UTC Comment hidden (obsolete)
Comment 66 j.kylmala 2015-08-29 08:24:14 UTC Comment hidden (obsolete)
Comment 67 j.kylmala 2015-08-29 08:37:59 UTC Comment hidden (obsolete)
Comment 68 j.kylmala 2015-08-29 09:12:10 UTC
(In reply to Jonathan Druart from comment #28)
> Created attachment 41347 [details] [review] [review]
> Bug 13618: Remove html filters at the OPAC
> 
> This patch removes the html filters at the OPAC, if necessary.
> 
> Generated with:
>   perl -p -i -e 's/\ ?\|\ ?html(\ ?)%/\1%/g' **/*.tt **/*.inc

@@ -85,7 +85,7 @@
                                 [% END %]
                             ).
                         [% END %]
-                        <a href="[% OPACBaseURL %]/cgi-bin/koha/opac-search.pl?[% query_cgi | url %][% limit_cgi |html | url %]&amp;count=[% countrss |html %]&amp;sort_by=acqdate_dsc&amp;forma~
+                        <a href="[% OPACBaseURL %]/cgi-bin/koha/opac-search.pl?[% query_cgi | url %][% limit_cgi |html | url %]&amp;count=[% countrss %]&amp;sort_by=acqdate_dsc&amp;format=rss2~
                     [% END # / IF total %]
                     </p>
                 [% END # / IF searchdesc %]

Shouldn't that one also be removed?
Comment 69 j.kylmala 2015-08-29 09:19:34 UTC
(In reply to Joonas Kylmälä from comment #68)
> (In reply to Jonathan Druart from comment #28)
> > Created attachment 41347 [details] [review] [review] [review]
> > Bug 13618: Remove html filters at the OPAC
> > 
> > This patch removes the html filters at the OPAC, if necessary.
> > 
> > Generated with:
> >   perl -p -i -e 's/\ ?\|\ ?html(\ ?)%/\1%/g' **/*.tt **/*.inc
> 
> @@ -85,7 +85,7 @@
>                                  [% END %]
>                              ).
>                          [% END %]
> -                        <a href="[% OPACBaseURL
> %]/cgi-bin/koha/opac-search.pl?[% query_cgi | url %][% limit_cgi |html | url
> %]&amp;count=[% countrss |html %]&amp;sort_by=acqdate_dsc&amp;forma~
> +                        <a href="[% OPACBaseURL
> %]/cgi-bin/koha/opac-search.pl?[% query_cgi | url %][% limit_cgi |html | url
> %]&amp;count=[% countrss %]&amp;sort_by=acqdate_dsc&amp;format=rss2~
>                      [% END # / IF total %]
>                      </p>
>                  [% END # / IF searchdesc %]
> 
> Shouldn't that one also be removed?

Also in this patch there was two more of these so they should be maybe removed also if this one is. I used regex ^\+.+\|html to find these
Comment 70 j.kylmala 2015-08-29 09:22:16 UTC Comment hidden (obsolete)
Comment 71 j.kylmala 2015-08-29 09:28:37 UTC
(In reply to Jonathan Druart from comment #37)
> Created attachment 41354 [details] [review] [review]
> Bug 13618: Specific for IntranetUser* and OPACUser* prefs

--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/sco/sco-main.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/sco/sco-main.tt
@@ -8,7 +8,7 @@
 <link rel="stylesheet" type="text/css" href="[% interface %]/[% theme %]/lib/bootstrap/css/bootstrap.min.css" />
 <link rel="stylesheet" type="text/css" href="[% interface %]/[% theme %]/lib/jquery/jquery-ui.css" />
 <link rel="stylesheet" type="text/css" href="[% interface %]/[% theme %]/css/sco.css" />
-[% IF ( OPACUserCSS ) %]<style type="text/css">[% OPACUserCSS %]</style>[% END %]
+[% IF ( OPACUserCSS ) %]<style type="text/css">[% OPACUserCSS.raw %]</style>[% END %]
 [% IF ( SCOUserCSS ) %]<style type="text/css">[% SCOUserCSS %]</style>[% END %]

Why not have also raw SCOUserCSS if we let the user have OPACUserCSS as raw?
Comment 72 j.kylmala 2015-08-29 10:25:24 UTC
The front page in Staff side doesn't render html under News->What's Next.
Comment 73 Jonathan Druart 2015-09-01 07:57:30 UTC
(In reply to Joonas Kylmälä from comment #68)
> (In reply to Jonathan Druart from comment #28)
> > Created attachment 41347 [details] [review] [review] [review]
> > Bug 13618: Remove html filters at the OPAC
> > 
> > This patch removes the html filters at the OPAC, if necessary.
> > 
> > Generated with:
> >   perl -p -i -e 's/\ ?\|\ ?html(\ ?)%/\1%/g' **/*.tt **/*.inc
> 
> @@ -85,7 +85,7 @@
>                                  [% END %]
>                              ).
>                          [% END %]
> -                        <a href="[% OPACBaseURL
> %]/cgi-bin/koha/opac-search.pl?[% query_cgi | url %][% limit_cgi |html | url
> %]&amp;count=[% countrss |html %]&amp;sort_by=acqdate_dsc&amp;forma~
> +                        <a href="[% OPACBaseURL
> %]/cgi-bin/koha/opac-search.pl?[% query_cgi | url %][% limit_cgi |html | url
> %]&amp;count=[% countrss %]&amp;sort_by=acqdate_dsc&amp;format=rss2~
>                      [% END # / IF total %]
>                      </p>
>                  [% END # / IF searchdesc %]
> 
> Shouldn't that one also be removed?

I don't think so, I think I have tested them.

(In reply to Joonas Kylmälä from comment #71)
> Why not have also raw SCOUserCSS if we let the user have OPACUserCSS as raw?

Done in another patch.

(In reply to Joonas Kylmälä from comment #72)
> The front page in Staff side doesn't render html under News->What's Next.

New patch pushed to the remote branch.
Comment 74 j.kylmala 2015-10-13 09:58:28 UTC
Jonathan, The commits in GitHub branch look good.
Comment 75 Jonathan Druart 2015-10-13 10:30:33 UTC
(In reply to Joonas Kylmälä from comment #74)
> Jonathan, The commits in GitHub branch look good.

I have added you signed-off-by line on the 2 last patch.
Feel free to mark the bug report as Signed off if you feel confident enough.
Comment 76 Jonathan Druart 2015-10-22 08:08:03 UTC
Remote branch rebased.
Comment 77 Bernardo Gonzalez Kriegel 2015-11-17 18:09:51 UTC
(In reply to Jonathan Druart from comment #76)
> Remote branch rebased.

Needs a new rebase, there are some conflicts
Comment 78 Jonathan Druart 2015-11-25 09:37:47 UTC
Remote branch rebased.
Comment 79 Bernardo Gonzalez Kriegel 2015-11-25 14:02:25 UTC
Hi Jonathan,
all patches apply, last patch signeed.
I have a followup to remove some tabs/spaces

Do I have to upload all here?

Also found some "| html | html_line_break " and "| html | url "
Don't know if they need to be removed

git grep "|html "
koha-tmpl/intranet-tmpl/prog/en/modules/members/tables/members_results.tt:20:                    "[% data.borrowernotes.replace('\\\\' , '\\\\') |html |html_line_break |collapse %]",
koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/tables/members_results.tt:29:                    "[% data.borrowernotes.replace('\\\\' , '\\\\') |html |html_line_break |collapse %]",
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt:44:                                No results found for that in [% LibraryName %] catalog. <a href="[% OPACBaseURL %]/cgi-bin/koha/opac-search.pl?[% query_cgi | url %][% limit_cgi | html | url %]&amp;format=rss2" class="rsssearchlink noprint"><img src="[% interface %]/[% theme %]/images/feed-icon-16x16.png" alt="Subscribe to this search" title="Subscribe to this search" border="0" class="rsssearchicon"/></a>
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-search-history.tt:88:                        <a href="[% OPACBaseURL %]/cgi-bin/koha/opac-search.pl?[% query_cgi | url %][% limit_cgi |html | url %]&amp;count=[% countrss %]&amp;sort_by=acqdate_dsc&amp;format=rss2" class="rsssearchlink noprint"><img src="[% interface %]/[% theme %]/images/feed-icon-16x16.png" alt="Subscribe to this search" title="Subscribe to this search" class="rsssearchicon"/></a>
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-search-history.tt:71:                                                        <td><a href="[% OPACBaseURL %]/cgi-bin/koha/opac-search.pl?[% query_cgi |html |url %][% limit_cgi |html | url %]&amp;[% s.query_cgi %]&amp;count=[% countrss %]&amp;sort_by=acqdate_dsc&amp;format=rss2" class="rsssearchlink"><img src="[% interface %]/[% theme %]/images/feed-icon-16x16.png" alt="Subscribe to this search" title="Subscribe to this search" class="rsssearchicon"/></a> <a href="/cgi-bin/koha/opac-search.pl?[% s.query_cgi %]">[% s.query_desc %]</a></td>

git grep "| html "
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt:111:                                                    <td><a href="[% OPACBaseURL %]/cgi-bin/koha/opac-search.pl?[% query_cgi |html |url %][% limit_cgi |html | url %]&amp;[% s.query_cgi %]&amp;count=[% countrss %]&amp;sort_by=acqdate_dsc&amp;format=rss2" class="rsssearchlink"><img src="[% interface %]/[% theme %]/images/feed-icon-16x16.png" alt="Subscribe to this search" title="Subscribe to this search" class="rsssearchicon"/></a> <a href="/cgi-bin/koha/opac-search.pl?[% s.query_cgi %]">[% s.query_desc %]</a></td>
Comment 80 Jonathan Druart 2015-11-25 14:49:55 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #79)
> Hi Jonathan,
> all patches apply, last patch signeed.
> I have a followup to remove some tabs/spaces

Where?

> Do I have to upload all here?

No, I will add your signoffs on my branch.

> Also found some "| html | html_line_break " and "| html | url "
> Don't know if they need to be removed

I don't think they should.
Comment 81 Bernardo Gonzalez Kriegel 2015-11-25 15:01:23 UTC Comment hidden (obsolete)
Comment 82 Bernardo Gonzalez Kriegel 2015-11-25 15:02:35 UTC
(In reply to Jonathan Druart from comment #80)
> (In reply to Bernardo Gonzalez Kriegel from comment #79)
> > Hi Jonathan,
> > all patches apply, last patch signeed.
> > I have a followup to remove some tabs/spaces
> 
> Where?
> 
I attached a patch with fixes needed by koha-qa
Comment 83 Jonathan Druart 2015-11-25 17:18:32 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #82)
> (In reply to Jonathan Druart from comment #80)
> > (In reply to Bernardo Gonzalez Kriegel from comment #79)
> > > Hi Jonathan,
> > > all patches apply, last patch signeed.
> > > I have a followup to remove some tabs/spaces
> > 
> > Where?
> > 
> I attached a patch with fixes needed by koha-qa

Thanks, added to the remote branch.
Feel free to switch the status to Signed Off :)
Comment 84 Bernardo Gonzalez Kriegel 2015-11-25 18:42:23 UTC
Checked opac and staff pages (not all), no problems found
Comment 85 Bernardo Gonzalez Kriegel 2015-11-28 14:15:34 UTC
Mmm, some problems editing records and items
There is a strange display on edit/add

On items it can be fixed with something like
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt
@@ -225,9 +225,9 @@ function confirm_deletion() {
         [% FOREACH ite IN item %]
                <li><div class="subfield_line" style="[% ite.visibility %]" id="subfield[% ite.tag %][% ite.subfield %][% ite
                 [% IF ( ite.mandatory ) %]
-               <label class="required">[% ite.subfield %] - [% ite.marc_lib %]</label>
+               <label class="required">[% ite.subfield %] - [% ite.marc_lib.raw %]</label>
                [% ELSE %]
-               <label>[% ite.subfield %] - [% ite.marc_lib %]</label>
+               <label>[% ite.subfield %] - [% ite.marc_lib.raw %]</label>
                [% END %]

                 [% SET mv = ite.marc_value %]
@@ -258,7 +258,7 @@ function confirm_deletion() {
                     [% ELSE  %]
                         <a href="#" id="buttonDot_[%- mv.id -%]" class="[%- mv.class -%]" title="Tag editor">...</a>
                     [% END %]
-                    [%- mv.javascript -%]
+                    [%- mv.javascript.raw -%]
                 [% ELSIF ( mv.type == 'text' ) %]
                     <input type="text" id="[%- mv.id -%]" name="field_value" class="input_marceditor" size="50" maxlength="[
                 [% ELSIF ( mv.type == 'textarea' ) %]

But there are some javascript errors in the console.
And we need a fix for record editor.
Comment 86 Jonathan Druart 2015-12-01 13:04:40 UTC
Good catch Bernardo.
Fixed.

  commit de101ae8b8e04eeb04abe83cbb69cc02bfa87eff
  Bug 13618: Fix for edit biblios and items    
    On editing biblios or items, the marc_lib, marc_value and javascript
    values are often populated with html code which needs to be displayed
    raw.
Comment 87 Jonathan Druart 2015-12-08 10:56:46 UTC
Please guys, we need a quick push of this one.
Comment 88 Marc Véron 2015-12-17 08:53:19 UTC
Wanted to test, got:

Applying: Bug 13618: Remove html filters at the OPAC
fatal: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-authoritiessearchresultlist.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Comment 89 Jonathan Druart 2015-12-17 10:24:58 UTC
Please use the remote branch.
Comment 90 Bernardo Gonzalez Kriegel 2015-12-17 12:13:16 UTC
(In reply to Jonathan Druart from comment #89)
> Please use the remote branch.

Hi, there is a easy conflict on koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc :)

Record and item editor now look good, 
but on item editor there is a 'Uncought SyntaxError: Unxepected token &', seems to come from

var columns_settings = [{&quot;columnname&quot;:&quot;barcode&quot;},
{&quot;columnname&quot;:&quot;dateaccessioned&quot;},
...
{&quot;columnname&quot;:&quot;stocknumber&quot;}]

No errors on record editor or plugins, there is a warning on MARC 006/008
"Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check http://xhr.spec.whatwg.org/."
Comment 91 Jonathan Druart 2015-12-17 12:24:34 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #90)
> (In reply to Jonathan Druart from comment #89)
> > Please use the remote branch.
> 
> Hi, there is a easy conflict on
> koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc :)

Rebased.

> Record and item editor now look good, 
> but on item editor there is a 'Uncought SyntaxError: Unxepected token &',
> seems to come from
> 
> var columns_settings = [{&quot;columnname&quot;:&quot;barcode&quot;},
> {&quot;columnname&quot;:&quot;dateaccessioned&quot;},
> ...
> {&quot;columnname&quot;:&quot;stocknumber&quot;}]

Introduced by a new patch, fixed in a new commit "Bug 13618: (follow-up) Specific for ColumnsSettings"

> No errors on record editor or plugins, there is a warning on MARC 006/008
> "Synchronous XMLHttpRequest on the main thread is deprecated because of its
> detrimental effects to the end user's experience. For more help, check
> http://xhr.spec.whatwg.org/."

Could you confirm you don't have it on master?
Comment 92 Bernardo Gonzalez Kriegel 2015-12-17 12:34:11 UTC
(In reply to Jonathan Druart from comment #91)
> 
> Rebased.

Ok!

> 
> Introduced by a new patch, fixed in a new commit "Bug 13618: (follow-up)
> Specific for ColumnsSettings"

No more error

> Could you confirm you don't have it on master?

It's on master, no a problem of this patchset.

No koha-qa errors.
Changed to Signed-Off
Comment 93 Jonathan Druart 2015-12-17 13:01:32 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #92)
> (In reply to Jonathan Druart from comment #91)
> Changed to Signed-Off

Thanks Bernardo!
I have added your signed-off-by line to the last 2 patches.
Comment 94 Liz Rea 2015-12-17 21:28:45 UTC
Created attachment 45784 [details]
Screenshot opac-shelves.pl

on /cgi-bin/koha/opac-shelves.pl the page returned's styling/layout goes funny with this patch set. Please see attached screenshot.

Liz
Comment 95 Liz Rea 2015-12-17 21:37:02 UTC
Created attachment 45785 [details]
List pages in OPAC also broken when the list contains items
Comment 96 Liz Rea 2015-12-17 21:38:29 UTC
Actually, please look over the lists functions in the OPAC generally, as they really seem worse for the wear after this patch.
Comment 97 Bernardo Gonzalez Kriegel 2015-12-17 22:51:42 UTC Comment hidden (obsolete)
Comment 98 Bernardo Gonzalez Kriegel 2015-12-17 22:52:38 UTC
(In reply to Liz Rea from comment #96)
> Actually, please look over the lists functions in the OPAC generally, as
> they really seem worse for the wear after this patch.

Could you test with last patch on top?
Comment 99 Jonathan Druart 2015-12-18 09:27:27 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #98)
> (In reply to Liz Rea from comment #96)
> > Actually, please look over the lists functions in the OPAC generally, as
> > they really seem worse for the wear after this patch.
> 
> Could you test with last patch on top?

Thanks Bernardo, it fixes the issue.
I have signed off the patch and pushed it to the branch.

It comes from a merge conflict (on Bug 13618: Remove html filters at the OPAC) which I have not fixed correctly in one of the many rebases.
Comment 100 Jonathan Druart 2016-01-04 11:39:42 UTC
Branches rebased against master.
Comment 101 Jonathan Druart 2016-01-04 11:40:04 UTC
Branch rebased against master.
Comment 102 Katrin Fischer 2016-01-06 19:47:03 UTC
Hi Jonathan,

as promised, started testing this. I checked out your branch, without rebasing it to current master.

I had to install 2 dependencies via cpanm:
  sudo cpanm Template::Stash::HTML::Entities
  sudo cpanm Template::Stash::AutoEscaping

We need to make sure those can be packaged!

Some glitches I found:
Prio 1:
 - HTML printer slips are broken
   To test: Patron account > print quick slip
Others:
 - Patron account in staff > notices tab > HTML notices
   To test: use an HTML version of a check-in or check-out notice
 - System preferences containing HTML tags display them
   To test: search for 'note' in system preferences
 - Staff > detail page > MARC view > Items tab > &nbsp; displays in URL column
   To test: Add a 952$u to one of multiple items

Some I am not totally sure about, but there is a change in behaviour to before:
 - HTML tags in restriction/debarment comments
 - HTML tags in messages in the patron account ('new message' link)

Stopping my testing for now.
Comment 103 Galen Charlton 2016-01-06 20:09:00 UTC
Not seeing any immediate bar to attempting to package the new deps, but I should emphasize that new dependencies means that this is not backportable as far as the packages are concerned.  To emphasize this point, I've recategorized this bug as an enhancement, as it's more about implementing a global solution rather than fixing a bunch of individual XSS issues.
Comment 104 Jonathan Druart 2016-01-07 09:45:29 UTC
(In reply to Katrin Fischer from comment #102)

Everything has been fixed in the following new commits:

commit 13faef4d28df0a70c2c89ab4317ebc2882fc3704
    Bug 13618: Fix for system preference description    
        If a syspref description contains html tag, do not display them

commit 2879a9976549f84c9520d5778a45d8348845d7b4
    Bug 13618: Do not display &nbsp; and html tags in item fields content
        Note that there might be other occurrences to fix!

commit d8547698a47e83cdb7d321391cadddd4f26e5f85
    Bug 13618: Do not display html tags in patron's notices

commit 58e62b53eda1110003d6c0b0cebc983c62bf2f46
    Bug 13618: Fix for debarredcomment and patron messages    
        At the OPAC and intranet.

commit 0aefdddb394db1fab7287970ab0b80f3a92479e8
    Bug 13618: (follow-up) Specific for other prefs    
        follow-up for SlipCSS

Remote branch updated.
Comment 105 Jonathan Druart 2016-01-07 09:46:02 UTC
Katrin, feel free to ask to new signoffs on these patches if you think it's needed.
Comment 106 Katrin Fischer 2016-01-11 21:12:36 UTC
Hi Jonathan,

I (re)testd some more:

NOW OK:
 - Patron account in staff > notices tab > HTML notices
 - System preferences containing HTML tags display them
   To test: search for 'note' in system preferences
 - Staff > detail page > MARC view > Items tab > &nbsp; displays in URL column
   To test: Add a 952$u to one of multiple items
 - HTML tags in messages in the patron account ('new message' link)

NOT OK:
 - HTML printer slips are broken
   To test: Patron account > print quick slip 
   This shows still the same problem for and I don't see a patch with a 
   matching description on the branch?
 - HTML tags in restriction/debarment comments
   Works partially - the restriction message on top of the patron account is ok.
   The entries in the little restrictions table inside the tab still appear
   broken - on the details and checkout tabs and also when editing a patron.
   

Other test results:
- OPAC is looking good, nothing found
- Staff is looking good as well - no new findings. :)
Comment 107 Katrin Fischer 2016-01-11 23:26:46 UTC
One more thing - I had to install 2 dependencies to get it to work. - Should both go into the PerlDependencies.pm file?
Comment 108 Jonathan Druart 2016-01-12 15:52:17 UTC
Last 2 patches updated on the remote branch.
Comment 109 Jonathan Druart 2016-01-12 16:31:05 UTC
(In reply to Katrin Fischer from comment #102)
> I had to install 2 dependencies via cpanm:
>   sudo cpanm Template::Stash::HTML::Entities
>   sudo cpanm Template::Stash::AutoEscaping

Are you sure about that? I have removed Template::Stash::HTML::Entities and everything looks fine.
Comment 110 Katrin Fischer 2016-01-12 16:35:33 UTC
But comment#3 told me to install it! Maybe it's not needed then :) I can retest this as well.
Comment 111 Katrin Fischer 2016-01-18 07:24:37 UTC
Hi Jonathan, working on this again - can you please update your branch to master once more?
Comment 112 Katrin Fischer 2016-01-18 07:29:05 UTC
Retesting the issues found last time:
 - HTML printer slips work perfectly now
 - HTML tags in restriction/debarment comments show

... and while you are on it, could you please add my sign off lines as well? :)

I am going to obsolete the patches here for now, to avoid confusion.
Comment 113 Katrin Fischer 2016-01-18 07:29:59 UTC
Works partially - the restriction message on top of the patron account is ok.
   The entries in the little restrictions table inside the tab still appear
   broken - on the details and checkout tabs and also when editing a patron.variables - intra
Comment 114 Jonathan Druart 2016-01-18 11:18:51 UTC Comment hidden (obsolete)
Comment 115 Jonathan Druart 2016-01-18 11:18:57 UTC Comment hidden (obsolete)
Comment 116 Jonathan Druart 2016-01-18 11:19:02 UTC Comment hidden (obsolete)
Comment 117 Jonathan Druart 2016-01-18 11:19:08 UTC Comment hidden (obsolete)
Comment 118 Jonathan Druart 2016-01-18 11:19:13 UTC Comment hidden (obsolete)
Comment 119 Jonathan Druart 2016-01-18 11:19:17 UTC Comment hidden (obsolete)
Comment 120 Jonathan Druart 2016-01-18 11:19:21 UTC Comment hidden (obsolete)
Comment 121 Jonathan Druart 2016-01-18 11:19:26 UTC Comment hidden (obsolete)
Comment 122 Jonathan Druart 2016-01-18 11:19:32 UTC Comment hidden (obsolete)
Comment 123 Jonathan Druart 2016-01-18 11:19:36 UTC Comment hidden (obsolete)
Comment 124 Jonathan Druart 2016-01-18 11:19:41 UTC Comment hidden (obsolete)
Comment 125 Jonathan Druart 2016-01-18 11:19:45 UTC Comment hidden (obsolete)
Comment 126 Jonathan Druart 2016-01-18 11:19:50 UTC Comment hidden (obsolete)
Comment 127 Jonathan Druart 2016-01-18 11:19:54 UTC Comment hidden (obsolete)
Comment 128 Jonathan Druart 2016-01-18 11:19:58 UTC Comment hidden (obsolete)
Comment 129 Jonathan Druart 2016-01-18 11:20:03 UTC Comment hidden (obsolete)
Comment 130 Jonathan Druart 2016-01-18 11:20:07 UTC Comment hidden (obsolete)
Comment 131 Jonathan Druart 2016-01-18 11:20:12 UTC Comment hidden (obsolete)
Comment 132 Jonathan Druart 2016-01-18 11:20:17 UTC Comment hidden (obsolete)
Comment 133 Jonathan Druart 2016-01-18 11:20:22 UTC Comment hidden (obsolete)
Comment 134 Jonathan Druart 2016-01-18 11:20:26 UTC Comment hidden (obsolete)
Comment 135 Jonathan Druart 2016-01-18 11:20:32 UTC Comment hidden (obsolete)
Comment 136 Jonathan Druart 2016-01-18 11:20:37 UTC Comment hidden (obsolete)
Comment 137 Jonathan Druart 2016-01-18 11:20:42 UTC Comment hidden (obsolete)
Comment 138 Jonathan Druart 2016-01-18 11:20:47 UTC Comment hidden (obsolete)
Comment 139 Jonathan Druart 2016-01-18 11:20:51 UTC Comment hidden (obsolete)
Comment 140 Jonathan Druart 2016-01-18 11:20:57 UTC Comment hidden (obsolete)
Comment 141 Jonathan Druart 2016-01-18 11:21:02 UTC Comment hidden (obsolete)
Comment 142 Jonathan Druart 2016-01-18 11:23:15 UTC
(In reply to Katrin Fischer from comment #112)
> Retesting the issues found last time:
>  - HTML printer slips work perfectly now
>  - HTML tags in restriction/debarment comments show
> 
> ... and while you are on it, could you please add my sign off lines as well?
> :)
> 
> I am going to obsolete the patches here for now, to avoid confusion.

I have added your signed-off-by line, attached the patches from the remote branch on this bug report and attached a new patch (found a bug when rebasing).
Comment 143 Jonathan Druart 2016-01-18 11:44:36 UTC Comment hidden (obsolete)
Comment 144 Jonathan Druart 2016-01-18 11:44:41 UTC Comment hidden (obsolete)
Comment 145 Jonathan Druart 2016-01-18 11:44:45 UTC Comment hidden (obsolete)
Comment 146 Jonathan Druart 2016-01-18 11:44:51 UTC Comment hidden (obsolete)
Comment 147 Jonathan Druart 2016-01-18 11:44:55 UTC Comment hidden (obsolete)
Comment 148 Jonathan Druart 2016-01-18 11:44:59 UTC Comment hidden (obsolete)
Comment 149 Jonathan Druart 2016-01-18 11:45:04 UTC Comment hidden (obsolete)
Comment 150 Jonathan Druart 2016-01-18 11:45:08 UTC Comment hidden (obsolete)
Comment 151 Jonathan Druart 2016-01-18 11:45:13 UTC Comment hidden (obsolete)
Comment 152 Jonathan Druart 2016-01-18 11:45:17 UTC Comment hidden (obsolete)
Comment 153 Jonathan Druart 2016-01-18 11:45:22 UTC Comment hidden (obsolete)
Comment 154 Jonathan Druart 2016-01-18 11:45:26 UTC Comment hidden (obsolete)
Comment 155 Jonathan Druart 2016-01-18 11:45:30 UTC Comment hidden (obsolete)
Comment 156 Jonathan Druart 2016-01-18 11:45:35 UTC Comment hidden (obsolete)
Comment 157 Jonathan Druart 2016-01-18 11:45:39 UTC Comment hidden (obsolete)
Comment 158 Jonathan Druart 2016-01-18 11:45:44 UTC Comment hidden (obsolete)
Comment 159 Jonathan Druart 2016-01-18 11:45:48 UTC Comment hidden (obsolete)
Comment 160 Jonathan Druart 2016-01-18 11:45:53 UTC Comment hidden (obsolete)
Comment 161 Jonathan Druart 2016-01-18 11:45:58 UTC Comment hidden (obsolete)
Comment 162 Jonathan Druart 2016-01-18 11:46:02 UTC Comment hidden (obsolete)
Comment 163 Jonathan Druart 2016-01-18 11:46:07 UTC Comment hidden (obsolete)
Comment 164 Jonathan Druart 2016-01-18 11:46:11 UTC Comment hidden (obsolete)
Comment 165 Jonathan Druart 2016-01-18 11:46:15 UTC Comment hidden (obsolete)
Comment 166 Jonathan Druart 2016-01-18 11:46:20 UTC Comment hidden (obsolete)
Comment 167 Jonathan Druart 2016-01-18 11:46:24 UTC Comment hidden (obsolete)
Comment 168 Jonathan Druart 2016-01-18 11:46:28 UTC Comment hidden (obsolete)
Comment 169 Jonathan Druart 2016-01-18 11:46:33 UTC Comment hidden (obsolete)
Comment 170 Jonathan Druart 2016-01-18 11:46:37 UTC Comment hidden (obsolete)
Comment 171 Kyle M Hall 2016-01-28 16:19:58 UTC
Jonathan, can you please rebase these patches?

Thanks!
Comment 172 Jonathan Druart 2016-01-29 09:53:44 UTC
Created attachment 47425 [details] [review]
Bug 13618: Use Template::Stash::AutoEscaping to use the html filter

Test plan:
0/ sudo cpanm Template::Stash::AutoEscaping
1/ Verify don't reproduce the XSS issue described on bug 13609 and other
xss related bugs.
2/ Try to find some encoding issues (detail page, search results,
facets, etc.)

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 173 Jonathan Druart 2016-01-29 09:53:50 UTC
Created attachment 47426 [details] [review]
Bug 13618: Remove html filters at the OPAC

This patch removes the html filters at the OPAC, if necessary.

Generated with:
  perl -p -i -e 's/\ ?\|\ ?html(\ ?)%/\1%/g' **/*.tt **/*.inc

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 174 Jonathan Druart 2016-01-29 09:53:55 UTC
Created attachment 47427 [details] [review]
Bug 13618: Specific places where we don't need to escape variables

There is no need to escape the html generated by the XSLT.

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 175 Jonathan Druart 2016-01-29 09:54:02 UTC
Created attachment 47428 [details] [review]
Bug 13618: Remove html filters at the intranet

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 176 Jonathan Druart 2016-01-29 09:54:07 UTC
Created attachment 47429 [details] [review]
Bug 13618: Specific places where we don't need to escape variables - intra

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 177 Jonathan Druart 2016-01-29 09:54:12 UTC
Created attachment 47430 [details] [review]
Bug 13618: Specific for pagination_bar

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 178 Jonathan Druart 2016-01-29 09:54:18 UTC
Created attachment 47431 [details] [review]
Bug 13618: Specific for the ISBD view

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 179 Jonathan Druart 2016-01-29 09:54:23 UTC
Created attachment 47432 [details] [review]
Bug 13618: Fix error 'Not a GLOB reference'

The interpolation of a variable on including a file caused an unexpected
error:
Template process failed: undef error - Not a GLOB reference at
/usr/lib/i386-linux-gnu/perl5/5.20/Template/Provider.pm line 619.

The easier fix is to replace it with a SWITCH.

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 180 Jonathan Druart 2016-01-29 09:54:28 UTC
Created attachment 47433 [details] [review]
Bug 13618: Specific for IntranetUser* and OPACUser* prefs

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 181 Jonathan Druart 2016-01-29 09:54:33 UTC
Created attachment 47434 [details] [review]
Bug 13618: Specific for ColumnsSettings

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 182 Jonathan Druart 2016-01-29 09:54:39 UTC
Created attachment 47435 [details] [review]
Bug 13618 - memberentrygen.tt errors Not a GLOB reference

Like Jonathan said:
The interpolation of a variable on including a file caused an unexpected
error:
Template process failed: undef error - Not a GLOB reference at
/usr/lib/i386-linux-gnu/perl5/5.20/Template/Provider.pm line 619.

Replaced it with a SWITCH, like the other patch for this similar error.

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 183 Jonathan Druart 2016-01-29 09:54:44 UTC
Created attachment 47436 [details] [review]
Bug 13618: Specific for other prefs

opacmainuserblock
opacnav
opacnavright
opaccredits
opacheader
opaccustomsearch
opacmysummaryhtml
opacmysummarynote
opacnavbottom
opacnoresultsfound
opacresultssidebar
opacsearchfortitlein
restrictedpagecontent
PatronSelfRegistrationAdditionalInstructions
intranetmainuserblock
intranetnav
intranetslipprinterjs
OpacSuppressionMessage
SCOUserCSS
SCOUserJS
SelfCheckHelpMessage
NoLoginInstructions

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 184 Jonathan Druart 2016-01-29 09:54:49 UTC
Created attachment 47437 [details] [review]
Bug 13618: Specific for Salutation on editing a patron

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 185 Jonathan Druart 2016-01-29 09:54:55 UTC
Created attachment 47438 [details] [review]
Bug 13618: Specific for XSLTBloc

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 186 Jonathan Druart 2016-01-29 09:55:00 UTC
Created attachment 47439 [details] [review]
Bug 13618: Fix escape on sending baskets or shelves by email

Test plan:
Send baskets and shelves by email.
With or without this patch, you should not see any changes.

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 187 Jonathan Druart 2016-01-29 09:55:06 UTC
Created attachment 47440 [details] [review]
Bug 13618: Fix for news

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 188 Jonathan Druart 2016-01-29 09:55:11 UTC
Created attachment 47441 [details] [review]
Bug 13618: Fix last occurrences recently introduced to master

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 189 Jonathan Druart 2016-01-29 09:55:17 UTC
Created attachment 47442 [details] [review]
Bug 13618: followup to remove tabs

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
This followup on top of remote branch
Only remove tabs and trailing spaces to make koha-qa pass

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 190 Jonathan Druart 2016-01-29 09:55:22 UTC
Created attachment 47443 [details] [review]
Bug 13618: Fix for edit biblios and items

On editing biblios or items, the marc_lib, marc_value and javascript
values are often populated with html code which needs to be displayed
raw.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 191 Jonathan Druart 2016-01-29 09:55:28 UTC
Created attachment 47444 [details] [review]
Bug 13618: (follow-up) Specific for ColumnsSettings

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 192 Jonathan Druart 2016-01-29 09:55:33 UTC
Created attachment 47445 [details] [review]
Bug 13618: (follow-up) add missing lines for opac-shelves

Proposed patch to fix opac-shelves

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 193 Jonathan Druart 2016-01-29 09:55:38 UTC
Created attachment 47446 [details] [review]
Bug 13618: Remove html filters for newly pushed code

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 194 Jonathan Druart 2016-01-29 09:55:44 UTC
Created attachment 47447 [details] [review]
Bug 13618: Fix for system preference description

If a syspref description contains html tag, do not display them

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 195 Jonathan Druart 2016-01-29 09:55:49 UTC
Created attachment 47448 [details] [review]
Bug 13618: Do not display &nbsp; and html tags in item fields content

Note that there might be other occurrences to fix!

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 196 Jonathan Druart 2016-01-29 09:55:55 UTC
Created attachment 47449 [details] [review]
Bug 13618: Do not display html tags in patron's notices

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 197 Jonathan Druart 2016-01-29 09:56:03 UTC
Created attachment 47450 [details] [review]
Bug 13618: Fix for debarredcomment and patron messages

At the OPAC and intranet.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 198 Jonathan Druart 2016-01-29 09:56:09 UTC
Created attachment 47451 [details] [review]
Bug 13618: (follow-up) Specific for other prefs

follow-up for SlipCSS and printslip

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 199 Jonathan Druart 2016-01-29 09:56:14 UTC
Created attachment 47452 [details] [review]
Bug 13618: Specific for branches.opac_info
Comment 200 Jonathan Druart 2016-01-29 09:57:23 UTC
@RM: please don't squash these patches, it will be helpful to have the history if a regression if found later.
Comment 201 Brendan Gallagher 2016-01-29 17:47:50 UTC
(In reply to Jonathan Druart from comment #200)
> @RM: please don't squash these patches, it will be helpful to have the
> history if a regression if found later.

Will not squash :)
Comment 202 Brendan Gallagher 2016-01-29 18:18:44 UTC
Pushed to Master - Should be in the May 2016 Release.  Thanks!  (we'll probably have some more spots to work on this).  Great job though!
Comment 203 Marcel de Rooy 2016-02-01 08:03:45 UTC
Great work!

Detail for patch: Use Template::Stash::AutoEscaping to use the html filter

Note that this still needs attention in terms of (Debian) packaging.
At first glance this module does not seem to be available in a Debian package.
Obviously, new perl dependencies should preferably be available already.
Comment 204 Jonathan Druart 2016-02-01 11:27:51 UTC
New test added to the QA script:

commit a9ddabb936a0a1539d01992908b3e6484b6d6466
    The html filter is not needed anymore (see bug 13618)
Comment 205 Julian Maurice 2016-02-03 10:49:25 UTC
(In reply to Marcel de Rooy from comment #203)
> Note that this still needs attention in terms of (Debian) packaging.
> At first glance this module does not seem to be available in a Debian
> package.

I started packaging this module.
ITP bug: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813574
Git repo: https://anonscm.debian.org/cgit/pkg-perl/packages/libtemplate-stash-autoescaping-perl.git/
Comment 206 Marcel de Rooy 2016-02-03 12:10:31 UTC
(In reply to Julian Maurice from comment #205)
> I started packaging this module.
> ITP bug: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813574
> Git repo:
> https://anonscm.debian.org/cgit/pkg-perl/packages/libtemplate-stash-
> autoescaping-perl.git/

Great! Thx
Comment 207 Mirko Tietgen 2016-02-04 06:08:29 UTC
Created attachment 47631 [details]
libtemplate-stash-autoescaping-perl_0.0303-1_all.deb

I built a package in Jessie, maybe it is useful for others too. It's attached.
Comment 208 Julian Maurice 2016-02-08 12:07:07 UTC
Package accepted into Debian unstable: https://packages.debian.org/sid/libtemplate-stash-autoescaping-perl
Comment 209 Katrin Fischer 2016-02-08 20:07:27 UTC
Thx Julian!
Comment 210 Brendan Gallagher 2016-02-11 19:45:08 UTC
Reverted from master.  Making Status as Blocked.  Jonathan please choose the status you'd prefer for this bug.
Comment 211 Jonathan Druart 2016-03-03 11:24:00 UTC
See bug 15715 for the cause of the revert.
Comment 212 Martin Renvoize 2017-02-07 14:52:35 UTC
The approach here is correct in my opinion.. we should treat everything as unsafe and thus escape it by default unless we've manually checked it and marked as safe.

Yes, this will impact performance at first and highlight some especially nasty areas of koha. We should use those highlights as a hitlist of areas to concentrate on a) checking security and marking as safe when possible and b) refactoring templates to more sparsely use variables when possible.
Comment 213 Jonathan Druart 2017-02-07 15:49:43 UTC
Created attachment 59983 [details] [review]
Bug 13618: Use Template::Stash::AutoEscaping to use the html filter

Test plan:
0/ sudo cpanm Template::Stash::AutoEscaping
1/ Verify don't reproduce the XSS issue described on bug 13609 and other
xss related bugs.
2/ Try to find some encoding issues (detail page, search results,
facets, etc.)

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Remove html filters at the OPAC

This patch removes the html filters at the OPAC, if necessary.

Generated with:
  perl -p -i -e 's/\ ?\|\ ?html(\ ?)%/\1%/g' **/*.tt **/*.inc

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Specific places where we don't need to escape variables

There is no need to escape the html generated by the XSLT.

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Remove html filters at the intranet

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Specific places where we don't need to escape variables - intra

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Specific for pagination_bar

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Specific for the ISBD view

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Fix error 'Not a GLOB reference'

The interpolation of a variable on including a file caused an unexpected
error:
Template process failed: undef error - Not a GLOB reference at
/usr/lib/i386-linux-gnu/perl5/5.20/Template/Provider.pm line 619.

The easier fix is to replace it with a SWITCH.

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Specific for IntranetUser* and OPACUser* prefs

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Specific for ColumnsSettings

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618 - memberentrygen.tt errors Not a GLOB reference

Like Jonathan said:
The interpolation of a variable on including a file caused an unexpected
error:
Template process failed: undef error - Not a GLOB reference at
/usr/lib/i386-linux-gnu/perl5/5.20/Template/Provider.pm line 619.

Replaced it with a SWITCH, like the other patch for this similar error.

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Specific for other prefs

opacmainuserblock
opacnav
opacnavright
opaccredits
opacheader
opaccustomsearch
opacmysummaryhtml
opacmysummarynote
opacnavbottom
opacnoresultsfound
opacresultssidebar
opacsearchfortitlein
restrictedpagecontent
PatronSelfRegistrationAdditionalInstructions
intranetmainuserblock
intranetnav
intranetslipprinterjs
OpacSuppressionMessage
SCOUserCSS
SCOUserJS
SelfCheckHelpMessage
NoLoginInstructions

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Specific for Salutation on editing a patron

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Specific for XSLTBloc

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Fix escape on sending baskets or shelves by email

Test plan:
Send baskets and shelves by email.
With or without this patch, you should not see any changes.

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Fix for news

Signed-off-by: Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: Fix last occurrences recently introduced to master

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: followup to remove tabs

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
This followup on top of remote branch
Only remove tabs and trailing spaces to make koha-qa pass

Bug 13618: Fix for edit biblios and items

On editing biblios or items, the marc_lib, marc_value and javascript
values are often populated with html code which needs to be displayed
raw.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: (follow-up) Specific for ColumnsSettings

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Bug 13618: (follow-up) add missing lines for opac-shelves

Proposed patch to fix opac-shelves

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Bug 13618: Remove html filters for newly pushed code

Bug 13618: Fix for system preference description

If a syspref description contains html tag, do not display them

Bug 13618: Do not display &nbsp; and html tags in item fields content

Note that there might be other occurrences to fix!

Bug 13618: Do not display html tags in patron's notices

Bug 13618: Fix for debarredcomment and patron messages

At the OPAC and intranet.

Bug 13618: (follow-up) Specific for other prefs

follow-up for SlipCSS and printslip
Comment 214 Jonathan Druart 2017-02-07 15:49:51 UTC
Created attachment 59984 [details] [review]
Bug 13618: Use a simplified version of Template::Stash::AutoEscaping

Koha::Template::Escape is a simple version of Template::Stash::AutoEscaping
It seems that removing and cleanly some pieces of Template::Stash::AutoEscaping
will bring us a lot of performance boost.
Comment 215 Jonathan Druart 2017-02-07 16:00:22 UTC
Created attachment 59987 [details] [review]
Bug 13618: Specific for branches.opac_info

Forgot this one when I squashed the others.
Comment 216 Jonathan Druart 2017-02-07 16:01:11 UTC Comment hidden (obsolete)
Comment 217 Jonathan Druart 2018-06-20 16:49:53 UTC
Patches have been rebased, adjusted for master and push to https://gitlab.com/joubu/Koha/tree/bug_13618

They do not deal with performance issues but are ready to be tested (to catch missing .raw)
Comment 218 Jonathan Druart 2018-06-21 14:10:30 UTC
(In reply to Jonathan Druart from comment #217)
> Patches have been rebased, adjusted for master and push to
> https://gitlab.com/joubu/Koha/tree/bug_13618
> 
> They do not deal with performance issues but are ready to be tested (to
> catch missing .raw)

No need to test if we do not have a solution on bug 20975.
Comment 219 Jonathan Druart 2018-07-19 21:00:19 UTC
I am back!

Next version has been pushed to the remote branch - https://gitlab.com/joubu/Koha/commits/bug_13618

Here is commit message of the main patch:

    As we did not fix the performance issue when autofiltering
    the variables (see bug 20975), the only solution we have is to add the
    filters explicitely.

    This patch has been autogenerated (using add_html_filters.pl, see next
    pathces) and add the html filter to all the variables displayed in the
    template.
    Exceptions are made (using the new 'raw' TT filter) to the variable we
    already listed in the previous versions of this patch.

    To test:
    - Use t/db_dependent/Koha/Patrons.t to populate your DB with autogenerated
    data which contain <script> tags

    - Remove them from borrower_debarments.comments (there are allowed here)
    update  borrower_debarments set comment="html tags possible here";

    - From the interface hit page and try to catch alert box.
    If you find one it means you find a possible XSS.
    To know where it comes from:
    * note the exact URL where you found it
    * note the alert box content
    * Dump your DB and search for the string in the dump to identify its
    location (for instance table.field)

    Next:
    * Ideally we would like to use the raw filter when it is not necessary
    to HTML escape the variables (in big loop for instance)
    * Provide a QA script to catch missing filters (we want html, uri, url
    or raw, certainly others that I am forgetting now)
    * Replace the html filters with uri when needed (!)


Please test!
Comment 220 Martin Renvoize 2018-07-20 14:24:34 UTC
Looks to be going along the right lines to me, I like the approach and can see it leading to a much better base to start with.

Maintaining it going forward is the next challenge.. hopefully some carefully placed git hooks could help there, or adding some logic to the qa script to try and catch them?

I've also often wondered about having a community arachni server (or some other automated penetration testing system) setup and hitting a clone of master regularly... think I've brought that idea up before but didn't have the time to pursue it.
Comment 221 Owen Leonard 2018-08-08 18:26:26 UTC
I did what I hope was a fairly thorough test of the staff client and found these issues:

- IntranetCirculationHomeHTML displays HTML tags as text
- Patron title include showing HTML:  <span class="patron-title">Mr&lt;/span&gt;
- Patron details -> Holds tab: Alerts data from the branches table
- Search results page layout is broken. Looks like page-numbers.inc has a section missing.
- Crazy encoding of action buttons on Lists page
- Incorrectly escaped HTML in Notices & slips list
- Label batch list title encoding wrong
- Spine label print shows HTML
- Administration -> Libraries: Alerts data from the branches table
- Administration -> Item types: Alerts data from the items table
- Item searching broken: "Unsupported format html at /home/vagrant/kohaclone/catalogue/itemsearch.pl line 42."
Comment 222 Jonathan Druart 2018-08-09 20:38:30 UTC
(In reply to Owen Leonard from comment #221)
> I did what I hope was a fairly thorough test of the staff client and found
> these issues:
> 
> - IntranetCirculationHomeHTML displays HTML tags as text

Done, specific patch for this pref.

> - Patron title include showing HTML:  <span
> class="patron-title">Mr&lt;/span&gt;

Done, see specific patch.

> - Patron details -> Holds tab: Alerts data from the branches table

Done, that was tricky and a part I forgot, we need to escape data using JS, see String.prototype.escapeHtml

> - Search results page layout is broken. Looks like page-numbers.inc has a
> section missing.

Ooops, wrong merge conflict resolution.

> - Crazy encoding of action buttons on Lists page
> - Incorrectly escaped HTML in Notices & slips list

Both fixed now.

> - Label batch list title encoding wrong
> - Spine label print shows HTML

Fixed but follow-ups needed (TODO LATER)

> - Administration -> Libraries: Alerts data from the branches table

It comes from opac_info, which can contain html characters.
See admin/branches.tt: library.opac_info is not escaped (" | $raw")

> - Administration -> Item types: Alerts data from the items table

Same as before for itemtype.checkinmsg. I have added a patch for the missing $raw filter to make it explicit.

> - Item searching broken: "Unsupported format html at
> /home/vagrant/kohaclone/catalogue/itemsearch.pl line 42."

Done, that was a hard one!
Comment 224 Owen Leonard 2018-08-13 17:06:46 UTC
Found two more issues:

- The branches table is alerting something on ILL requests -> New request
- IntranetUserJS incorrectly outputs encoded HTML entities
Comment 225 Jonathan Druart 2018-08-13 17:33:36 UTC
(In reply to Owen Leonard from comment #224)
> Found two more issues:
> 
> - The branches table is alerting something on ILL requests -> New request

This comes from ILL backends (not in Koha).

> - IntranetUserJS incorrectly outputs encoded HTML entities

Fixed in a new patch.
Comment 226 Owen Leonard 2018-08-14 15:24:20 UTC
I have tested this to the best of my ability and found it to be working!

****************
** Signed off **
****************
Comment 227 Jonathan Druart 2018-08-14 15:26:49 UTC
(In reply to Owen Leonard from comment #226)
> I have tested this to the best of my ability and found it to be working!
> 
> ****************
> ** Signed off **
> ****************

Signed-off-by lines added to the remote branch, thanks for testing!

I also have rebased the remote branch against master.
Comment 228 Martin Renvoize 2018-08-14 20:35:52 UTC
I've spent a few hours with this one this evening...

The QA script raises a few warnings, but I think they're false positives or pre-existing issues in all cases at this point.

I'm sure there will be issues with a patchset of this magnitude, but I am confident we will correct them before release and that they would not be caught at this point by delaying any further.

**********
Passing QA
**********
Comment 229 Martin Renvoize 2018-08-14 21:00:59 UTC
Created attachment 77804 [details] [review]
Bug 13618: [DO NOT PUSH] have fun

In order to generate quickly a lot of relevant data I have modified a
bit TestBuilder->_gen_text to insert <script> tags in DB. The
transaction have been removed from t/db_dependent/Koha/Patrons.t

To use it, use a clean data (sample data only) the run
t/db_dependent/Koha/Patrons.t (only once, it will fail it you run it
more)
Search for patrons, without this patch you will get a looot of alert
boxes.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 230 Martin Renvoize 2018-08-14 21:02:48 UTC
Created attachment 77805 [details] [review]
Bug 13618: [DO NOT PUSH] have fun

In order to generate quickly a lot of relevant data I have modified a
bit TestBuilder->_gen_text to insert <script> tags in DB. The
transaction have been removed from t/db_dependent/Koha/Patrons.t

To use it, use a clean data (sample data only) the run
t/db_dependent/Koha/Patrons.t (only once, it will fail it you run it
more)
Search for patrons, without this patch you will get a looot of alert
boxes.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 231 Jonathan Druart 2018-08-14 21:08:31 UTC
Created attachment 77806 [details] [review]
Bug 13618: [DO NOT PUSH] have fun

In order to generate quickly a lot of relevant data I have modified a
bit TestBuilder->_gen_text to insert <script> tags in DB. The
transaction have been removed from t/db_dependent/Koha/Patrons.t

To use it, use a clean data (sample data only) the run
t/db_dependent/Koha/Patrons.t (only once, it will fail it you run it
more)
Search for patrons, without this patch you will get a looot of alert
boxes.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 232 Martin Renvoize 2018-08-14 21:09:49 UTC
I was going to upload the patches for you Nick, but it turns out some of them are too large for bugzilla so Jonathan is adding my signoff lines to his tracking branch on gitlab ;)

Note, bug 15717 is rolled into this branch patchset.. it all checks out too ;)
Comment 233 Jonathan Druart 2018-08-14 21:13:59 UTC
Created attachment 77807 [details] [review]
Bug 13618: [DO NOT PUSH] have fun

In order to generate quickly a lot of relevant data I have modified a
bit TestBuilder->_gen_text to insert <script> tags in DB. The
transaction have been removed from t/db_dependent/Koha/Patrons.t

To use it, use a clean data (sample data only) the run
t/db_dependent/Koha/Patrons.t (only once, it will fail it you run it
more)
Search for patrons, without this patch you will get a looot of alert
boxes.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 234 Jonathan Druart 2018-08-14 21:14:08 UTC
Created attachment 77808 [details] [review]
Bug 13618: [DO NOT PUSH] Add script to add the html filters

Usage: perl add_html_filters.pl **/*.inc **/*.tt
It also takes --test for debugging

Then:
git grep Asset|grep -v USE | grep -v '| $raw'
=> We should deal with them on a separate commit

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 235 Jonathan Druart 2018-08-14 21:16:44 UTC
The two patches used to build this patchset and test it have been attached to this bug report.

Others are not as the first one is too big (413 Request Entity Too Large). Martin's Signed-off-by lines have been added to my remove branch and the "DO NOT PUSH" patches removed.
Comment 236 Jonathan Druart 2018-08-14 21:21:03 UTC
Created attachment 77809 [details]
patches_to_be_pushed.tar.gz

For reference.
Comment 237 Nick Clemens 2018-08-17 17:22:25 UTC
Awesome work all, special thanks to Jonathan for working on this one for so long and to everyone who helped along the way.

Pushed to master for 18.11
Comment 238 Jonathan Druart 2018-08-17 18:02:05 UTC
Created attachment 77997 [details] [review]
Bug 13618: Remove filter when assigning array
Comment 239 Jonathan Druart 2018-08-17 18:04:06 UTC
Created attachment 77998 [details] [review]
Bug 13618: Fix xt/tt_valid.t

We are going to say it quietly but this test does not catch the problem
when there are no spaces....shhhh

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 240 Nick Clemens 2018-08-17 18:08:20 UTC
(In reply to Jonathan Druart from comment #239)
> Created attachment 77998 [details] [review] [review]
> Bug 13618: Fix xt/tt_valid.t
> 
> We are going to say it quietly but this test does not catch the problem
> when there are no spaces....shhhh
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Followup pushed to master
Comment 242 Jonathan Druart 2018-08-24 22:16:32 UTC
Created attachment 78158 [details] [review]
Bug 13618: (follow-up) Manually replace missing .raw

Must be |$raw, not |raw

Error:
Template process failed: undef error - raw: filter not found at
/home/vagrant/kohaclone/C4/Templates.pm line 122

To recreate:
Add a new restriction and visit circulation.pl?borrowernumber=42
Comment 243 Nick Clemens 2018-08-25 19:14:12 UTC
(In reply to Jonathan Druart from comment #242)
> Created attachment 78158 [details] [review] [review]
> Bug 13618: (follow-up) Manually replace missing .raw
> 
> Must be |$raw, not |raw
> 
> Error:
> Template process failed: undef error - raw: filter not found at
> /home/vagrant/kohaclone/C4/Templates.pm line 122
> 
> To recreate:
> Add a new restriction and visit circulation.pl?borrowernumber=42

Followup pushed to master
Comment 244 Ere Maijala 2018-08-27 12:11:08 UTC
Looks like escaping was added also to places where it doesn't belong. The examples I stumbled on were just setting a variable:

https://github.com/Koha-Community/Koha/blob/master/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt#L328

and 

https://github.com/Koha-Community/Koha/blob/master/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt#L330
Comment 245 Jonathan Druart 2018-08-29 17:39:30 UTC
Created attachment 78274 [details] [review]
Bug 13618: (follow-up) Add html filters to all the variables
Comment 246 Jonathan Druart 2018-08-29 17:42:51 UTC
Thanks Ere for reporting it!
Comment 247 Nick Clemens 2018-08-30 14:07:56 UTC
(In reply to Jonathan Druart from comment #245)
> Created attachment 78274 [details] [review] [review]
> Bug 13618: (follow-up) Add html filters to all the variables

Followup pushed to master
Comment 248 Jonathan Druart 2018-09-04 19:13:23 UTC
Created attachment 78415 [details] [review]
Bug 13618: Do not use html filters with KohaSpan

To recreate the issue:
Go to Home › Administration › Library groups
Create a new group
 "Group <span class='name'>xxx</span> created."

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 249 Nick Clemens 2018-09-07 16:04:07 UTC
(In reply to Jonathan Druart from comment #248)
> Created attachment 78415 [details] [review] [review]
> Bug 13618: Do not use html filters with KohaSpan
> 
> To recreate the issue:
> Go to Home › Administration › Library groups
> Create a new group
>  "Group <span class='name'>xxx</span> created."
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Followup pushed to master
Comment 250 Marcel de Rooy 2018-09-14 09:35:11 UTC
Lesson pasted from bug 21293:

All occurrences of [% var = something | html %] are error prone. If something is not a string, but an object, array, hash etc., we are in trouble.
This probably needs more attention since we are passing objects to templates in more scripts.
Comment 251 Nick Clemens 2018-09-26 10:18:14 UTC
Created attachment 79404 [details] [review]
Bug 13618: (follow-up) Pass opacuser_js from plugins as raw

We expect this field to contain script tags, html processing breaks them
Comment 252 Nick Clemens 2018-09-28 13:30:18 UTC
*** Bug 20874 has been marked as a duplicate of this bug. ***
Comment 253 Nick Clemens 2018-09-28 13:30:38 UTC
*** Bug 20873 has been marked as a duplicate of this bug. ***
Comment 254 Nick Clemens 2018-09-28 13:30:53 UTC
*** Bug 20872 has been marked as a duplicate of this bug. ***
Comment 255 Nick Clemens 2018-09-28 13:31:08 UTC
*** Bug 20871 has been marked as a duplicate of this bug. ***
Comment 256 Nick Clemens 2018-09-28 13:31:22 UTC
*** Bug 20869 has been marked as a duplicate of this bug. ***
Comment 257 Nick Clemens 2018-09-28 13:31:36 UTC
*** Bug 20870 has been marked as a duplicate of this bug. ***
Comment 258 Nick Clemens 2018-10-01 16:32:42 UTC
(In reply to Nick Clemens from comment #251)
> Created attachment 79404 [details] [review] [review]
> Bug 13618: (follow-up) Pass opacuser_js from plugins as raw
> 
> We expect this field to contain script tags, html processing breaks them

Follow-up pushed to master
Comment 259 Jonathan Druart 2018-10-01 20:26:29 UTC
A new check to catch missing filters is not in the QA script, please update!
Comment 260 Nick Clemens 2018-10-02 00:18:10 UTC
(In reply to Jonathan Druart from comment #259)
> A new check to catch missing filters is not in the QA script, please update!

s/not/now/
Comment 261 Jonathan Druart 2018-10-02 11:49:28 UTC
Created attachment 79818 [details] [review]
Bug 13618: remove wrong html filter in 'Item search fields' admin

The dropdown list contain the arrayref: ARRAY(0x...)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 262 Jonathan Druart 2018-10-16 16:33:57 UTC
*** Bug 20909 has been marked as a duplicate of this bug. ***
Comment 263 Jonathan Druart 2018-10-25 16:09:43 UTC
Created attachment 81183 [details] [review]
Bug 13618: Remove html filter for LAST

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 264 Nick Clemens 2018-10-26 17:13:44 UTC
(In reply to Jonathan Druart from comment #261)
>Created attachment 79818 [details] [review] [review]
>Bug 13618: remove wrong html filter in 'Item search fields' admin
>
>The dropdown list contain the arrayref: ARRAY(0x...)
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

(In reply to Jonathan Druart from comment #263)
> Created attachment 81183 [details] [review] [review]
> Bug 13618: Remove html filter for LAST
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Last two followups pushed to master
Comment 265 M. Tompsett 2018-12-04 17:47:52 UTC
http://koha.1045719.n5.nabble.com/HTML-filtering-issue-td6004477.html
Fix here? Fix on another bug?
Comment 266 Mark Alexander 2018-12-24 16:51:26 UTC
In the OPAC, when looking at the MARC view for an item, there is a table at the bottom of the page showing some holding information. Those table fields that used to be blank in 18.05 are now displaying as "&nbsp;" (without the quotes) in 18.11. In my particular case, the fields that are displayed this way are:

* Withdrawn status
* Lost status
* Damaged status
* Not for loan

I'll attach some before-and-after screenshots.

I determined that this was caused by the change to opac-MARCdetail.tt made for bug 13618, by reverting that change for line 151.  That is, I changed that line from:

    <td>[% item.$sf_code | html %]</td>

to this:

    <td>[% item.$sf_code %]</td>

That removed the &nbsp; mess, but I'm guessing this is not the correct solution.
Comment 267 Mark Alexander 2018-12-24 16:53:56 UTC
Created attachment 83479 [details]
OPAC MARC view holding detail in 18.05
Comment 268 Mark Alexander 2018-12-24 16:54:39 UTC
Created attachment 83480 [details]
OPAC MARC view holding detail in 18.11
Comment 269 Katrin Fischer 2018-12-28 07:22:03 UTC
This bug has been marked resolved fixed - please file any remaining issues as new bugs and link to this one using the "depends" on the new bug.
Comment 270 Jonathan Druart 2019-02-19 12:46:30 UTC
*** Bug 22342 has been marked as a duplicate of this bug. ***