Author: markt
Date: Thu Apr 24 08:35:06 2014
New Revision: 1589635
URL: http://svn.apache.org/r1589635
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56334
Correct double unescaping
Modified:
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/Validator.java
tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java
tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1589635&r1=1589634&r2=1589635&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Thu Apr 24 08:35:06 2014
@@ -28,12 +28,6 @@ None
PATCHES PROPOSED TO BACKPORT:
[ New proposals should be added at the end of the list ]
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56334
- Correct double unescaping
-
http://people.apache.org/~markt/patches/2014-04-17-attribute-escaping-tc6-v2.patch
- +1: markt, remm, fhanik
- -1:
-
* Enabling building with Java 8
http://people.apache.org/~markt/patches/2014-04-12-build-with-java8-tc6-v1.patch
(Note: It is easier to verify the AbstractReplicatedMap changes by diffing
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=1589635&r1=1589634&r2=1589635&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 Thu Apr
24 08:35:06 2014
@@ -203,15 +203,22 @@ public class ELParser {
while (hasNextChar()) {
char ch = nextChar();
if (prev == '\\') {
+ if (ch == '$' || (!isDeferredSyntaxAllowedAsLiteral && ch ==
'#')) {
prev = 0;
- if (ch == '\\') {
+ buf.append(ch);
+ continue;
+ } else if (ch == '\\') {
+ // Not an escape (this time).
+ // Optimisation - no need to set prev as it is unchanged
+ buf.append('\\');
+ continue;
+ } else {
+ // Not an escape
+ prev = 0;
buf.append('\\');
- prev = '\\';
- } else if (ch == '$'
- || (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) {
buf.append(ch);
+ continue;
}
- // else error!
} else if (prev == '$'
|| (!isDeferredSyntaxAllowedAsLiteral && prev == '#')) {
if (ch == '{') {
@@ -235,6 +242,98 @@ public class ELParser {
return buf.toString();
}
+
+ /**
+ * Escape '\\', '$' and '#', inverting the unescaping performed in
+ * {@link #skipUntilEL()}.
+ *
+ * @param input Non-EL input to be escaped
+ * @param isDeferredSyntaxAllowedAsLiteral
+ *
+ * @return The escaped version of the input
+ */
+ private static String escapeLiteralExpression(String input,
+ boolean isDeferredSyntaxAllowedAsLiteral) {
+ int len = input.length();
+ int lastAppend = 0;
+ StringBuilder output = null;
+ for (int i = 0; i < len; i++) {
+ char ch = input.charAt(i);
+ if (ch =='$' || (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) {
+ if (output == null) {
+ output = new StringBuilder(len + 20);
+ }
+ output.append(input.subSequence(lastAppend, i));
+ lastAppend = i + 1;
+ output.append('\\');
+ output.append(ch);
+ }
+ }
+ if (output == null) {
+ return input;
+ } else {
+ output.append(input.substring(lastAppend, len));
+ return output.toString();
+ }
+ }
+
+
+ /**
+ * Escape '\\', '\'' and '\"', inverting the unescaping performed in
+ * {@link #skipUntilEL()}.
+ *
+ * @param input Non-EL input to be escaped
+ * @param isDeferredSyntaxAllowedAsLiteral
+ *
+ * @return The escaped version of the input
+ */
+ private static String escapeELText(String input) {
+ int len = input.length();
+ char quote = 0;
+ int lastAppend = 0;
+
+ if (len > 1) {
+ // Might be quoted
+ quote = input.charAt(0);
+ if (quote == '\'' || quote == '\"') {
+ if (input.charAt(len - 1) != quote) {
+ throw new IllegalArgumentException(Localizer.getMessage(
+
"org.apache.jasper.compiler.ELParser.invalidQuotesForStringLiteral",
+ input));
+ }
+ lastAppend = 1;
+ len--;
+ } else {
+ quote = 0;
+ }
+ }
+
+ StringBuilder output = null;
+ for (int i = lastAppend; i < len; i++) {
+ char ch = input.charAt(i);
+ if (ch == '\\' || ch == quote) {
+ if (output == null) {
+ output = new StringBuilder(len + 20);
+ output.append(quote);
+ }
+ output.append(input.subSequence(lastAppend, i));
+ lastAppend = i + 1;
+ output.append('\\');
+ output.append(ch);
+ }
+ }
+ if (output == null) {
+ return input;
+ } else {
+ output.append(input.substring(lastAppend, len));
+ if (quote != 0) {
+ output.append(quote);
+ }
+ return output.toString();
+ }
+ }
+
+
/*
* @return true if there is something left in EL expression buffer other
* than white spaces.
@@ -281,7 +380,7 @@ public class ELParser {
/*
* Parse a string in single or double quotes, allowing for escape sequences
- * '\\', and ('\"', or "\'")
+ * '\\', '\"' and "\'"
*/
private Token parseQuotedChars(char quote) {
StringBuilder buf = new StringBuilder();
@@ -290,10 +389,13 @@ public class ELParser {
char ch = nextChar();
if (ch == '\\') {
ch = nextChar();
- if (ch == '\\' || ch == quote) {
+ if (ch == '\\' || ch == '\'' || ch == '\"') {
buf.append(ch);
+ } else {
+ throw new IllegalArgumentException(Localizer.getMessage(
+
"org.apache.jasper.compiler.ELParser.invalidQuoting",
+ expression));
}
- // else error!
} else if (ch == quote) {
buf.append(ch);
break;
@@ -448,8 +550,13 @@ public class ELParser {
protected static class TextBuilder extends ELNode.Visitor {
+ private final boolean isDeferredSyntaxAllowedAsLiteral;
protected StringBuilder output = new StringBuilder();
+ protected TextBuilder(boolean isDeferredSyntaxAllowedAsLiteral) {
+ this.isDeferredSyntaxAllowedAsLiteral =
isDeferredSyntaxAllowedAsLiteral;
+ }
+
public String getText() {
return output.toString();
}
@@ -464,18 +571,18 @@ public class ELParser {
@Override
public void visit(Function n) throws JasperException {
- output.append(n.getOriginalText());
+ output.append(escapeLiteralExpression(n.getOriginalText(),
isDeferredSyntaxAllowedAsLiteral));
output.append('(');
}
@Override
public void visit(Text n) throws JasperException {
- output.append(n.getText());
+
output.append(escapeLiteralExpression(n.getText(),isDeferredSyntaxAllowedAsLiteral));
}
@Override
public void visit(ELText n) throws JasperException {
- output.append(n.getText());
+ output.append(escapeELText(n.getText()));
}
}
}
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=1589635&r1=1589634&r2=1589635&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 Thu Apr
24 08:35:06 2014
@@ -1359,7 +1359,8 @@ class Validator {
// The wrinkle is that the output of any EL must not be
// re-escaped as that must be output as is.
if (el != null) {
- XmlEscapeNonELVisitor v = new
XmlEscapeNonELVisitor();
+ XmlEscapeNonELVisitor v = new
XmlEscapeNonELVisitor(
+
pageInfo.isDeferredSyntaxAllowedAsLiteral());
el.visit(v);
value = v.getText();
} else {
@@ -1403,6 +1404,11 @@ class Validator {
private static class XmlEscapeNonELVisitor extends
ELParser.TextBuilder {
+ protected XmlEscapeNonELVisitor(
+ boolean isDeferredSyntaxAllowedAsLiteral) {
+ super(isDeferredSyntaxAllowedAsLiteral);
+ }
+
@Override
public void visit(Text n) throws JasperException {
output.append(xmlEscape(n.getText()));
Modified: tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java?rev=1589635&r1=1589634&r2=1589635&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java
(original)
+++ tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java Thu
Apr 24 08:35:06 2014
@@ -16,126 +16,289 @@
*/
package org.apache.jasper.compiler;
+import javax.el.ELContext;
+import javax.el.ELException;
+import javax.el.ExpressionFactory;
+import javax.el.ValueExpression;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.apache.el.ExpressionFactoryImpl;
import org.apache.jasper.JasperException;
import org.apache.jasper.compiler.ELNode.Nodes;
import org.apache.jasper.compiler.ELParser.TextBuilder;
+import org.apache.jasper.el.ELContextImpl;
-import junit.framework.TestCase;
-
-public class TestELParser extends TestCase {
+/**
+ * You will need to keep your wits about you when working with this class. Keep
+ * in mind the following:
+ * <ul>
+ * <li>If in doubt, read the EL and JSP specifications. Twice.</li>
+ * <li>The escaping rules are complex and subtle. The explanation below (as
well
+ * as the tests and the implementation) may have missed an edge case
despite
+ * trying hard not to.
+ * <li>The strings passed to {@link #doTestParser(String,String)} are Java
+ * escaped in the source code and will be unescaped before being used.</li>
+ * <li>LiteralExpressions always occur outside of "${...}" and "#{...}".
Literal
+ * expressions escape '$' and '#' with '\\' if '$' or '#' is followed by
'{'
+ * but neither '\\' nor '{' is escaped.</li>
+ * <li>LiteralStrings always occur inside "${...}" or "#{...}". Literal strings
+ * escape '\'', '\"' and '\\' with '\\'. Escaping '\"' is optional if the
+ * literal string is delimited by '\''. Escaping '\'' is optional if the
+ * literal string is delimited by '\"'.</li>
+ * </ul>
+ */
+public class TestELParser {
+ @Test
public void testText() throws JasperException {
- doTestParser("foo");
+ doTestParser("foo", "foo");
}
+ @Test
public void testLiteral() throws JasperException {
- doTestParser("${'foo'}");
+ doTestParser("${'foo'}", "foo");
}
+ @Test
public void testVariable() throws JasperException {
- doTestParser("${test}");
+ doTestParser("${test}", null);
}
+ @Test
public void testFunction01() throws JasperException {
- doTestParser("${do(x)}");
+ doTestParser("${do(x)}", null);
}
+ @Test
public void testFunction02() throws JasperException {
- doTestParser("${do:it(x)}");
+ doTestParser("${do:it(x)}", null);
}
+ @Test
public void testFunction03() throws JasperException {
- doTestParser("${do:it(x,y)}");
+ doTestParser("${do:it(x,y)}", null);
}
+ @Test
public void testFunction04() throws JasperException {
- doTestParser("${do:it(x,y,z)}");
+ doTestParser("${do:it(x,y,z)}", null);
}
+ @Test
public void testCompound01() throws JasperException {
- doTestParser("1${'foo'}1");
+ doTestParser("1${'foo'}1", "1foo1");
}
+ @Test
public void testCompound02() throws JasperException {
- doTestParser("1${test}1");
+ doTestParser("1${test}1", null);
}
+ @Test
public void testCompound03() throws JasperException {
- doTestParser("${foo}${bar}");
+ doTestParser("${foo}${bar}", null);
}
+ @Test
public void testTernary01() throws JasperException {
- doTestParser("${true?true:false}");
+ doTestParser("${true?true:false}", "true");
}
+ @Test
public void testTernary02() throws JasperException {
- doTestParser("${a==1?true:false}");
+ doTestParser("${a==1?true:false}", null);
}
+ @Test
public void testTernary03() throws JasperException {
- doTestParser("${a eq1?true:false}");
+ doTestParser("${a eq1?true:false}", null);
}
+ @Test
public void testTernary04() throws JasperException {
- doTestParser(" ${ a eq 1 ? true : false } ");
+ doTestParser(" ${ a eq 1 ? true : false } ", null);
}
+ @Test
public void testTernary05() throws JasperException {
// Note this is invalid EL
- doTestParser("${aeq1?true:false}");
+ doTestParser("${aeq1?true:false}", null);
}
+ @Test
public void testTernary06() throws JasperException {
- doTestParser("${do:it(a eq1?true:false,y)}");
+ doTestParser("${do:it(a eq1?true:false,y)}", null);
}
+ @Test
public void testTernary07() throws JasperException {
- doTestParser(" ${ do:it( a eq 1 ? true : false, y ) } ");
+ doTestParser(" ${ do:it( a eq 1 ? true : false, y ) } ", null);
}
+ @Test
public void testTernary08() throws JasperException {
- doTestParser(" ${ do:it ( a eq 1 ? true : false, y ) } ");
+ doTestParser(" ${ do:it ( a eq 1 ? true : false, y ) } ", null);
}
+ @Test
public void testTernary09() throws JasperException {
- doTestParser(" ${ do : it ( a eq 1 ? true : false, y ) } ");
+ doTestParser(" ${ do : it ( a eq 1 ? true : false, y ) } ", null);
}
+ @Test
public void testTernary10() throws JasperException {
- doTestParser(" ${!empty my:link(foo)} ");
+ doTestParser(" ${!empty my:link(foo)} ", null);
}
+ @Test
+ public void testTernary11() throws JasperException {
+ doTestParser("${true?'true':'false'}", "true");
+ }
+
+
+ @Test
+ public void testTernary12() throws JasperException {
+ doTestParser("${true?'tr\"ue':'false'}", "tr\"ue");
+ }
+
+
+ @Test
+ public void testTernary13() throws JasperException {
+ doTestParser("${true?'tr\\'ue':'false'}", "tr'ue");
+ }
+
+
+ @Test
public void testTernaryBug56031() throws JasperException {
- doTestParser("${my:link(!empty registration ? registration :
'/test/registration')}");
+ doTestParser("${my:link(!empty registration ? registration :
'/test/registration')}", null);
+ }
+
+
+ @Test
+ public void testQuotes01() throws JasperException {
+ doTestParser("'", "'");
+ }
+
+
+ @Test
+ public void testQuotes02() throws JasperException {
+ doTestParser("'${foo}'", null);
+ }
+
+
+ @Test
+ public void testQuotes03() throws JasperException {
+ doTestParser("'${'foo'}'", "'foo'");
+ }
+
+
+ @Test
+ public void testEscape01() throws JasperException {
+ doTestParser("${'\\\\'}", "\\");
+ }
+
+
+ @Test
+ public void testEscape02() throws JasperException {
+ doTestParser("\\\\x${'\\\\'}", "\\\\x\\");
+ }
+
+
+ @Test
+ public void testEscape03() throws JasperException {
+ doTestParser("\\\\", "\\\\");
+ }
+
+
+ @Test
+ public void testEscape04() throws JasperException {
+ doTestParser("\\$", "\\$");
+ }
+
+
+ @Test
+ public void testEscape05() throws JasperException {
+ doTestParser("\\#", "\\#");
+ }
+
+
+ @Test
+ public void testEscape07() throws JasperException {
+ doTestParser("${'\\\\$'}", "\\$");
}
- private void doTestParser(String input) throws JasperException {
- Nodes nodes = ELParser.parse(input, false);
- TextBuilder textBuilder = new TextBuilder();
+ @Test
+ public void testEscape08() throws JasperException {
+ doTestParser("${'\\\\#'}", "\\#");
+ }
+
+
+ @Test
+ public void testEscape09() throws JasperException {
+ doTestParser("\\${", "${");
+ }
+
+
+ @Test
+ public void testEscape10() throws JasperException {
+ doTestParser("\\#{", "#{");
+ }
+
+
+ private void doTestParser(String input, String expected) throws
JasperException {
+ ELException elException = null;
+ String elResult = null;
+
+ // Don't try and evaluate expressions that depend on variables or
functions
+ if (expected != null) {
+ try {
+ ExpressionFactory factory = new ExpressionFactoryImpl();
+ ELContext context = new ELContextImpl();
+ ValueExpression ve = factory.createValueExpression(context,
input, String.class);
+ elResult = ve.getValue(context).toString();
+ Assert.assertEquals(expected, elResult);
+ } catch (ELException ele) {
+ elException = ele;
+ }
+ }
+
+ Nodes nodes = null;
+ try {
+ nodes = ELParser.parse(input, false);
+ Assert.assertNull(elException);
+ } catch (IllegalArgumentException iae) {
+ Assert.assertNotNull(elResult, elException);
+ // Not strictly true but enables us to report both
+ iae.initCause(elException);
+ throw iae;
+ }
+
+ TextBuilder textBuilder = new TextBuilder(false);
nodes.visit(textBuilder);
- assertEquals(input, textBuilder.getText());
+ Assert.assertEquals(input, textBuilder.getText());
}
}
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=1589635&r1=1589634&r2=1589635&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Thu Apr 24 08:35:06 2014
@@ -135,6 +135,10 @@
can only be used when running with Java 6 or later. The "1.8" options
make sense only when running with Java 8 (or later). (kkolinko)
</fix>
+ <fix>
+ <bug>56334</bug>: Fix a regression in the handling of back-slash
+ escaping introduced by the fix for <bug>55735</bug>. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
