You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@roller.apache.org by Allen Gilliland <Al...@Sun.COM> on 2006/01/16 03:56:39 UTC

Roller formbeans

would anyone object to the idea of removing the xdoclet generated
formbeans and instead just maintain the formbeans manually?  I find that
using the xdoclet generated formbeans is a bit confusing and at times
problematic.

For one, xdoclet seems to generate some formbeans which aren't actually
used, like PermissionsForm, ObjectAuditForm, and RoleForm.

Secondly, most of the xdoclet generated formbeans need to be extended
anyways because there isn't a 1-to-1 mapping between our pojo attributes
and the fields we need on the web forms.  I think having the "Ex"
formbeans just makes things more confusing.

Thirdly, the xdoclet generated formbeans always modify all primitive and
basic attribute types or a pojo even if those attributes shouldn't be
changed.  This coupled with the way we use the copyTo() and copyFrom()
methods is a bit lazy and insecure.  We were recently bitten by this one
on the profile page, where a malicious user could access and change
attributes that they shouldn't be able to.

I don't think we have enough formbeans to really make this
unreasonable.  I think this would only affect 10 or 12 formbeans, which
isn't too bad.

-- Allen


Re: Roller formbeans

Posted by Allen Gilliland <Al...@Sun.COM>.
I actually don't mind formbeans entirely, but I don't like how we autogenerate them.

As far as the StrutsAction2 stuff goes, that just looks way to preliminary to be useful anytime soon.  Definitely something to keep an eye on and hopefully Ted will keep us up to date on what's happening.

-- Allen


On Wed, 2006-01-25 at 14:29, Matt Raible wrote:
> +1 for migrating to Struts Action 2 and getting rid of all these silly
> form beans. ;-)
> 
> http://wiki.apache.org/struts/StrutsAction2
> 
> Matt
> 
> On 1/15/06, Allen Gilliland <Al...@sun.com> wrote:
> > On Sun, 2006-01-15 at 20:17, David M Johnson wrote:
> > > On Jan 15, 2006, at 9:56 PM, Allen Gilliland wrote:
> > > > would anyone object to the idea of removing the xdoclet generated
> > > > formbeans and instead just maintain the formbeans manually?  I find
> > > > that
> > > > using the xdoclet generated formbeans is a bit confusing and at times
> > > > problematic.
> > >
> > > -1
> > >
> > > I don't see a the problem(s) here. Below are my counter arguments.
> >
> > somehow i figured you'd say that.
> >
> > >
> > > > For one, xdoclet seems to generate some formbeans which aren't
> > > > actually
> > > > used, like PermissionsForm, ObjectAuditForm, and RoleForm.
> > >
> > > That's easy to fix, just leave the @struts.form tag off of the POJO
> > > and no
> > > form will be generated.
> >
> > agreed.  i didn't realize that was the way it was controlled.
> >
> > >
> > > > Secondly, most of the xdoclet generated formbeans need to be extended
> > > > anyways because there isn't a 1-to-1 mapping between our pojo
> > > > attributes
> > > > and the fields we need on the web forms.  I think having the "Ex"
> > > > formbeans just makes things more confusing.
> > > >
> > > > Thirdly, the xdoclet generated formbeans always modify all
> > > > primitive and
> > > > basic attribute types or a pojo even if those attributes shouldn't be
> > > > changed.  This coupled with the way we use the copyTo() and copyFrom()
> > > > methods is a bit lazy and insecure.  We were recently bitten by
> > > > this one
> > > > on the profile page, where a malicious user could access and change
> > > > attributes that they shouldn't be able to.
> > >
> > > Laziness is not necessarily a bad thing. Generating the forms reduces
> > > the
> > > chance of errors and the amount of code we have to write and maintain.
> >
> > i don't mind a little bit of laziness, and certainly not when it also
> > reduces the amount of code we have to maintain, but being lazy can also
> > get us in trouble.
> >
> > > I don't think that hand written forms will make is more secure. You
> > > still have to
> > > think carefully about what should be copied.
> >
> > I strongly disagree.  I think that manually written formbeans would
> > force us to be more aware of what attributes of a pojo are allowed to be
> > changed via a form element.  currently we don't do that because the
> > copyTo()/copyFrom() just blindly sets all attributes.
> >
> > I would cite the profile servlet as an example again.  Because of our
> > laziness it was possible for a malicious user to change things in their
> > UserData object which should not even have been exposed at all.  Even
> > now the UserForm.copyTo() method actually calls methods like setId() and
> > setDateCreated() which is silly, those things should never ever be
> > changed.
> >
> > -- Allen
> >
> >
> > >
> > > If you don't like the behavior of the copyTo() and copyFrom()
> > > methods, then
> > > fix them in the struts-form.xdt template that we use to generate the
> > > forms.
> > >
> > > - Dave
> > >
> >
> >


