> > Interestingly, the doc for the cancel tag *does* say that validate() > won't be called and that the Action will be called normally. I never > noticed this before. So, at least no one can claim this behavior isn't > documented :)
Yes, but if you don't want to use the cancel tag you probably don't read its doc. And if you don't use it you wont put the if (isCancelled(request) ) check in your execute. And this is exactly the case when you could get in trouble ... There's really two problems here... one revolves around how this feature > maybe should have been designed in the first place. Tamas may be right > in that regard. The second problem though is you have apps built with > it the way it is now, for better or worse, and breaking those to close > this hole isn't really the best idea. Yes, it's very easy to criticize something after it was implemented. But I admit a solution that isn't backward compatible isn't viable. With that in mind, I'm thinking something along the lines of Paul's > original suggestion may in fact be the best... add a "cancelable" > attribute to the <action> element in config. Default to true. When set > to false and the Action is called, that's the "hacker" case we want to > avoid... maybe throw an IllegalStateException? Maybe look for an > "illegalCancel" forward? Not sure what's best, but the point is to > avoid calling execute() in that case. This would maintain the existing > behavior by default too. > > No, actually, better yet, as Paul suggested, add cancelable to the > <processor> element as well. I think you really need it to be on both > <processor> and <action> though... set it to false on the <processor> to > globally close the security hole, then set it to true on the <action>'s > where it applies. Well this seems to be backward compatible. But you will still have to add tha cancelable= "true" for all the mappings that can be cancelled in your existing applications. Frank Tamas
