search

gradus / coffeecup

keeping the project alive with this clone of mauricemach/coffeekup
MIT License
190 stars 40 forks source link

Attribute content should auto-escape #71

Open tremby opened 11 years ago

tremby commented 11 years ago

I could be wrong about this, but I can't think of any situation where a person would not want to escape what goes into an HTML attribute.

Doing, for instance, span 'data-equation': 'x<22', "x is less than 22" leads to HTML of dubious validity (but it will probably still render properly).

Trying span 'data-outcome': 'She said "yes"', "He asked if she wanted to" will render as <span data-outcome="She said "yes"">He asked if she wanted to</span>, which is definitely invalid. So when there is arbitrary data going into an attribute I have to add the h() helper around the value every time, which is not so neat.

I'd say this should go for attribute names and the ID and class values too, even though the characters which would be escaped are likely not ever valid in attribute, ID and class names. Better to throw warnings or end up with invalid ID/class names than to end up with invalid HTML.

Even if you don't agree that they should always be escaped (I suppose if you happen to have the strings in already-escaped format you might not want to), it should at least be an option.

tremby commented 11 years ago

Ah, on reading the code in more detail I see that the autoescape option is in fact auto-escaping attribute values as well as element content.

So I suppose that makes this mostly null, and all I'm asking for now is a separate autoescape option for attribute values (I always want to autoescape them -- as explained above I can't see any valid use cases for not doing so -- and I always want to handle element content on a case by case basis), and for you to consider escaping attribute names and ID/class values too.