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).
Created attachment 76230 [details] [review] 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
Created attachment 76231 [details] [review] 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
Created attachment 76232 [details] [review] 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
Created attachment 76233 [details] [review] 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
Created attachment 76234 [details] [review] 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
Created attachment 76235 [details] [review] 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
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.
(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
No longer valid, new version of bug 13618 has been submitted. Thanks for the feedback Julian!