You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2014/04/29 02:19:34 UTC
svn commit: r1590848 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/jasper/compiler/ test/org/apache/jasper/compiler/
test/webapp-3.0/bug5nnnn/ webapps/docs/
Author: kkolinko
Date: Tue Apr 29 00:19:33 2014
New Revision: 1590848
URL: http://svn.apache.org/r1590848
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56334
Merged r1590842 from tomcat/trunk:
Additional tests and fixes
Includes the following:
1. Allow '\' in xmlns attributes of UninterpretedTag. A test added.
(Java escaping was missing. Xml-escaping is still missing. I think it is unlikely that anybody would use such values for xmlns attributes)
2. Fix interaction between Validator.ValidateVisitor.checkXmlAttributes(CustomTag ..) and getJspAttribute().
- EL expression was parsed twice in both methods. Now I am passing the already parsed EL.
- getJspAttribute() has EL validation code, so reduce duplication
- When calling getJspAttribute() you have to pass original attrs.getValue(i), not the textual value.
3. Fix Validator.ValidateVisitor.XmlEscapeNonELVisitor
- It was not EL-escaping its text. Tests added.
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java
tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Validator.java
tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestParser.java
tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56334.jspx
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1590842
Modified: tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java?rev=1590848&r1=1590847&r2=1590848&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java Tue Apr 29 00:19:33 2014
@@ -255,7 +255,7 @@ public class ELParser {
*
* @return The escaped version of the input
*/
- private static String escapeLiteralExpression(String input,
+ static String escapeLiteralExpression(String input,
boolean isDeferredSyntaxAllowedAsLiteral) {
int len = input.length();
int lastAppend = 0;
@@ -552,10 +552,10 @@ public class ELParser {
}
- protected static class TextBuilder extends ELNode.Visitor {
+ static class TextBuilder extends ELNode.Visitor {
- private final boolean isDeferredSyntaxAllowedAsLiteral;
- protected StringBuilder output = new StringBuilder();
+ protected final boolean isDeferredSyntaxAllowedAsLiteral;
+ protected final StringBuilder output = new StringBuilder();
protected TextBuilder(boolean isDeferredSyntaxAllowedAsLiteral) {
this.isDeferredSyntaxAllowedAsLiteral = isDeferredSyntaxAllowedAsLiteral;
Modified: tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Generator.java?rev=1590848&r1=1590847&r2=1590848&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Generator.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Generator.java Tue Apr 29 00:19:33 2014
@@ -1831,7 +1831,7 @@ class Generator {
out.print(attrs.getQName(i));
out.print("=");
out.print(DOUBLE_QUOTE);
- out.print(attrs.getValue(i).replace("\"", """));
+ out.print(escape(attrs.getValue(i).replace("\"", """)));
out.print(DOUBLE_QUOTE);
}
}
@@ -1851,7 +1851,7 @@ class Generator {
out.print(" + \"\\\"");
} else {
out.print(DOUBLE_QUOTE);
- out.print(jspAttrs[i].getValue().replace("\"", """));
+ out.print(escape(jspAttrs[i].getValue().replace("\"", """)));
out.print(DOUBLE_QUOTE);
}
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Validator.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Validator.java?rev=1590848&r1=1590847&r2=1590848&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Validator.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/Validator.java Tue Apr 29 00:19:33 2014
@@ -560,7 +560,7 @@ class Validator {
// request-time expression
throwErrorIfExpression(n, "name", "jsp:param");
n.setValue(getJspAttribute(null, "value", null, null, n
- .getAttributeValue("value"), n, false));
+ .getAttributeValue("value"), n, null, false));
visitBody(n);
}
@@ -579,7 +579,7 @@ class Validator {
JspUtil.checkAttributes("Include action", n, includeActionAttrs,
err);
n.setPage(getJspAttribute(null, "page", null, null, n
- .getAttributeValue("page"), n, false));
+ .getAttributeValue("page"), n, null, false));
visitBody(n);
}
@@ -587,7 +587,7 @@ class Validator {
public void visit(Node.ForwardAction n) throws JasperException {
JspUtil.checkAttributes("Forward", n, forwardActionAttrs, err);
n.setPage(getJspAttribute(null, "page", null, null, n
- .getAttributeValue("page"), n, false));
+ .getAttributeValue("page"), n, null, false));
visitBody(n);
}
@@ -604,7 +604,7 @@ class Validator {
String value = n.getAttributeValue("value");
n.setValue(getJspAttribute(null, "value", null, null, value,
- n, false));
+ n, null, false));
boolean valueSpecified = n.getValue() != null;
@@ -640,7 +640,7 @@ class Validator {
err.jspError(n, "jsp.error.usebean.noSession");
Node.JspAttribute jattr = getJspAttribute(null, "beanName", null,
- null, n.getAttributeValue("beanName"), n, false);
+ null, n.getAttributeValue("beanName"), n, null, false);
n.setBeanName(jattr);
if (className != null && jattr != null)
err.jspError(n, "jsp.error.usebean.notBoth");
@@ -678,11 +678,11 @@ class Validator {
err.jspError(n, "jsp.error.plugin.nocode");
Node.JspAttribute width = getJspAttribute(null, "width", null,
- null, n.getAttributeValue("width"), n, false);
+ null, n.getAttributeValue("width"), n, null, false);
n.setWidth(width);
Node.JspAttribute height = getJspAttribute(null, "height", null,
- null, n.getAttributeValue("height"), n, false);
+ null, n.getAttributeValue("height"), n, null, false);
n.setHeight(height);
visitBody(n);
@@ -692,7 +692,7 @@ class Validator {
public void visit(Node.NamedAttribute n) throws JasperException {
JspUtil.checkAttributes("Attribute", n, attributeAttrs, err);
n.setOmit(getJspAttribute(null, "omit", null, null, n
- .getAttributeValue("omit"), n, false));
+ .getAttributeValue("omit"), n, null, false));
visitBody(n);
}
@@ -771,7 +771,7 @@ class Validator {
}
jspAttrs[i] = getJspAttribute(null, attrs.getQName(i),
attrs.getURI(i), attrs.getLocalName(i), value, n,
- false);
+ null, false);
}
n.setJspAttributes(jspAttrs);
}
@@ -919,12 +919,13 @@ class Validator {
if ("name".equals(attrs.getLocalName(i))) {
n.setNameAttribute(getJspAttribute(null, attrs.getQName(i),
attrs.getURI(i), attrs.getLocalName(i), attrs
- .getValue(i), n, false));
+ .getValue(i), n, null, false));
} else {
if (jspAttrIndex < jspAttrSize) {
- jspAttrs[jspAttrIndex++] = getJspAttribute(null, attrs
- .getQName(i), attrs.getURI(i), attrs
- .getLocalName(i), attrs.getValue(i), n, false);
+ jspAttrs[jspAttrIndex++] = getJspAttribute(null,
+ attrs.getQName(i), attrs.getURI(i),
+ attrs.getLocalName(i), attrs.getValue(i), n,
+ null, false);
}
}
}
@@ -1091,10 +1092,11 @@ class Validator {
pageInfo.isDeferredSyntaxAllowedAsLiteral() ||
libraryVersion < 2.1;
- String attributeValue;
+ String xmlAttributeValue = attrs.getValue(i);
+
ELNode.Nodes el = null;
if (!runtimeExpression && !pageInfo.isELIgnored()) {
- el = ELParser.parse(attrs.getValue(i),
+ el = ELParser.parse(xmlAttributeValue,
deferredSyntaxAllowedAsLiteral);
Iterator<ELNode> nodes = el.iterator();
while (nodes.hasNext()) {
@@ -1116,18 +1118,20 @@ class Validator {
}
}
}
- if (elExpression) {
- attributeValue = attrs.getValue(i);
- } else {
- // Should be a single Text node
- attributeValue = ((ELNode.Text) el.iterator().next()).getText();
- }
- } else {
- attributeValue = attrs.getValue(i);
}
boolean expression = runtimeExpression || elExpression;
+ // When attribute is not an expression,
+ // contains its textual value with \$ and \# escaping removed.
+ String textAttributeValue;
+ if (!elExpression && el != null) {
+ // Should be a single Text node
+ textAttributeValue = ((ELNode.Text) el.iterator().next()).getText();
+ } else {
+ textAttributeValue = xmlAttributeValue;
+ }
+
for (int j = 0; tldAttrs != null && j < tldAttrs.length; j++) {
if (attrs.getLocalName(i).equals(tldAttrs[j].getName())
&& (attrs.getURI(i) == null
@@ -1192,11 +1196,11 @@ class Validator {
Boolean.TYPE == expectedClass ||
expectedClass.isEnum()) {
try {
- expressionFactory.coerceToType(attributeValue, expectedClass);
+ expressionFactory.coerceToType(textAttributeValue, expectedClass);
} catch (Exception e) {
err.jspError
(n, "jsp.error.coerce_to_type",
- tldAttr.getName(), expectedType, attributeValue);
+ tldAttr.getName(), expectedType, textAttributeValue);
}
}
}
@@ -1204,7 +1208,7 @@ class Validator {
jspAttrs[i] = new Node.JspAttribute(tldAttr,
attrs.getQName(i), attrs.getURI(i),
attrs.getLocalName(i),
- attributeValue, false, null, false);
+ textAttributeValue, false, null, false);
} else {
if (deferred && !tldAttr.isDeferredMethod() && !tldAttr.isDeferredValue()) {
@@ -1218,29 +1222,11 @@ class Validator {
tldAttr.getName());
}
- if (elExpression) {
- // El expression
- validateFunctions(el, n);
- jspAttrs[i] = new Node.JspAttribute(tldAttr,
- attrs.getQName(i), attrs.getURI(i),
- attrs.getLocalName(i),
- attributeValue, false, el, false);
- ELContextImpl ctx = new ELContextImpl();
- ctx.setFunctionMapper(getFunctionMapper(el));
- try {
- jspAttrs[i].validateEL(this.pageInfo.getExpressionFactory(), ctx);
- } catch (ELException e) {
- this.err.jspError(n.getStart(),
- "jsp.error.invalid.expression",
- attributeValue, e.toString());
- }
- } else {
- // Runtime expression
- jspAttrs[i] = getJspAttribute(tldAttr,
- attrs.getQName(i), attrs.getURI(i),
- attrs.getLocalName(i),
- attributeValue, n, false);
- }
+ // EL or Runtime expression
+ jspAttrs[i] = getJspAttribute(tldAttr,
+ attrs.getQName(i), attrs.getURI(i),
+ attrs.getLocalName(i),
+ xmlAttributeValue, n, el, false);
}
} else {
@@ -1253,13 +1239,14 @@ class Validator {
jspAttrs[i] = new Node.JspAttribute(tldAttr,
attrs.getQName(i), attrs.getURI(i),
attrs.getLocalName(i),
- attributeValue, false, null, false);
+ textAttributeValue, false, null, false);
}
if (expression) {
tagDataAttrs.put(attrs.getQName(i),
TagData.REQUEST_TIME_VALUE);
} else {
- tagDataAttrs.put(attrs.getQName(i), attributeValue);
+ tagDataAttrs.put(attrs.getQName(i),
+ textAttributeValue);
}
found = true;
break;
@@ -1269,7 +1256,7 @@ class Validator {
if (tagInfo.hasDynamicAttributes()) {
jspAttrs[i] = getJspAttribute(null, attrs.getQName(i),
attrs.getURI(i), attrs.getLocalName(i),
- attributeValue, n, true);
+ xmlAttributeValue, n, el, true);
} else {
err.jspError(n, "jsp.error.bad_attribute", attrs
.getQName(i), n.getLocalName());
@@ -1347,10 +1334,13 @@ class Validator {
* If value is null, checks if there are any NamedAttribute subelements
* in the tree node, and if so, constructs a JspAttribute out of a child
* NamedAttribute node.
+ *
+ * @param el EL expression, if already parsed by the caller (so that we
+ * can skip re-parsing it)
*/
private Node.JspAttribute getJspAttribute(TagAttributeInfo tai,
String qName, String uri, String localName, String value,
- Node n, boolean dynamic)
+ Node n, ELNode.Nodes el, boolean dynamic)
throws JasperException {
Node.JspAttribute result = null;
@@ -1370,7 +1360,6 @@ class Validator {
value.substring(3, value.length() - 2), true, null,
dynamic);
} else {
- ELNode.Nodes el = null;
if (!pageInfo.isELIgnored()) {
// The attribute can contain expressions but is not a
// scriptlet expression; thus, we want to run it through
@@ -1378,12 +1367,18 @@ class Validator {
// validate expression syntax if string contains
// expression(s)
- el = ELParser.parse(value,
+ if (el == null) {
+ el = ELParser.parse(value,
pageInfo.isDeferredSyntaxAllowedAsLiteral());
+ }
if (el.containsEL()) {
validateFunctions(el, n);
} else {
+ // Get text with \$ and \# escaping removed.
+ // Should be a single Text node
+ value = ((ELNode.Text) el.iterator().next())
+ .getText();
el = null;
}
}
@@ -1449,7 +1444,9 @@ class Validator {
@Override
public void visit(Text n) throws JasperException {
- output.append(xmlEscape(n.getText()));
+ output.append(ELParser.escapeLiteralExpression(
+ xmlEscape(n.getText()),
+ isDeferredSyntaxAllowedAsLiteral));
}
}
Modified: tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestParser.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestParser.java?rev=1590848&r1=1590847&r2=1590848&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestParser.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestParser.java Tue Apr 29 00:19:33 2014
@@ -404,6 +404,10 @@ public class TestParser extends TomcatBa
Assert.assertTrue(result, result.contains("<set data-value=\"03b\\\\x\\?resize03b\"/>"));
Assert.assertTrue(result, result.contains("<04a\\?resize04a/>"));
Assert.assertTrue(result, result.contains("<04b\\\\x\\?resize04b/>"));
+ Assert.assertTrue(result, result.contains("<set data-value=\"05a$${&\"/>"));
+ Assert.assertTrue(result, result.contains("<set data-value=\"05b$${&2\"/>"));
+ Assert.assertTrue(result, result.contains("<set data-value=\"05c##{>hello<\"/>"));
+ Assert.assertTrue(result, result.contains("<set xmlns:foo=\"urn:06a\\bar\\baz\"/>"));
}
/** Assertion for text printed by tags:echo */
Modified: tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56334.jspx
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56334.jspx?rev=1590848&r1=1590847&r2=1590848&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56334.jspx (original)
+++ tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56334.jspx Tue Apr 29 00:19:33 2014
@@ -29,4 +29,12 @@
<jsp:element name="04b\\x${'\\?resize04b'}"></jsp:element>
+ <!-- Test 5: Use \$, \# in template text -->
+ <set data-value="05a\$\${&" />
+ <set data-value="05b\$\${&${1+1}" />
+ <set data-value="05c\#\#{>${'hello'}<" />
+
+ <!-- Test 6: nonTaglibXmlnsAttributes on a Node.UninterpretedTag -->
+ <set xmlns:foo="urn:06a\bar\baz" />
+
</jsp:root>
\ No newline at end of file
Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1590848&r1=1590847&r2=1590848&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Tue Apr 29 00:19:33 2014
@@ -150,7 +150,7 @@
<changelog>
<fix>
<bug>56334</bug>: Fix a regression in the handling of back-slash
- escaping introduced by the fix for <bug>55735</bug>. (markt)
+ escaping introduced by the fix for <bug>55735</bug>. (markt/kkolinko)
</fix>
<fix>
<bug>56425</bug>: Improve method matching for EL expressions. When
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org