On 7/11/07, Allen Gilliland <[EMAIL PROTECTED]> wrote:
Okay, well for some reason I am not getting escaped html even though the
comment wrapper is escaping html from the comment content before
returning it, so that is one thing that needs to be fixed.
The other problem is that there isn't (and hasn't been) a way to get to
option #3 where html is fully allowed. This is what is used to render
comment content in macros in 3.1 ...
#if($config.commentEscapeHtml)
#set($content = $utils.escapeHTML($comment.content))
#else
#set($content = $utils.transformToHTMLSubset($utils.escapeHTML($
comment.content)))
#end
Hmmm... that must have slipped in during XSS fixes. I'd better check
3.1.1 for that issue.
... and that basically means your only options are #1 (no html) or #2
(partial html), which is not really what we are aiming for i don't think.
Then, even if html escaping is working properly for #1, i don't think
that's the right way of providing that functionality. If you don't want
people putting html in comments then we should be rejecting those
comments, not just leaving the html there and escaping it. The problem
with just escaping the html is that 1) it's ugly and if someone puts
lots of html in their comment and it gets escaped then it makes the
comment almost unreadable and 2) it's dangerous because technically if
at some later point in time html escaping isn't applied anymore then
potentially dangerous html in the comment can become active again. So
it makes far more sense to me that if the admin wants to prevent html in
comments then we should reject those comments with an error message.
So, getting back to my original suggestions again I think we can revise
a couple things.
1. I think we still need a new runtime property for "enable html in
comments?" This is kind of like a built in comment validator which if
enabled will reject comments with html in them.
Hmm... When HTML in comments is disabled I'm not sure if we should
reject HTML, strip it or simply escape it as we have been doing.
What if somebody wants to write about the <h1> tag and wants the
comment to display the string <h1> and not have it interpreted as
HTML. If HTML is turned off, why should should we reject <h1>. Why not
just treat it as text by escaping it at display time?
If we had a WYSIWYG comment editor then they could type <h1> and it
would appear as <h1>, but since we don't they have to type <h1>
which is bit of a pain. Then again, if somebody is writing about HTML
elements then they're probably smart enough to know how to escape
them.
Not sure what the right thing is to do here, but we need to be careful
about making such a significant change in comment behavior.
2. For doing the html subset we can basically leave it how it is now as
a comment plugin and what it will do is the same thing it does now, just
escape all html then unescape the supported tags. So the expected use
case here would be if the user checked the "enable html in comments?"
box to allow for html in their comments, then checked the HTML Subset
plugin which transforms the comment contents so that only certain html
tags are left unescaped.
Yes, that sounds good.
- Dave
