Smarty Forum Index Smarty
WARNING: All discussion is moving to https://reddit.com/r/smarty, please go there! This forum will be closing soon.

whitespacefilter removing js code

 
This forum is locked: you cannot post, reply to, or edit topics.   This topic is locked: you cannot edit posts or make replies.    Smarty Forum Index -> Bugs
View previous topic :: View next topic  
Author Message
mplx
Smarty n00b


Joined: 13 Jun 2010
Posts: 3

PostPosted: Thu Oct 27, 2011 8:27 am    Post subject: whitespacefilter removing js code Reply with quote

in opposite to the old whitespacefilter from smarty 3.0, the new filter is removing html comments. now this can be a problem as it is also removing javascript code:
Code:
<script><!-- alert('removed!'); //--><script>

will render as
Code:
<script></script>


the responsible code is in line 42 from outputfilter.trimwhitespace.php:
Code:
    // Strip all HTML-Comments
    $source = preg_replace( '#<!--.*?-->#ms', '', $source );


i think it should work ok when moved to line 57 when capturing elements not to mess with has been performed (untested).
Back to top
View user's profile Send private message
lethalfalcon
Smarty n00b


Joined: 17 Aug 2012
Posts: 3

PostPosted: Fri Aug 17, 2012 4:59 pm    Post subject: Not just JS code Reply with quote

I'm using MPDF, which uses conditional comments for things like the page header and footer, by enclosing them in <!--mpdf mpdf--> tags. Which trimwhitespace now removes. I fail to see why a WHITESPACE trimming filter is removing actual code. Shouldn't there be another filter for doing that kind of work? trimuselessdata or something? Either that, or rename the filter to trimallnonrenderabledata, since that's what it's doing now.

Back in 2005, there was another post on this forum asking for comment removal, and they were told that it should be another filter, because trimming whitespace is all that this does. Please consider your stance, because there are other things that use conditional comments besides old js standards.
Back to top
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Fri Aug 17, 2012 5:37 pm    Post subject: Re: Not just JS code Reply with quote

lethalfalcon wrote:
I'm using MPDF, which uses conditional comments for things like the page header and footer, by enclosing them in <!--mpdf mpdf--> tags. Which trimwhitespace now removes. I fail to see why a WHITESPACE trimming filter is removing actual code. Shouldn't there be another filter for doing that kind of work? trimuselessdata or something? Either that, or rename the filter to trimallnonrenderabledata, since that's what it's doing now.

Back in 2005, there was another post on this forum asking for comment removal, and they were told that it should be another filter, because trimming whitespace is all that this does. Please consider your stance, because there are other things that use conditional comments besides old js standards.


you are free to copy the plugin an simple remove the lines responsible for comment-removal. It is Line 44.
_________________
Twitter
Back to top
View user's profile Send private message Visit poster's website
lethalfalcon
Smarty n00b


Joined: 17 Aug 2012
Posts: 3

PostPosted: Fri Aug 17, 2012 5:53 pm    Post subject: Reply with quote

Yeah, I am, and already have done so. It's just very inconvenient when this comes up out of nowhere.

Why was it determined that a whitespace trimmer should trim code?

This wasn't illustrated in the 3.0 -> 3.1 changes, either, which is why I'm a little upset.

If nothing else, is there a way I can tell smarty to leave something alone and not apply filters to it? Something like a {nofilter} tag?
Back to top
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Fri Aug 17, 2012 6:12 pm    Post subject: Reply with quote

lethalfalcon wrote:
Yeah, I am, and already have done so. It's just very inconvenient when this comes up out of nowhere.

Why was it determined that a whitespace trimmer should trim code?

This wasn't illustrated in the 3.0 -> 3.1 changes, either, which is why I'm a little upset.


This change was, in fact, introduced by me in 3.1.0. Noting this change in the upgrade notes seems to be my fault as well, I guess. I'm sorry for that!

lethalfalcon wrote:
If nothing else, is there a way I can tell smarty to leave something alone and not apply filters to it? Something like a {nofilter} tag?


Such a tag would have no effect on an output-filter. I'm still sketiching out a possible solution to the "plugin configuration mess" ("> <" or "><"? HTML or XHTML? and so on…)

as of now can't tell you more than "removing that line helps short term".
_________________
Twitter
Back to top
View user's profile Send private message Visit poster's website
lethalfalcon
Smarty n00b


Joined: 17 Aug 2012
Posts: 3

PostPosted: Fri Aug 17, 2012 6:30 pm    Post subject: Reply with quote

What would be nice is if you could pass parameters via registerFilter(). Then you could easily allow for people to select what to trim. However, that would require changes to core.

A simpler option would be to have two filters, trimwhitespace and trimcomments. You're already running every one of the preg_replace commands separately, so it wouldn't really be any more inefficient having it in two different filters other than the overhead of calling the second file.
Back to top
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Fri Aug 17, 2012 7:10 pm    Post subject: Reply with quote

lethalfalcon wrote:
What would be nice is if you could pass parameters via registerFilter(). Then you could easily allow for people to select what to trim. However, that would require changes to core.


I was thinking about a "central configuration facility". Something along the lines of $smarty->config('trimwhitespace.comments', false); This would behave very much like Smarty_Security.

lethalfalcon wrote:
A simpler option would be to have two filters, trimwhitespace and trimcomments. You're already running every one of the preg_replace commands separately, so it wouldn't really be any more inefficient having it in two different filters other than the overhead of calling the second file.


This is indeed a solution. But it is only a solution to *the very problem* *you* are facing *right now*. I'd much prefer a solution that takes care of this "but I want plugin X to behave like Y" business - once and for all.
_________________
Twitter
Back to top
View user's profile Send private message Visit poster's website
Display posts from previous:   
This forum is locked: you cannot post, reply to, or edit topics.   This topic is locked: you cannot edit posts or make replies.    Smarty Forum Index -> Bugs All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group
Protected by Anti-Spam ACP