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 "<xss>"}ESCAPE{/if} > {if $xss eq "<xss>"}NOESCAPE{/if} > > I get the following output: > > <xss> > <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