You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2014/03/14 20:53:26 UTC
[Bug 56265] New: Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
Bug ID: 56265
Summary: Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Product: Tomcat 7
Version: 7.0.52
Hardware: PC
Status: NEW
Severity: normal
Priority: P2
Component: Jasper
Assignee: dev@tomcat.apache.org
Reporter: knst.kolinko@gmail.com
The following was reported on the users list,
"double xmlEscape in dynamic attributes in 7.0.52"
http://marc.info/?t=139479709800007&r=1&w=2
[quote]
Hi,
I have several custom jspx tags with dynamic attributes that worked well up to
Tomcat 7.0.47, but they do not work properly on Tomcat 7.0.52. Same problems
occur also when using Spring form tags (I suspect that other libraries would
have same problem, but I didn't test them).
sample (data-test[2] is dynamic attribute, onclick is static):
<c:set var="world" value="'World'"></c:set>
<sf:form onclick="window.alert('Hello ${world}!')"
data-test="window.alert('Hello ${world}!')"
data-test2="window.alert('Hello World!')"
tomcat 7.0.47 output:
<form onclick="window.alert('Hello 'World'!')"
data-test="window.alert('Hello 'World'!')"
data-test2="window.alert('Hello World!')"
tomcat 7.0.52 output:
<form onclick="window.alert('Hello 'World'!')"
data-test="window.alert(&#039;Hello 'World'!&#039;)"
data-test2="window.alert('Hello World!')"
If there is EL used in dynamic attribute (data-test), non-EL part of that
attribute is escaped twice, EL part is escaped only once. Tomcat 7.0.47 would
escape everything just once.
Everything works as before if static attribute is used (onclick) or there is no
EL in dynamic attribute (data-test2).
[/quote]
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
--- Comment #1 from Konstantin Kolinko <kn...@gmail.com> ---
Created attachment 31388
--> https://issues.apache.org/bugzilla/attachment.cgi?id=31388&action=edit
test.war
A simple web application that reproduces this issue.
Steps to reproduce on 7.0.52:
1. Deploy
2. Access http://localhost:8080/test/
3.
Actual:
---
[data-test]: [window.alert('Hello 'World'!')]
[data-test2]: [window.alert('Hello World!')]
---
Expected:
The following rows, in any order:
---
[data-test]: [window.alert('Hello 'World'!')]
[data-test2]: [window.alert('Hello World!')]
---
Tomcat 7.0.42 does not have this issue, produces the "Expected" output.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
--- Comment #10 from Mark Thomas <ma...@apache.org> ---
Fixed in 8.0.x for 8.0.4 onwards.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
Konstantin Kolinko <kn...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
OS| |All
--- Comment #2 from Konstantin Kolinko <kn...@gmail.com> ---
In 7.0.52 the text of index_jspx.java contains:
[[[
private boolean _jspx_meth_my_005fmytag_005f0(javax.servlet.jsp.PageContext
_jspx_page_context)
throws java.lang.Throwable {
javax.servlet.jsp.PageContext pageContext = _jspx_page_context;
javax.servlet.jsp.JspWriter out = _jspx_page_context.getOut();
// my:mytag
org.apache.jsp.tag.webmytag_tagx _jspx_th_my_005fmytag_005f0 = (new
org.apache.jsp.tag.webmytag_tagx());
_jsp_instancemanager.newInstance(_jspx_th_my_005fmytag_005f0);
_jspx_th_my_005fmytag_005f0.setJspContext(_jspx_page_context);
// /index.jspx(27,3) null
_jspx_th_my_005fmytag_005f0.setDynamicAttribute(null, "data-test2",
"window.alert('Hello World!')");
// /index.jspx(27,3) null
_jspx_th_my_005fmytag_005f0.setDynamicAttribute(null, "data-test",
(java.lang.Object)
org.apache.jasper.runtime.PageContextImpl.proprietaryEvaluate("window.alert('Hello
${world}!')", java.lang.Object.class,
(javax.servlet.jsp.PageContext)_jspx_page_context, null, false));
_jspx_th_my_005fmytag_005f0.doTag();
_jsp_instancemanager.destroyInstance(_jspx_th_my_005fmytag_005f0);
return false;
}
]]]
So I wonder why is it "window.alert('Hello ${world}!')" ,
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
Konstantin Kolinko <kn...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|normal |regression
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
--- Comment #9 from Mark Thomas <ma...@apache.org> ---
I've spent the morning reviewing this issue and the various related issues. I'm
now convinced that Konstantin's v2 patch was correct and that the test changes
in the v4 are also correct. I will therefore be applying the v4 patch (with a
few extra comments) shortly and then looking at back-porting it.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #31401|0 |1
is obsolete| |
--- Comment #8 from Mark Thomas <ma...@apache.org> ---
Created attachment 31402
--> https://issues.apache.org/bugzilla/attachment.cgi?id=31402&action=edit
Proposed patch v4
I have updated my previous patch to follow the Validator changes in v2 but
still with the test fixes from v3. I'm not 100% confident this patch is right
yet. I want to spend more time reviewing it but I've put it here in case it is
useful.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #11 from Mark Thomas <ma...@apache.org> ---
This has also been fixed in 7.0.x for 7.0.53 onwards and in 6.0.x for 6.0.40
onwards.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
--- Comment #4 from Konstantin Kolinko <kn...@gmail.com> ---
Created attachment 31390
--> https://issues.apache.org/bugzilla/attachment.cgi?id=31390&action=edit
2014-03-15_56265_tc8_v1.patch
Patch for this issue.
Essentially it limits the effect of r1539173 to uninterpreted tags only.
There is no JUnit testcase for this issue yet.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
--- Comment #5 from Konstantin Kolinko <kn...@gmail.com> ---
Created attachment 31400
--> https://issues.apache.org/bugzilla/attachment.cgi?id=31400&action=edit
2014-03-18_56265_tc8_v2.patch
A better solution, with tests.
This covers this bug and xmlescaping in attributes of bug 55198.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55198
XmlEscaping in bodies of bug 55198 is not covered, not investigated (nobody
raised that besides me in discussion of bug 55198). Mark had compatibility
concerns.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55198#c6
It is possible to introduce an option to make xmlescaping in attributes
optional (a TODO comment in the patch).
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
--- Comment #7 from Konstantin Kolinko <kn...@gmail.com> ---
(In reply to Mark Thomas from comment #6)
> Created attachment 31401 [details]
> Proposed patch v3
>
> I've been looking at v2 of the patch.
>
> I think I found a couple of bugs in the testBug56265()
> - The method wasn't annotated with @Test so the test did not execute
> - bug56265.jsp assigned a value to a request attribute named 'text' but
> tried to use an attribute named 'world'.
My bad. I have to fix it.
> - The test was expected value [1] to be unescaped but I think it should have
> been expecting it to be escaped
Wrong. The tag bug56265.tagx print the values as
<jsp:text>[${e.key}]: [${e.value}]</jsp:text>
Those are the attribute values how they were provided by XML parser. They are
unescaped ones.
> I've attached v3 of the patch for comment that - assuming my analysis of v2
> is correct - addresses the issues I found and adds a few more tests. The fix
> part of the patch is now a lot simpler.
Wrong. Escaping discussed in bug 55198 shall be applied to template content of
the document - to attributes of Node.UninterpretedTag only (and to textual
content in the bodies if we extend this).
There is no reason to apply xmlescaping in the dozen of other cases when
getAttribute(...) method is called.
There is also "if (pageInfo.isELIgnored())" branch that shall be handled in the
same way as an attribute that does not contain EL expressions. My patch covers
that, though I have not wrote a test case.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
--- Comment #3 from Konstantin Kolinko <kn...@gmail.com> ---
Thus far:
1. It is reproducible with the current trunk
The following line numbers are from debugging the reproducer with current trunk
@1577714.
2. 'setDynamicAttribute(' is written out at
Generator$GenerateVisitor.generateSetters(...) at Generator.java L3098
At this point attrs[i].getValue() is already
"window.alert('Hello ${world}!')"
3. The escaping happens when creating Node.JspAttribute.
That is in Validator$ValidateVisitor.getJspAttribute(...) at Validator.java
L1379
The code there originates from r1539173
Apparently the goal of r1539173 was to apply xml escaping to attributes in
UninterpretedTag nodes, but it was applied to any tag attributes containing EL
expressions.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 56265] Unexpected escaping in the values of dynamic tag
attributes containing EL expressions
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
--- Comment #6 from Mark Thomas <ma...@apache.org> ---
Created attachment 31401
--> https://issues.apache.org/bugzilla/attachment.cgi?id=31401&action=edit
Proposed patch v3
I've been looking at v2 of the patch.
I think I found a couple of bugs in the testBug56265()
- The method wasn't annotated with @Test so the test did not execute
- bug56265.jsp assigned a value to a request attribute named 'text' but tried
to use an attribute named 'world'.
- The test was expected value [1] to be unescaped but I think it should have
been expecting it to be escaped
I've attached v3 of the patch for comment that - assuming my analysis of v2 is
correct - addresses the issues I found and adds a few more tests. The fix part
of the patch is now a lot simpler.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org