Re: Escaping onclick-attribute JavaScript

[Sander Aarts]

It's better not to use inline event handlers at all, but use JavaScript to bind the event handler to the DOM node:

HTML:

<button class="delete"><img ...></button>

jQuery (for simplicity, but could be vanllia JS of course):

$('button.delete').on('click', function() {

    if (confirm('Really delete?')) {

        deleteThis(this);

    }

});

That way you keep your layers separated and you don't run in the problem you mentioned as well.

Oh, and I used a <button> so that it's keyboard accessible as well ;)

Cheers!

Sander

2012/9/28 HQJaTu <ja...@hqcodeshop.fi>

A typical example of HTML/JavaScript -mix is an onclick-handler asking for confirmation:
<img onclick="if (confirm('Really delete?')) deleteThis(this);" />

To add complexity, in the question there may be a name/description of actual item to be deleted:
<img onclick="if (confirm('Really delete FooBar?')) deleteThis(this);" />

Yet another layer of complexity, do the same thing with a Smarty-template:
<?php
$item = "FooBar";
?>
<img onclick="if (confirm('Really delete {$item}?')) deleteThis(this);" />

On this round we're assuming that item has originally been entered by an user. Some sort of escaping technique is required:
<?php
$item = "Foo 'Bar' with a nasty HTML-tag <b>";
?>
<img onclick="if (confirm('Really delete {$item|escape:javascript}?')) deleteThis(this);" />

Worst case scenario:
<?php
$item = "Foo 'Bar' with a "nasty" HTML-tag <b>";
?>

This won't work, the double quote character breaks onclick="" attribute:
<img onclick="if (confirm('Really delete {$item|escape:javascript}?')) deleteThis(this);" />

This won't work, the single quote is not properly parsed for JavaScript:
<img onclick="if (confirm('Really delete {$item|escape:htmlall}?')) deleteThis(this);" />


To make this work, I'm suggesting a new escape-modifier htmljs, it runs PHP addslashes() to keep JavaScript-parser happy, but behaves exactly like htmlall after that. See my attached patch-file for details.

Now this will work:
<img onclick="if (confirm('Really delete {$item|escape:htmljs}?')) deleteThis(this);" />


I know that there are a number of ways to circumvent the original problem. But what if I'm just like any other programmer and don't want to go trough all the trouble. A simple solution is always better and easier to maintain. Escaping user input is so important and I'll accept any help there is to keep my site XSS-free. I'm sure that this new escape modifier will make somebody else happier too.

What do you think?

Regards,
Jari Turkia
--
You received this message because you are subscribed to the Google Groups "Smarty Developers" group.
To view this discussion on the web visit https://groups.google.com/d/msg/smarty-developers/-/rO46gqVoiPYJ.
To post to this group, send email to smarty-d...@googlegroups.com.
To unsubscribe from this group, send email to smarty-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/smarty-developers?hl=en.

[HQJaTu]

In my post I said: "I know that there are a number of ways to circumvent the original problem."
Your suggestion simply is not feasible in all situations. Not that I'm using jQuery for example.

However, I did learn about combining modifiers after my post.
My current solution is to have two modifiers:
<img onclick="if (confirm('Really delete {$item|escape:javascript|escape:htmlall}?')) deleteThis(this);" />

This does exactly what I proposed earlier. So, this might be an addition to documentation.

Regards,
Jari Turkia

[Sander Aarts]

2012/10/1 HQJaTu <ja...@hqcodeshop.fi>

In my post I said: "I know that there are a number of ways to circumvent the original problem."
Your suggestion simply is not feasible in all situations. Not that I'm using jQuery for example.

I only used jQuery because it's condenses the example code. You can use any JavaScript library or none to accomplish this.

I think my suggestion, or something similar, is feasible in most if not all situations. Especially now that we have data-attributes to easily pass extra data.

gr,

Sander

 

To view this discussion on the web visit https://groups.google.com/d/msg/smarty-developers/-/tqxcYRRBbAoJ.

To post to this group, send email to smarty-d...@googlegroups.com.
To unsubscribe from this group, send email to smarty-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/smarty-developers?hl=en.

--

Vriendelijke groeten,

Sander Aarts

Sander Aarts, Front-end Web Developer

Sebastian Webprojecten

Stadhuisplein 345

5038 TH Tilburg

t 013 518 61 20

i www.sebastian.nl