Postback Forms - Caching and Modules

• Key: STR-2795
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Niall Pemberton
• Reporter: Niall Pemberton
• Component/s: Tag Libraries
• Affects Version/s: Nightly Build
• Fix Version/s: 1.3.4

Environment

Operating System: other
Platform: Other

Description

I experimented with a postback form in the struts-examples exercise modules and
found two problems:

1) Postback forms take no account of modules so when the FormTag lookup()
method calls TagUtils.getActionMappingName() it doesn't find the action.

2) More seriously, postback forms don't take into account the fact that servlet
containers cache Tags. The way "postback" has been implemented, by setting
the "action" attribute, is not going to work properly when the FormTag is
cached and re-used on a different form.

Comments

[Don Brown]

Definitely needs to be looked at before the next release. Any solutions?

[Don Brown]

After looking further, I guess I don't see the problem:

1) getActionMappingName() is only for extracting the action name from a URI and has nothing to do with an Action lookup. Perhaps you meant getActionMappingURI()?

2) The action attribute is cleared on every recycle, so I don't see how this will affect a reuse

[Niall Pemberton]

OK its a while since I looked at this, but....

1) I agree this doesn't make sense - but TagUtils.getActionMappingName() doesn't remove the module prefix so if you have a "module" uri - something like "/myModule/myAction.do" then when it tries to lookup the ActionMapping it uses "/myModule/myAction" rather than "/myAction" when it calls moduleConfig.findActionConfig() and the mapping is not found.

2) A tag's attributes/properties are managed by the container and shouldn't be modified - the only places the action attribute is cleared is in the release() method - but there is no guarantee this gets called between every use of the tag - since its up to the container the pooling strategy is uses, when the properties are set and when release is called - see the java docs for the release method:

   http://java.sun.com/j2ee/1.4/docs/api/javax/servlet/jsp/tagext/Tag.html#release()

I tried this on Tomcat and got the wrong url rendered from a cached instance of the Tag.

[Don Brown]

As to your points:

1) True, but I don't see any way around this. Sure, we could check for the current module, but otherwise, we'd have to iterate through all the modules to detect another one, and that isn't feasible I don't think

2) Well, if the container doesn't pool properly, that is out of our control, and more tags than this one will have problems.

I'm resolving this as I can't reproduce the issue, but please reopen if you can think of ways we can write this more defensively.

[Niall Pemberton]

The problem is not with container pooling, but in the way that "postback forms" have been implemented in this tag - the way the tag's "action" attribute is modified is incorrect.

On the issue of the modules - it will only ever be the current module - so iterating through other modules would not be necessary.

Closing this is a mistake IMO - the only way this issue is not going to bite people is if the "postback forms" feature is never used.

[Don Brown]

Could you say exactly why the current way is bad and how it should be resolved. I'm pretty new to tags so I'm obviously missing something :)

[Niall Pemberton]

The reason the current way is bad is because containers think they control setting the tags attributes and are free to implement whatever tag pooling strategy they think best. In the case of Tomcat it caches and re-use tags with the same attribute settings - without calling the property setters or the reset() method. As postback forms set the "action" attribute to the current uri, this means that if the tag is reused that incorrect uri gets rendered. This is easily re-created in Tomcat - which I did in the struts-examples webapp while testing the patch I'm about to commit.

I'm re-opening this and will commit a patch for this bug shortly.

[Niall Pemberton]

Fixed in revision 405216.

http://svn.apache.org/viewcvs?rev=405216&view=rev