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 >
