Huge performance issue cause by bug 13618 (XSS)
Hi devs, I have a very bad news... Working on bug 15715, I have found that the patchset introduced by bug 13618 (Prevent XSS in the Staff Client and the OPAC) causes a very important performance issue. Indeed, for each variable displayed in the template, a Template::Stash::AutoEscaping->get method is called. To try and understand the problem, I have put the following snippet in the mainpage.pl script: +my @loop; +for my $i ( 0 .. 10000 ) { + push @loop, { + foo => 'my foo', + bar => 'my bar', + }; +} +$template->param( loop => \@loop ); and then, in the template +[% FOR l IN loop %] + [% l.foo %] [% l.bar %] + <br/> +[% END %] It will display 10k foo and 10k bar There are indeed 20k (+ delta) calls to the Template::Stash::AutoEscaping method, as expected. But the load of the page increases from 2sec to 10sec (no plack, measured with Firebug > network tab) On catalogue/search.pl, 1800 calls, from 6sec to 6.5s On authorities/authorities.pl?breedingid=186&authtypecode=CORPO_NAME&authid=0 (From bug 15715), 64k calls (!!!) from ~10sec to ~4min On cataloguing/addbiblio.pl?biblionumber=1, 13k calls, from 3sec to 9sec Do not take these measures too strictly, but they give an idea of the issue... So I don't see a lot of methods to fix that: 1/ Revert and abandon 13618 2/ Understand how it's possible to have 60k calls to display a form and try to improve these areas 3/ Profile Template::Stash::AutoEscaping to know if we cannot optimise what it does 4/ Any brilliant ideas? Jonathan https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15715 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13618
Too bad, especially since we already have performance issues.. I would go for option 1 part a = revert. (It is one of the last pushes, I think.) And some further research on 2,3 or 4 ? :) ________________________________________ Van: koha-devel-bounces@lists.koha-community.org [koha-devel-bounces@lists.koha-community.org] namens Jonathan Druart [jonathan.druart@bugs.koha-community.org] Verzonden: woensdag 10 februari 2016 15:49 Aan: koha-devel@lists.koha-community.org Onderwerp: [Koha-devel] Huge performance issue cause by bug 13618 (XSS) Hi devs, I have a very bad news... Working on bug 15715, I have found that the patchset introduced by bug 13618 (Prevent XSS in the Staff Client and the OPAC) causes a very important performance issue. Indeed, for each variable displayed in the template, a Template::Stash::AutoEscaping->get method is called. To try and understand the problem, I have put the following snippet in the mainpage.pl script: +my @loop; +for my $i ( 0 .. 10000 ) { + push @loop, { + foo => 'my foo', + bar => 'my bar', + }; +} +$template->param( loop => \@loop ); and then, in the template +[% FOR l IN loop %] + [% l.foo %] [% l.bar %] + <br/> +[% END %] It will display 10k foo and 10k bar There are indeed 20k (+ delta) calls to the Template::Stash::AutoEscaping method, as expected. But the load of the page increases from 2sec to 10sec (no plack, measured with Firebug > network tab) On catalogue/search.pl, 1800 calls, from 6sec to 6.5s On authorities/authorities.pl?breedingid=186&authtypecode=CORPO_NAME&authid=0 (From bug 15715), 64k calls (!!!) from ~10sec to ~4min On cataloguing/addbiblio.pl?biblionumber=1, 13k calls, from 3sec to 9sec Do not take these measures too strictly, but they give an idea of the issue... So I don't see a lot of methods to fix that: 1/ Revert and abandon 13618 2/ Understand how it's possible to have 60k calls to display a form and try to improve these areas 3/ Profile Template::Stash::AutoEscaping to know if we cannot optimise what it does 4/ Any brilliant ideas? Jonathan https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15715 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13618 _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
I have been slow to push anything much since I pushed that big one to give a little time. I can revert that patch set which is still an option On Wednesday, February 10, 2016, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> wrote:
Too bad, especially since we already have performance issues.. I would go for option 1 part a = revert. (It is one of the last pushes, I think.) And some further research on 2,3 or 4 ? :)
________________________________________ Van: koha-devel-bounces@lists.koha-community.org <javascript:;> [ koha-devel-bounces@lists.koha-community.org <javascript:;>] namens Jonathan Druart [jonathan.druart@bugs.koha-community.org <javascript:;>] Verzonden: woensdag 10 februari 2016 15:49 Aan: koha-devel@lists.koha-community.org <javascript:;> Onderwerp: [Koha-devel] Huge performance issue cause by bug 13618 (XSS)
Hi devs,
I have a very bad news... Working on bug 15715, I have found that the patchset introduced by bug 13618 (Prevent XSS in the Staff Client and the OPAC) causes a very important performance issue.
Indeed, for each variable displayed in the template, a Template::Stash::AutoEscaping->get method is called.
To try and understand the problem, I have put the following snippet in the mainpage.pl script: +my @loop; +for my $i ( 0 .. 10000 ) { + push @loop, { + foo => 'my foo', + bar => 'my bar', + }; +} +$template->param( loop => \@loop );
and then, in the template +[% FOR l IN loop %] + [% l.foo %] [% l.bar %] + <br/> +[% END %]
It will display 10k foo and 10k bar There are indeed 20k (+ delta) calls to the Template::Stash::AutoEscaping method, as expected. But the load of the page increases from 2sec to 10sec (no plack, measured with Firebug > network tab)
On catalogue/search.pl, 1800 calls, from 6sec to 6.5s
On authorities/ authorities.pl?breedingid=186&authtypecode=CORPO_NAME&authid=0 (From bug 15715), 64k calls (!!!) from ~10sec to ~4min
On cataloguing/addbiblio.pl?biblionumber=1, 13k calls, from 3sec to 9sec
Do not take these measures too strictly, but they give an idea of the issue...
So I don't see a lot of methods to fix that: 1/ Revert and abandon 13618 2/ Understand how it's possible to have 60k calls to display a form and try to improve these areas 3/ Profile Template::Stash::AutoEscaping to know if we cannot optimise what it does 4/ Any brilliant ideas?
Jonathan
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15715 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13618 _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org <javascript:;> http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/ _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org <javascript:;> http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Sent from Gmail Mobile
Greetings, Jonathan Druart wrote:
Indeed, for each variable displayed in the template, a Template::Stash::AutoEscaping->get method is called. ... and then, in the template +[% FOR l IN loop %] + [% l.foo %] [% l.bar %] + <br/> +[% END %]
It will display 10k foo and 10k bar There are indeed 20k (+ delta) calls to the Template::Stash::AutoEscaping method, as expected. But the load of the page increases from 2sec to 10sec (no plack, measured with Firebug > network tab)
It isn't brilliant, if it does work, because that's a lot of editing, but I was noticing people starting to use ".raw". Does a .raw have a large impact? +[% FOR l IN loop %] + [% l.foo.raw %] [% l.bar.raw %] + <br/> +[% END %] If it increased from 2s to 3s with .raw, that wouldn’t be so bad. I'm just brainstorming. GPML, Mark Tompsett
* Mark Tompsett (mtompset@hotmail.com) wrote:
Greetings,
Jonathan Druart wrote:
Indeed, for each variable displayed in the template, a Template::Stash::AutoEscaping->get method is called. ... and then, in the template +[% FOR l IN loop %] + [% l.foo %] [% l.bar %] + <br/> +[% END %]
It will display 10k foo and 10k bar There are indeed 20k (+ delta) calls to the Template::Stash::AutoEscaping method, as expected. But the load of the page increases from 2sec to 10sec (no plack, measured with Firebug > network tab)
It isn't brilliant, if it does work, because that's a lot of editing, but I was noticing people starting to use ".raw". Does a .raw have a large impact? +[% FOR l IN loop %] + [% l.foo.raw %] [% l.bar.raw %] + <br/> +[% END %]
If it increased from 2s to 3s with .raw, that wouldn’t be so bad. I'm just brainstorming.
Hi Mark .raw skips doing the escaping, opening us up for XSS attacks again. So not really a solution I'm afraid. Chris -- Chris Cormack Catalyst IT Ltd. +64 4 803 2238 PO Box 11-053, Manners St, Wellington 6142, New Zealand
Interestingly enough there was recently a long debate about xss escaping over in the Mojo irc channel. They obviously have their own temptation engine which uses Mojo::DOM in parts (which includes the xss_escape code). I'm envisioning the we'll end up needing to do that digging into Template::Stash::AutoEscaping and would suggest whoever does perhaps bases some enhancements on the mojo code ;-) For reference: http://irclog.perlgeek.de/mojo/2016-02-09#i_12010985 , https://github.com/kraih/mojo/commit/ccbd7e92ffe2029e83f14e077095d6ddda27cd1... Martin
I have reverted the patch series. On Wed, Feb 10, 2016 at 11:19 PM, Renvoize, Martin < martin.renvoize@ptfs-europe.com> wrote:
Interestingly enough there was recently a long debate about xss escaping over in the Mojo irc channel. They obviously have their own temptation engine which uses Mojo::DOM in parts (which includes the xss_escape code).
I'm envisioning the we'll end up needing to do that digging into Template::Stash::AutoEscaping and would suggest whoever does perhaps bases some enhancements on the mojo code ;-)
For reference: http://irclog.perlgeek.de/mojo/2016-02-09#i_12010985 , https://github.com/kraih/mojo/commit/ccbd7e92ffe2029e83f14e077095d6ddda27cd1...
Martin
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- --------------------------------------------------------------------------------------------------------------- Brendan A. Gallagher ByWater Solutions CEO Support and Consulting for Open Source Software Installation, Data Migration, Training, Customization, Hosting and Complete Support Packages Headquarters: Santa Barbara, CA - Office: Redding, CT Phone # (888) 900-8944 http://bywatersolutions.com info@bywatersolutions.com
participants (6)
-
Brendan Gallagher -
Chris Cormack -
Jonathan Druart -
Marcel de Rooy -
Mark Tompsett -
Renvoize, Martin