Dave wrote:
On 7/9/07, Allen Gilliland <[EMAIL PROTECTED]> wrote:
i am looking at some ways to improve the comment system and provide
greater flexibility in how comment content is handled when it's
submitted, particularly in the realm of html vs. non-html comment
posting. xss is becoming so common now that allowing anonymous users to
post html comments is pretty dangerous and so I am trying to find some
better ways to allow for comment formatting to take place on non-html
comments.
after looking through what we have right now i think we need to fix a
couple issues. the main problem is with the current autoformat and
escape html comment settings.
1. these settings are global settings and apply to all comments on all
blogs, however they are not tracked on a per comment basis and this
makes it nearly impossible to change these settings without messing up
old comments. for example, if you start your site by allowing html in
comments, then disable html and enable autoformatting then it's going to
apply that to all your old comments which don't need it and make things
look pretty ugly :/ same situation applies in reverse as well. if you
start by using autoformatting and then disable it then all your old
comments are going to show up unformatted.
Yes. That's a pretty serious problem.
2. these settings are applied in velocity templates and macros and are
not enforced in any way, so it would be easy for a savvy user to
circumvent these settings if they really wanted too. it is more
appropriate if we apply any plugins or transformations inside the code
so that we aren't relying on logic in macros/templates.
I'm not so worried about blog owners circumventing the settings, but I
agree that we should move away from logic in macros/templates wherever
possible.
So I think we need to fix these things first, then I'd like to add a
more flexible way of applying custom formatting options to comments. #2
is very easy to fix since we can just put that logic in the comment
wrapper class. fixing #1 is more tricky.
For #1 I would like to introduce the concept of pluggable
CommentFormatter classes. Basically, these would work exactly like
CommentValidators except that instead of just checking the comment for
some kind of illegal content they would actual be performing
transformations on the content, much like our weblog entry plugins do
now. So site admins would be able to define what comment formatters are
available to the application and enable/disable them globally to make
them apply to comments. We would track what formatters to apply to a
given comment by adding a new column in the db for 'formatters' and
listing which formatters to apply to each comment.
We would start by defining formatters to replace our current settings
for autoformatting and escaping html and via a simple upgrade we can set
these formatters to all existing comments if those settings are enabled.
Then moving forward it's very easy to add formatters to the application
and enable/disable them whenever the admin wants. Since the formatters
are tracked with the comments it is possible to disable a formatter from
being used on new comments but have it still available to apply to old
comments. This also provides a very simple way for site admins to
create custom formatters to suit whatever their needs may be, for
example you could do a wiki syntax comment formatter if you wanted.
I think you are on the right track. Are you going to propose this for
4.0 or later?
I would like to get it done for 4.0 because it's one of the things that
we want to get fixed up on our site and I already have the work done in
my workspace. I think the work is small enough that it doesn't really
need a full proposal, this is the list of changes and additions ...
1. Added new column 'plugins' to roller_comment table, varchar(255).
2. Added new attribute 'plugins' to WeblogEntryComment pojo, including
mapping elements in jpa and hibernate mapping files.
3. Added new interface WeblogEntryCommentPlugin in
business.plugins.comment package. Interface has 1 method,
render(comment) which returns a string of the transformed content.
5. Added 1 new comment plugin, AutoformatPlugin which takes plain text
and formats it using html. This is basically a copy of the convert
linebreaks entry plugin as far as functionality goes.
6. Added 2 new methods to PluginManager and PluginManagerImpl.
applyCommentPlugins(comment) which returns a string, and
getCommentPlugins() which returns the list of plugins.
7. Added new property 'comments.plugins.classnames' in roller.properties
which lists the configured comment plugin classes.
8. Added new runtime property 'users.comments.plugins' which is a comma
separated list of the plugins which are enabled.
9. Modified CommentServlet to basically call
comment.setPlugins(runtimeConfig.getProp('users.comments.plugins')).
This basically applies whatever comment plugins are configured to the
specific comment as it gets saved.
10. Modified Comment pojo wrapper so that the call to getContent()
actually applies plugins before returning output.
11. Unit test for AutoformatPlugin.
Things that I still need to do ...
* Add to the current upgrade process to set roller_comment.plugins =
'AutoFormat Plugin' if the installation is currently using comment
autoformatting.
* Add a way for site admins to enable/disable comment plugins via the
global settings page.
* Cleanup current comment macros and move logic out of macro and into
pojo wrapper. We already to html escaping in the pojo wrapper, but we
also need to do a couple other things.
That's it. Pretty straight forward really.
-- Allen
- Dave