Re: [patch] add $smarty->escape_output feature

Monte Ohrt <[email protected]> Fri, 05 May 2006 09:52:14 -0500
Newsgroups gmane.comp.php.smarty.devel
Message-ID <[email protected]>
? I'm not against the feature at all, as a matter of fact it has been 
discussed many times before. I was only making an observation: only 
escaped echo'ed content, don't escape content within logical statements. 
It looks like it already works that way... I have not looked at the 
patch, nor have time in the near future. Maybe boots or messju can help 
out with a cvs commit in the meantime.

Monte

Martijn van Zal wrote:
> I think this is a great addition to Smarty, and I'm sure going to use
> it. 
> It changes the way you think about escaping, normally you have so escape
> all variables which somehow could contain html, if you forget to do it,
> you won't notice until someone (deliberately) abuses it.
>
> Using this function you don't have to worry about html which could be in
> your variables, it just displays it the way you want it all the time. If
> you forget to use the noencode modifier you find out early in the
> process, and it's easy to fix. It should be more difficult to insert
> HTML code in a template.
>
> I think Monthe misunderstanded your first mail and overreacted a little,
> your last explanation is perfectly clear.
>
> Kindest regards,
>  
> Martijn van Zal
> Technical Director
> Email. [email protected]
> Cell. +31 (0)6 42721045
>
> Brothers in art
> Stationsstraat 20b 
> 1211 EN Hilversum
> Tel. +31 (0)35 6220093
> Fax. +31 (0)35 6210996
> www.brothersinart.net
>
> -----Original Message-----
> From: Andreas Korthaus [mailto:[email protected]] 
> Sent: donderdag 4 mei 2006 18:19
> To: [email protected]; Monte Ohrt
> Subject: Re: [SMARTY-DEV] [patch] add $smarty->escape_output feature
>
> Hi!
>
> Monte Ohrt wrote:
>  > I don't think you should require this in the template:
>  >
>  > {if $foo|noescape eq "<bar>"}
>
> I don't require that!
>
> When I apply my patch and try the following code:
>
> xss.php:
> <?php
> $smarty = new Smarty();
> $smarty->escape_output='html';
> $smarty->assign('xss', '<xss>');
> $smarty->display('xss.tpl');
> ?>
>
> xss.tpl:
> {$xss}
> {$xss|noescape}
> {if $xss eq "&lt;xss&gt;"}ESCAPE{/if}
> {if $xss eq "<xss>"}NOESCAPE{/if}
>
> I get the following output:
>
> &lt;xss&gt;
> <xss>
> NOESCAPE
>
> That's because I don't apply a (default_)modifier on a variable, but I
> change what the compiler does when writing "echo $variable" to the
> compiled template. The "heart" of my change is the following in function
> _compile_tag() from Smarty_Compiler.class.php (line 435).
>
> Before my change:
>
> if (preg_match(
>    '~^' .
>    $this->_num_const_regexp . '|' .
>    $this->_obj_call_regexp . '|' .
>    $this->_var_regexp . '$~', $tag_command)) {
>
>      $_return = $this->_parse_var_props($tag_command . $tag_modifier);
>
>      return "<?php echo $_return; ?>" . $this->_additional_newline; }
>
> After my change:
>
> if (preg_match(
>    '~^' .
>    $this->_num_const_regexp . '|' .
>    $this->_obj_call_regexp . '|' .
>    $this->_var_regexp . '$~', $tag_command)) {
>
>      $_return = $this->_parse_var_props($tag_command . $tag_modifier);
>
>      // THAT'S WHAT'S NEW:
>      $_return_escaped = $this->_escape_var($_return, $tag_modifier);
>
>      return "<?php echo $_return_escaped; " .
> $this->_additional_newline; }
>
>
> So only if the regular expression above is true, the "current tag" gets
> escaped.
>
> As far as I understand, block functions like {if} are parsed completely
> somewhere else, with all included vars. And I don't need to catch them,
> because value of variables INSIDE {if} will never be written to STDOUT
> when the PHP interpreter executes the compiled template.
>
> If you do
>
> {if $var eq "<xss>"}{$var}{/if}
>
> you don't have to worry about what you find in {if}. Only {$var} can
> reach the users browser, and that's what is catched by my patch. {if}
> only creates PHP-Code, which will be away after interpreting the
> PHP-script.
>
> BUT: I'm not sure if there are more wholes in the smarty core compiler,
> where  variables like $var still can reach the browser using something
> similar like {if}. It must be a function, which get's a variable as
> input, and which becomes part of the output. That's what modifiers do,
> so I added my escaping _after_ applying all modifiers. Perhaps you can
> think about something else here?
>
> Or perhaps you can think about a case, where escaping output from
> modifers could be a bad idea?
>
> It's important to realize, that $smarty->escape_output only makes sense,
>   if assigning HTML-Code from PHP to the template is considered to be
> bad practice. Passing HTML _SHOULD_ be more difficult and uncomfortable.
>
> If you excessively want to pass HTML to your templates,
> $smarty->escape_output is not the right feature for you. If you want to
> make these people happy too, you will end up with something like the
> current $default_modifiers, which are not useful for anybody.
>
> Of course there can allways be exceptions, that's what the "noescape" 
> modifier is for.
>
> If a user doesn't agree with that, he should not use
> $smarty->escape_output at all. That's the only way to come to a
> clean/complete solution.
>
> People also don't tend to use $default_template_handler_func if they are
> happy with the default ;-)
>
>  > The engine should determine that $foo is not output (not echo()), so
>   
>> it shouldn't get escaped in the first place.
>>     
>
> That's exactly what's happening with the patch ;-)
>
>  > foo is {$foo}
>  >
>  > That should get escaped. The compiled template would be something  >
> like: echo htmlspecialchars($this->_tpl_vars['foo'],ENT_QUOTES);
>
> exactly! That's what my patch does.
>
> boots thinks[1] about not using escape "modes" like
> "html|htmlall|url|user_defined", but a callback-function. I'm not sure
> how to add parameters to the callback function (like ENT_QUOTES and
> UTF-8 for htmlspecialchars). If you have to add a user-space wrapper
> function to pass the parameters, escaping becomes slower than my
> solution, which hardcodes htmlspecialchars()... in the compiled
> templates.
>
> What do you think about that?
>
> [1]: http://www.phpinsider.com/smarty-forum/viewtopic.php?p=30374#30374
>
>
> Best regards
> Andreas
>
> --
> Smarty Development Mailing List (http://smarty.php.net/) To unsubscribe,
> visit: http://www.php.net/unsub.php
>
>   

-- 
Smarty Development Mailing List (http://smarty.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php