[Bug 20975] New: Improve auto escaping performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 Bug ID: 20975 Summary: Improve auto escaping performance Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 13618 On bug 13618 we are trying to get rid of XSS issues with a global solution. Using Template::Stash::AutoEscaping we are going to apply a html filter to all of our template variables. The problem is that we can have thousands of variables displayed (and so escaped). Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13618 [Bug 13618] Prevent XSS in the Staff Client and the OPAC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 76230 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76230&action=edit AutoEscaping - no filter for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done perl benchmark_AutoEscaping.pl 0.20s user 0.00s system 98% cpu 0.208 total perl benchmark_AutoEscaping.pl 0.19s user 0.02s system 98% cpu 0.214 total perl benchmark_AutoEscaping.pl 0.18s user 0.02s system 98% cpu 0.204 total perl benchmark_AutoEscaping.pl 0.18s user 0.02s system 98% cpu 0.202 total perl benchmark_AutoEscaping.pl 0.18s user 0.02s system 97% cpu 0.205 total -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 76231 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76231&action=edit AutoEscaping - TT html filters for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done perl benchmark_AutoEscaping.pl 0.30s user 0.09s system 95% cpu 0.414 total perl benchmark_AutoEscaping.pl 0.27s user 0.02s system 97% cpu 0.295 total perl benchmark_AutoEscaping.pl 0.28s user 0.02s system 99% cpu 0.307 total perl benchmark_AutoEscaping.pl 0.30s user 0.02s system 98% cpu 0.320 total perl benchmark_AutoEscaping.pl 0.27s user 0.02s system 99% cpu 0.290 total -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 76232 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76232&action=edit AutoEscaping - Template::Stash::AutoEscaping (escape all) for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done perl benchmark_AutoEscaping.pl 9.67s user 5.08s system 99% cpu 14.837 total perl benchmark_AutoEscaping.pl 9.00s user 4.10s system 99% cpu 13.167 total perl benchmark_AutoEscaping.pl 9.24s user 4.97s system 99% cpu 14.263 total perl benchmark_AutoEscaping.pl 9.91s user 4.19s system 99% cpu 14.188 total perl benchmark_AutoEscaping.pl 8.86s user 4.44s system 99% cpu 13.347 total -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 76233 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76233&action=edit AutoEscaping - Do not escape using ignore_escape The variable are not escaped (let's suppose it's what we want) for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done perl benchmark_AutoEscaping.pl 7.37s user 4.12s system 99% cpu 11.557 total perl benchmark_AutoEscaping.pl 7.15s user 4.24s system 99% cpu 11.438 total perl benchmark_AutoEscaping.pl 6.99s user 3.70s system 99% cpu 10.751 total perl benchmark_AutoEscaping.pl 6.32s user 3.76s system 99% cpu 10.120 total perl benchmark_AutoEscaping.pl 6.32s user 3.71s system 99% cpu 10.057 total -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 76234 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76234&action=edit AutoEscaping - Do not escape using .raw Same as before, vars will not be escaped for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done perl benchmark_AutoEscaping.pl 7.43s user 3.92s system 99% cpu 11.400 total perl benchmark_AutoEscaping.pl 6.56s user 3.75s system 99% cpu 10.377 total perl benchmark_AutoEscaping.pl 6.11s user 4.08s system 99% cpu 10.233 total perl benchmark_AutoEscaping.pl 7.02s user 3.93s system 99% cpu 10.984 total perl benchmark_AutoEscaping.pl 6.91s user 3.70s system 99% cpu 10.664 total -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 76235 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76235&action=edit AutoEscaping - Koha::Template::AutoEscaping for x in 1 2 3 4 5; do time perl benchmark_AutoEscaping.pl; done perl benchmark_AutoEscaping.pl 0.34s user 0.07s system 96% cpu 0.425 total perl benchmark_AutoEscaping.pl 0.35s user 0.04s system 99% cpu 0.387 total perl benchmark_AutoEscaping.pl 0.35s user 0.04s system 99% cpu 0.396 total perl benchmark_AutoEscaping.pl 0.50s user 0.03s system 99% cpu 0.532 total perl benchmark_AutoEscaping.pl 0.38s user 0.02s system 99% cpu 0.400 total -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |In Discussion --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Koha::Template::AutoEscaping is a naive adaptation of Template::Stash::AutoEscaping. I have tried to simplify the code to remove what will not be useful for us. The performances are good, I guess it helps to avoid 1 object creation (::Escaped::HTML) by variable. But... it does not work. If you are applying the last patch on top of bug 13618 you will notice that variables are double-escaped. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #8 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Jonathan Druart from comment #7)
The performances are good, I guess it helps to avoid 1 object creation (::Escaped::HTML) by variable.
It's actually worse than that. Template::Stash::AutoEscaping creates an object for each string concatenation, and Template::Toolkit concatenates stuff to its internal buffer all the time. I ran benchmark_AutoEscaping.pl (using Template::Stash::AutoEscaping) with NYTProf and it spends most of the time in Template::Stash::AutoEscaping::Escaped::Base::concat (63% of total time) where it creates 50000 new objects. But apparently most of the time here is spent doing simple string concatenation. (not so) fun fact: the "HTML-escaping" process represents less than 3% of concat in terms of time spent. I don't know how (or if) this can be improved, but I think the time loss is not acceptable, and unless we find a better solution we should keep using html filter manually (and probably add a rule in coding guidelines) Note that we can use TT filters as blocks, like this: [% FILTER html %] [% this_will_be_escaped %] and [% this.will(be).escaped | too %] [% END %] Maybe this could reduce the hassle of writing "|html" everywhere -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |WONTFIX Status|In Discussion |RESOLVED --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- No longer valid, new version of bug 13618 has been submitted. Thanks for the feedback Julian! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20975 Bug 20975 depends on bug 13618, which changed state. Bug 13618 Summary: Add additional template filter methods and a filter presence test to Koha https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13618 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org