_TemplatedMixin should fully escape html entities

[gbranchaud]

The _TemplatedMixin does some basic escaping for properties that are not prefixed by a "!" (i.e. "${foo}" will be escaped, but not "${!foo}").

The current escaping logic simply replaces double-quotes by their equivalent html entity. To me this is not sufficient.

I would like support for replacing all characters into their html entity (dojox/html/entities does this) or at least for the ​basic 4-5 ones recommended by OWASP.

Attached to this case is a basic test case illustrating the desired behavior.

Note: this might be considered a change too breaking to be implemented. In that case, please consider making the escaping logic easily user-extendable in _TemplatedMixin (with a protected escapeValue(value) method, maybe?) so that custom widgets can extend a single method with more complex escaping.

[gbranchaud]

simple test case showing desired behavior

[bill]

Replying to gbranchaud:

The current escaping logic simply replaces double-quotes by their equivalent html entity. To me this is not sufficient.

Yah, I think the code is designed for attributes, for example a template like:

<div id="${id}"></div>

... although that's not needed nowadays since you can achieve the same functionality in your prototype:

_setIdAttr: "domNode"

The code also mentions it's for attributes:

// Safer substitution, see heading "Attribute values" in
// http://www.w3.org/TR/REC-html40/appendix/notes.html#h-B.3.2
value.toString().replace(/"/g,"""); //TODO: add &amp? use encodeXML method?

I agree that it should work for innerHTML too, and replacing the 5 entities in that link (" ' < > &) seems reasonable. OTOH _TemplatedMixin will probably be replaced completely in 2.0.

But I patched it anyway. Try ​https://github.com/dojo/dijit/pull/1 and tell me if it works for you. BTW, turns out that we already had a test case for this but it wasn't catching the error.

[bill]

I'll check in the change to have a separate (overridable) methods that escapes < > ' " &.

[gbranchaud]

I tested rev ​3365a6d... with our app and it works just like I expected. Thanks a lot for the work!

[cjolif]

seems it has been checked in? If confirmed shouldn't this be closed?

[bill]

Yes, fixed in 3365a6d, for some reason this ticket didn't automatically close.

[Bill Keese <bill@…>]

In 5f94249123a5e434b01fb3ba897c855af69bbf76/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Bill Keese <bill@…>]

In f65925debca2997f5d8cd347f7b552ae133fff20/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled?