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