Bug 49526 - c:set does not remove variables from ELContext VariableMapper
Summary: c:set does not remove variables from ELContext VariableMapper
Alias: None
Product: Taglibs
Classification: Unclassified
Component: Standard Taglib (show other bugs)
Version: 1.2.0
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
Depends on:
Reported: 2010-06-29 23:28 UTC by Jeremy Boynes
Modified: 2010-07-03 03:37 UTC (History)
0 users

Patch to remove variable from VariableMapper if page scope (1.13 KB, patch)
2010-06-29 23:48 UTC, Jeremy Boynes
Details | Diff
Test case that verifies the mapper variable is handled correctly (5.43 KB, patch)
2010-07-02 23:10 UTC, Jeremy Boynes
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Boynes 2010-06-29 23:28:11 UTC
The sequence
  <c:set var="hello" value="hello"/>
  <c:set var="x" value="#{hello}"/>
  <c:set var="x" value="world"/>
  <c:out value="${x}"/>

generates "hello" rather than "world"

The commented out code at SetSupport#138 appears to be needed.
Comment 1 Jeremy Boynes 2010-06-29 23:30:08 UTC
That code always removed any expression from the VariableMapper. Per the question, it looks like it should only do that if variable is in page scope; setting in request scope, for example, should leave the original ValueExpression bound.
Comment 2 Jeremy Boynes 2010-06-29 23:48:43 UTC
Created attachment 25665 [details]
Patch to remove variable from VariableMapper if page scope
Comment 3 Henri Yandell 2010-07-02 21:04:47 UTC
Seems good. Need to write a test for this I think; fits the Cactus test nicely.

I've updated the standard-test package so tests are passing; though the maven-jetty-plugin appears to give an error after tests pass [yet doesn't when tests fail].
Comment 4 Jeremy Boynes 2010-07-02 22:07:47 UTC
Thanks. I'm not getting an error per se, but I am getting a warning:
2010-07-02 18:10:06.977:org.apache.jasper.runtime.PageContextImpl:WARN:  Internal error flushing the buffer in release()

Testing this does not actually need the container support Cactus provides; an alternative would be to use something like EasyMock and add a unit test within standard-impl itself. That would also allow us to verify all the interactions with the container.
Comment 5 Jeremy Boynes 2010-07-02 23:10:59 UTC
Created attachment 25687 [details]
Test case that verifies the mapper variable is handled correctly

Also updates to junit 4.8.1 and adds EasyMock dependency to pom.
EasyMock uses Apache 2.0 license.
Comment 6 Henri Yandell 2010-07-03 03:37:22 UTC
Thanks - both patches applied.

svn ci -m "Implementing commented out code - c:set now removes variables from ELContext VariableMapper. Test and fix patches from Jeremy Boynes #49526"
Sending        impl/pom.xml
Sending        impl/src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java
Adding         impl/src/test/java/org/apache/taglibs/standard/tag/common
Adding         impl/src/test/java/org/apache/taglibs/standard/tag/common/core
Adding         impl/src/test/java/org/apache/taglibs/standard/tag/common/core/TestSetSupport.java
Transmitting file data ...
Committed revision 960166.