Author: kkolinko
Date: Sun May 11 19:53:59 2014
New Revision: 1593845
URL: http://svn.apache.org/r1593845
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56334
Merged r1590848 from tomcat/tc7.0.x/trunk:
Additional fixes for BZ 56334.
Includes the following:
1. Allow '\' in xmlns attributes of UninterpretedTag.
(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.
Modified:
tomcat/tc6.0.x/trunk/ (props changed)
tomcat/tc6.0.x/trunk/STATUS.txt
tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java
tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.java
tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc6.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1590842
Merged /tomcat/tc7.0.x/trunk:r1590848
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1593845&r1=1593844&r2=1593845&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sun May 11 19:53:59 2014
@@ -28,14 +28,6 @@ None
PATCHES PROPOSED TO BACKPORT:
[ New proposals should be added at the end of the list ]
-* Additional fixes for BZ 56334
- http://svn.apache.org/r1590848
- +1: kkolinko, markt, fhanik
- -1:
- kkolinko: I expect to prepare a more formal patch for this later. The
- merge is unlikely to complete cleanly without Mark's
- 2014-04-28-bug56334-tc6-v1.patch being applied first.
-
* Clean-up and add additional packages
https://svn.apache.org/r1593262
https://svn.apache.org/r1593285
Modified: tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java?rev=1593845&r1=1593844&r2=1593845&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java Sun May
11 19:53:59 2014
@@ -252,7 +252,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;
@@ -548,10 +548,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/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java?rev=1593845&r1=1593844&r2=1593845&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java Sun May
11 19:53:59 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);
}
@@ -1838,7 +1838,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/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.java?rev=1593845&r1=1593844&r2=1593845&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.java Sun May
11 19:53:59 2014
@@ -541,7 +541,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);
}
@@ -558,14 +558,14 @@ 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);
}
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);
}
@@ -580,7 +580,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;
@@ -615,7 +615,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");
@@ -652,11 +652,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);
@@ -736,7 +736,7 @@ class Validator {
}
jspAttrs[i] = getJspAttribute(null, attrs.getQName(i),
attrs.getURI(i), attrs.getLocalName(i), value, n,
- false);
+ null, false);
}
n.setJspAttributes(jspAttrs);
}
@@ -881,12 +881,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);
}
}
}
@@ -1050,10 +1051,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()) {
@@ -1075,18 +1077,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
@@ -1138,18 +1142,18 @@ class Validator {
}
// Check casting
try {
- ELSupport.checkType(attributeValue,
expectedClass);
+
ELSupport.checkType(textAttributeValue, expectedClass);
} catch (Exception e) {
err.jspError
(n, "jsp.error.coerce_to_type",
- tldAttr.getName(), expectedType,
attributeValue);
+ tldAttr.getName(), expectedType,
textAttributeValue);
}
}
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()) {
@@ -1163,29 +1167,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), attrs
- .getValue(i), n, false);
- }
+ // EL or Runtime expression
+ jspAttrs[i] = getJspAttribute(tldAttr,
+ attrs.getQName(i), attrs.getURI(i),
+ attrs.getLocalName(i),
+ xmlAttributeValue, n, el, false);
}
} else {
@@ -1198,14 +1184,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), attrs
- .getValue(i));
+ tagDataAttrs.put(attrs.getQName(i),
+ textAttributeValue);
}
found = true;
break;
@@ -1214,8 +1200,8 @@ class Validator {
if (!found) {
if (tagInfo.hasDynamicAttributes()) {
jspAttrs[i] = getJspAttribute(null, attrs.getQName(i),
- attrs.getURI(i), attrs.getLocalName(i), attrs
- .getValue(i), n, true);
+ attrs.getURI(i), attrs.getLocalName(i),
+ xmlAttributeValue, n, el, true);
} else {
err.jspError(n, "jsp.error.bad_attribute", attrs
.getQName(i), n.getLocalName());
@@ -1293,10 +1279,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;
@@ -1316,7 +1305,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
@@ -1324,12 +1312,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;
}
}
@@ -1395,7 +1389,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/tc6.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1593845&r1=1593844&r2=1593845&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Sun May 11 19:53:59 2014
@@ -161,7 +161,7 @@
</fix>
<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>
Correct the handling of back-slash escaping in the EL parser and no
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
