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