Re: Roller formbeans

Posted by Matt Raible <mr...@gmail.com>.
+1 for migrating to Struts Action 2 and getting rid of all these silly
form beans. ;-)

http://wiki.apache.org/struts/StrutsAction2

Matt

On 1/15/06, Allen Gilliland <Al...@sun.com> wrote:
> On Sun, 2006-01-15 at 20:17, David M Johnson wrote:
> > On Jan 15, 2006, at 9:56 PM, Allen Gilliland wrote:
> > > would anyone object to the idea of removing the xdoclet generated
> > > formbeans and instead just maintain the formbeans manually?  I find
> > > that
> > > using the xdoclet generated formbeans is a bit confusing and at times
> > > problematic.
> >
> > -1
> >
> > I don't see a the problem(s) here. Below are my counter arguments.
>
> somehow i figured you'd say that.
>
> >
> > > For one, xdoclet seems to generate some formbeans which aren't
> > > actually
> > > used, like PermissionsForm, ObjectAuditForm, and RoleForm.
> >
> > That's easy to fix, just leave the @struts.form tag off of the POJO
> > and no
> > form will be generated.
>
> agreed.  i didn't realize that was the way it was controlled.
>
> >
> > > Secondly, most of the xdoclet generated formbeans need to be extended
> > > anyways because there isn't a 1-to-1 mapping between our pojo
> > > attributes
> > > and the fields we need on the web forms.  I think having the "Ex"
> > > formbeans just makes things more confusing.
> > >
> > > Thirdly, the xdoclet generated formbeans always modify all
> > > primitive and
> > > basic attribute types or a pojo even if those attributes shouldn't be
> > > changed.  This coupled with the way we use the copyTo() and copyFrom()
> > > methods is a bit lazy and insecure.  We were recently bitten by
> > > this one
> > > on the profile page, where a malicious user could access and change
> > > attributes that they shouldn't be able to.
> >
> > Laziness is not necessarily a bad thing. Generating the forms reduces
> > the
> > chance of errors and the amount of code we have to write and maintain.
>
> i don't mind a little bit of laziness, and certainly not when it also
> reduces the amount of code we have to maintain, but being lazy can also
> get us in trouble.
>
> > I don't think that hand written forms will make is more secure. You
> > still have to
> > think carefully about what should be copied.
>
> I strongly disagree.  I think that manually written formbeans would
> force us to be more aware of what attributes of a pojo are allowed to be
> changed via a form element.  currently we don't do that because the
> copyTo()/copyFrom() just blindly sets all attributes.
>
> I would cite the profile servlet as an example again.  Because of our
> laziness it was possible for a malicious user to change things in their
> UserData object which should not even have been exposed at all.  Even
> now the UserForm.copyTo() method actually calls methods like setId() and
> setDateCreated() which is silly, those things should never ever be
> changed.
>
> -- Allen
>
>
> >
> > If you don't like the behavior of the copyTo() and copyFrom()
> > methods, then
> > fix them in the struts-form.xdt template that we use to generate the
> > forms.
> >
> > - Dave
> >
>
>

Re: Roller formbeans

