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

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