Paul Benedict wrote:
Hmm, maybe I don't understand the problem... as I understand it, cancel
essentially has no
effect on anything in Struts unless you manually check for it and act
accordingly, correct? Or
are you saying that everything happens *except* validation?
Correct. Cancel has no effect unless you religiously check for it and do
something with it..
unless you use a DispatchAction in which the "cancelled" method is always invoked.
Right, at least I got that part of it :)
That's the part I guess I'm not clear on... how is validation bypassed? Is that
*ALL* that is
bypassed, or is form population bypassed too?
This is the code from RequestProcessor.processPopulate:
-snip-
According to the code the form is always populated. If the cancel button is in
the request, the
cancel request attribute is added; this is then used in processValidate() to skip validation.
Yes, I see what you mean. Unless someone can point out a reason you
would want it to function that way, then I would agree it's a bug. I
for one (or two I suppose!) would not have expected it to work that way,
and it doesn't seem right to me either.
Try it yourself!! Just add "?org.apache.struts.taglib.html.CANCEL=true" to any
GET URL and your
execute() method will magically be called as if you didn't have any validation
added to your code.
I'll take your word for it :) The code you showed certainly does seem
to bear this out. I did look at processValidate(), and I certainly
concur with your analysis.
Going back to Ted's pointer, if one had that in all their Actions
religiously, than arguably this wouldn't really be a problem. The fact
that the form gets populated without being validated afterwards, while I
would say isn't optimal, probably wouldn't matter much.
But if you forgot that somewhere, then one could say this is a security
hole... for instance, if someone had a given parameter that was used to
select a database table for updating, and part of the form validation
was to ensure it was only one of three acceptable values, then this
clearly could be a security problem (although I'd say the bad
application design was a bigger issue!)
Now that I agree (and I think understand!), the question is how best to
address it... I would think the solution should look something like
this, in RequestProcessor.java:
// Set the cancellation request attribute if appropriate
if ((request.getParameter(Constants.CANCEL_PROPERTY) != null) ||
(request.getParameter(Constants.CANCEL_PROPERTY_X) != null)) {
request.setAttribute(Globals.CANCEL_KEY, Boolean.TRUE);
ActionForward f = mapping.findForward("cancel");
if (f != null) {
doForward(f.getPath(), request, response);
}
}
RequestUtils.populate(form, mapping.getPrefix(), mapping.getSuffix(),
request);
Note the form is only populated AFTER we've determined whether we should
cancel or not. This would also maintain the current behavior, although
arguably that is not desirable. What else would you want to do if there
was no cancel forward defined though? I'm not sure.
Your solution of defining what mappings are cancelable or not would also
do the trick of course, but that seems to me like a bit more work than
it really should be. Arguably, by default you would want to say that
all Actions are either cancelable or not, rather than having to set
something on all mappings. Just my superficial thinking about it though :)
Paul
Frank
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