Posted by Allen Gilliland <Al...@Sun.COM>.
On Sun, 2006-01-15 at 20:17, David M Johnson wrote:
> On Jan 15, 2006, at 9:56 PM, Allen Gilliland wrote:
> > would anyone object to the idea of removing the xdoclet generated
> > formbeans and instead just maintain the formbeans manually?  I find  
> > that
> > using the xdoclet generated formbeans is a bit confusing and at times
> > problematic.
> 
> -1
> 
> I don't see a the problem(s) here. Below are my counter arguments.

somehow i figured you'd say that.

> 
> > For one, xdoclet seems to generate some formbeans which aren't  
> > actually
> > used, like PermissionsForm, ObjectAuditForm, and RoleForm.
> 
> That's easy to fix, just leave the @struts.form tag off of the POJO  
> and no
> form will be generated.

agreed.  i didn't realize that was the way it was controlled.

> 
> > Secondly, most of the xdoclet generated formbeans need to be extended
> > anyways because there isn't a 1-to-1 mapping between our pojo  
> > attributes
> > and the fields we need on the web forms.  I think having the "Ex"
> > formbeans just makes things more confusing.
> >
> > Thirdly, the xdoclet generated formbeans always modify all  
> > primitive and
> > basic attribute types or a pojo even if those attributes shouldn't be
> > changed.  This coupled with the way we use the copyTo() and copyFrom()
> > methods is a bit lazy and insecure.  We were recently bitten by  
> > this one
> > on the profile page, where a malicious user could access and change
> > attributes that they shouldn't be able to.
> 
> Laziness is not necessarily a bad thing. Generating the forms reduces  
> the
> chance of errors and the amount of code we have to write and maintain.

i don't mind a little bit of laziness, and certainly not when it also
reduces the amount of code we have to maintain, but being lazy can also
get us in trouble.

> I don't think that hand written forms will make is more secure. You  
> still have to
> think carefully about what should be copied.

I strongly disagree.  I think that manually written formbeans would
force us to be more aware of what attributes of a pojo are allowed to be
changed via a form element.  currently we don't do that because the
copyTo()/copyFrom() just blindly sets all attributes.

I would cite the profile servlet as an example again.  Because of our
laziness it was possible for a malicious user to change things in their
UserData object which should not even have been exposed at all.  Even
now the UserForm.copyTo() method actually calls methods like setId() and
setDateCreated() which is silly, those things should never ever be
changed.

-- Allen


> 
> If you don't like the behavior of the copyTo() and copyFrom()  
> methods, then
> fix them in the struts-form.xdt template that we use to generate the  
> forms.
> 
> - Dave
> 


Re: Roller formbeans

Posted by David M Johnson <Da...@Sun.COM>.
On Jan 15, 2006, at 9:56 PM, Allen Gilliland wrote:
> would anyone object to the idea of removing the xdoclet generated
> formbeans and instead just maintain the formbeans manually?  I find  
> that
> using the xdoclet generated formbeans is a bit confusing and at times
> problematic.

-1

I don't see a the problem(s) here. Below are my counter arguments.

> For one, xdoclet seems to generate some formbeans which aren't  
> actually
> used, like PermissionsForm, ObjectAuditForm, and RoleForm.

That's easy to fix, just leave the @struts.form tag off of the POJO  
and no
form will be generated.

> Secondly, most of the xdoclet generated formbeans need to be extended
> anyways because there isn't a 1-to-1 mapping between our pojo  
> attributes
> and the fields we need on the web forms.  I think having the "Ex"
> formbeans just makes things more confusing.
>
> Thirdly, the xdoclet generated formbeans always modify all  
> primitive and
> basic attribute types or a pojo even if those attributes shouldn't be
> changed.  This coupled with the way we use the copyTo() and copyFrom()
> methods is a bit lazy and insecure.  We were recently bitten by  
> this one
> on the profile page, where a malicious user could access and change
> attributes that they shouldn't be able to.

Laziness is not necessarily a bad thing. Generating the forms reduces  
the
chance of errors and the amount of code we have to write and maintain.

I don't think that hand written forms will make is more secure. You  
still have to
think carefully about what should be copied.

If you don't like the behavior of the copyTo() and copyFrom()  
methods, then
fix them in the struts-form.xdt template that we use to generate the  
forms.

- Dave