Dave wrote:
On 3/19/07, Allen Gilliland <[EMAIL PROTECTED]> wrote:
I'd like to suggest that we do one more thing to fix this problem
starting in the current trunk. I'd like to go ahead and make our pojo
wrappers static so that we can place custom code in various methods to
handle situations like this. The problem with the current fix is that
it relies on the fact that people are using the macros and that can't be
guaranteed, so to truly solve this problem we need the functionality to
be in the pojo wrappers themselves so that there is no way to get
unescaped data.
So to do this all I am planning to do is copy the current generated
wrappers into the actual source tree and commit them, then modify the
various getXXX() methods on the CommentDataWrapper so that they escape
the data.
Not only does this help fix this security issue at the very root level,
but it will also open up opportunities to do more with our wrappers
general. So is anyone else opposed to making the pojo wrappers static?
I don't think this change would need to be back ported to older
releases, so it would just go into the current trunk.
I'm +0 on this.
It eliminates one reason we need XDoclet, that's good.
It opens opportunities for smarter POJO wrappers, which is good --
but, other than comments security what else can we use that for?
Well, comment security is one place, but there's lots of other places
where we do little things like escape the data for html/xml or other
little formatting changes which can all be applied automatically in the
wrapper so that we don't have to rely on template writers to do that.
That makes templates cleaner and nicer looking and ultimately more
usable IMO.
Plugins are another good example. I think I had mentioned this before,
but IMO when you call $entry.text in your template you should get back
the fully formatted entry body rather than the original text. There
would be various places that we can make use of functionality like this.
And a third option is that we can then add methods which only exist in
the wrappers but not in the pojo, which makes sense in some situations.
It adds more code to maintain and more code to update when a new field
or POJO is added, but I guess with a good IDE that work is negligible.
I agree that this means a little more work, but I also think this is
good work. We don't change the wrapped methods very often so I don't
think the work is going to be very much, and doing this manually is more
secure. For example I believe at one point I noticed that some of the
pojo getId() methods were wrapped and really they shouldn't have been
because there is no use for them and it's most likely that was just
copied code from somewhere and nobody noticed. So doing this manually
makes some sense to me because we are forced to make sure that only the
data we want available to users is exposed and it's less likely that
data would exposed accidentally.
-- Allen
So unless somebody else has some good solid objections, go for it.
- Dave
