[
https://issues.apache.org/jira/browse/WW-5124?focusedWorklogId=610552&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-610552
]
ASF GitHub Bot logged work on WW-5124:
--------------------------------------
Author: ASF GitHub Bot
Created on: 14/Jun/21 08:05
Start Date: 14/Jun/21 08:05
Worklog Time Spent: 10m
Work Description: JCgH4164838Gh792C124B5 commented on pull request #489:
URL: https://github.com/apache/struts/pull/489#issuecomment-860077589
Hi @yasserzamani .
Thanks for taking a look, and asking some good questions, which made me
review and re-think things. 😄
Apologies in advance for the long response ...
Unfortunately, from what I could find, performing state cleanup (or
initialization) for custom JSP tags seems to rely a lot on "null setting" to
ensure explicit consistency of state. It looks ugly (and is ugly to
implement), but seems to be the only way to handle the situation.
In terms of specifications and references, there was various material that I
came across:
- There is the JSP 2.0 specification
(https://jcp.org/aboutJava/communityprocess/final/jsr152/index.html), which
talks about JSP tag handlers and the JSP lifecycle.
- There is the package javax.servlet.jsp.tagext Description
(https://docs.oracle.com/javaee/7/api/javax/servlet/jsp/tagext/package-summary.html).
It provides an overview and general background of the JSP Tag Libraries. The
section on "Tag Handler as a Container-Managed Object" briefly mentions Tag
Pooling.
- There is the JSP Tag interface release method
(https://docs.oracle.com/javaee/7/api/javax/servlet/jsp/tagext/Tag.html#release--).
It mentions that `doStartTag()` and `doEndTag()` can be invoked multiple
times before the JSP page implementation (container) calls `release()`. So if
a container uses tag pooling, there is no guarantee as to when it `release()`
might be invoked by the container. That appears to make `doEndTag()` the only
choice for cleaning up state between potential calls to the same Tag object
instance ... but it does not make it clear if it is "_correct_" or safe to do
so.
- There are also some SO posts that mention using `doEndTag()` to perform
state cleanup
(https://stackoverflow.com/questions/2481159/how-to-prevent-jsp-tags-from-being-reused-after-being-classloaded
and https://stackoverflow.com/questions/616064/jsp-tag-lifecycle).
- There is a blog on the topic of memory leaks with JSP Tag Pooling
(https://blog.codecentric.de/en/2009/08/jsp-tag-pooling-memory-leaks-2/), that
shows using `doEndTag()` to clean up references as well.
- There is an old Tomcat Bugzilla
(https://bz.apache.org/bugzilla/show_bug.cgi?id=33934) which **argues against**
cleaning up state in `doEndTag()`, and instead only do so in the `release()`
method. The issue appears to be that if a tag is reused on a page with the
same attribute set, the container is not required to invoke setters for
attributes that have the same value - which can result in incorrect behaviour
if the tag itself has cleared them (such as cleanup in `doEndTag()`).
There are seem to be different opinions on how to "properly" handle JSP Tag
state cleanup when Tag Pooling is involved, but unfortunately I could not find
any "crystal clear" answer on the topic.
Attempting a solution like in this PR (clearing state in `doEndTag()`)
appears to resolve the specific JIRA issue itself, but could potentially
introduce unexpected / incorrect behaviour in other circumstances (such as if
the container determines it does not need to call a setter again due to the
attribute being the same on the same page). So **we should consider it
carefully** before moving forward with such a change, as it may "_break more
than it fixes_".
We could certainly modify the PR to switch to performing the state cleanup
in the `release()` method, which might be the **safer** move overall. It would
still have _some_ benefits, even if that choice might not resolve the reported
JIRA issue itself.
What do you (and others) think ?
However, if only `release()` cleanup is implemented, I think the reported
issue seen running in a JSP container with Tag Pooling enabled would **still**
occur (with old values from previous tag instance usage affecting subsequent
usage).
I suppose the question is _whether there is **any** way to reconcile the JSP
2.0 specification 2-54 statement_ (in the "Methods" section): "_After the
doEndTag invocation, the tag handler is available for further invocations (and
it is expected to have retained its properties)_", **and** _the weird behaviour
that can happen with Tag Pooling if the tag handler instance's properties are
retained in a subsequent usage_. So maybe the weird behaviour is "normal", due
to the requirements of the JSP specification ?
The only **other option** I can think of, would be to perform state cleanup
only in `release()` (to be consistent with the absolute wording of the JSP
specification), but provide a _new tag attribute_ that would allow users to
choose to reset tag state in cases of Tag Pooling. Something like a `boolean`
"resetTagStateAfterUse" ... it might suffer side effects of its own due to Tag
Pooling, but at least the user would have _opted-in_ to potentially breaking
activity.
After the above review, I am leaning towards refactoring the PR to move
cleanup to happen only via the `release()` method, and then possibly looking at
introducing a "resetTagStateAfterUse" attribute to optionally allow some
cleanup in the `doEndTag()` method.
Please let me know what you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 610552)
Time Spent: 1h 10m (was: 1h)
> Tag attribute values cached
> ---------------------------
>
> Key: WW-5124
> URL: https://issues.apache.org/jira/browse/WW-5124
> Project: Struts 2
> Issue Type: Bug
> Environment: Here you have a repo that reproduce this:
> https://github.com/dfliess/struts2-tagpooling-bug
> Reporter: Diego Alejandro Fliess
> Priority: Major
> Fix For: 2.5.27, 2.6
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> On some application servers, like glassfish, when handling jsp tag pooling,
> attribute values are cached or not reinitiallized.
> For example:
> {code:java}
> <s:textfield size="50">
> <jsp:attribute name="placeholder">THIS TEXTFIELD SHOULD HAVE
> VALUE</jsp:attribute>
> <jsp:attribute name="value">VALUE</jsp:attribute>
> </s:textfield>
> <s:textfieldsize="50">
> <jsp:attribute name="placeholder">THIS TEXT FIELD SHOULDN'T HAVE
> VALUE</jsp:attribute>
> </s:textfield>{code}
> Â
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
