Author: kkolinko
Date: Mon Apr 28 23:35:36 2014
New Revision: 1590842
URL: http://svn.apache.org/r1590842
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56334
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/trunk/java/org/apache/jasper/compiler/ELParser.java
tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
tomcat/trunk/java/org/apache/jasper/compiler/Validator.java
tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java?rev=1590842&r1=1590841&r2=1590842&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java Mon Apr 28
23:35:36 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/trunk/java/org/apache/jasper/compiler/Generator.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Generator.java?rev=1590842&r1=1590841&r2=1590842&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Mon Apr 28
23:35:36 2014
@@ -1820,7 +1820,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);
}
}
@@ -1839,7 +1839,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/trunk/java/org/apache/jasper/compiler/Validator.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Validator.java?rev=1590842&r1=1590841&r2=1590842&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Validator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Validator.java Mon Apr 28
23:35:36 2014
@@ -561,7 +561,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);
}
@@ -580,7 +580,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);
}
@@ -588,7 +588,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);
}
@@ -605,7 +605,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;
@@ -641,7 +641,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");
@@ -680,11 +680,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);
@@ -694,7 +694,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);
}
@@ -773,7 +773,7 @@ class Validator {
}
jspAttrs[i] = getJspAttribute(null, attrs.getQName(i),
attrs.getURI(i), attrs.getLocalName(i), value, n,
- false);
+ null, false);
}
n.setJspAttributes(jspAttrs);
}
@@ -922,12 +922,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,30 +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(
- expressionFactory);
-
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 {
@@ -1254,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;
@@ -1270,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());
@@ -1345,10 +1331,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;
@@ -1368,7 +1357,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
@@ -1376,12 +1364,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;
}
}
@@ -1448,7 +1442,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/trunk/test/org/apache/jasper/compiler/TestParser.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java?rev=1590842&r1=1590841&r2=1590842&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java (original)
+++ tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java Mon Apr 28
23:35:36 2014
@@ -412,6 +412,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/trunk/test/webapp/bug5nnnn/bug56334.jspx
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx?rev=1590842&r1=1590841&r2=1590842&view=diff
==============================================================================
--- tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx (original)
+++ tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx Mon Apr 28 23:35:36 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/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1590842&r1=1590841&r2=1590842&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Apr 28 23:35:36 2014
@@ -188,7 +188,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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
