You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/04/24 10:35:07 UTC
svn commit: r1589635 - in /tomcat/tc6.0.x/trunk: STATUS.txt
java/org/apache/jasper/compiler/ELParser.java
java/org/apache/jasper/compiler/Validator.java
test/org/apache/jasper/compiler/TestELParser.java webapps/docs/changelog.xml
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1589635 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/jasper/compiler/ELParser.java
java/org/apache/jasper/compiler/Validator.java test/org/apache/jasper/compiler/TestELParser.java
webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
On 24/04/2014 10:00, Konstantin Kolinko wrote:
> 2014-04-24 12:35 GMT+04:00 <ma...@apache.org>:
>> Author: markt
>> Date: Thu Apr 24 08:35:06 2014
>> New Revision: 1589635
<snip/>
>> 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
<snip/>
>> @@ -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);
>> + }
>
> String.subSequence() = String.substring(),
> but StringBuilder.append uses array copy with string but iteration
> with a sequence
StringBuilder.append also does a "instanceof" check and appends if the
CharSequence is an instance of String.
While I'm happy to clean up trunk I don't see the point in back-porting.
<snip/>
>> + /**
>> + * 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",
>
> The LocalString.properties change is missing from backport. See r1587887
> BTW, maybe
> s /must be contained with/must be contained within/
> in message text.
I'll get that fixed.
<snip/>
>> + 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));
>
> Ditto, s/subSequence/substring/.
Ditto, change is unnecessary.
>> + 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.
<snip/>
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1589635 - in /tomcat/tc6.0.x/trunk: STATUS.txt
java/org/apache/jasper/compiler/ELParser.java java/org/apache/jasper/compiler/Validator.java
test/org/apache/jasper/compiler/TestELParser.java webapps/docs/changelog.xml
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-04-24 12:35 GMT+04:00 <ma...@apache.org>:
> 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);
> + }
String.subSequence() = String.substring(),
but StringBuilder.append uses array copy with string but iteration
with a sequence
> + output.append(input.subSequence(lastAppend, i));
> + lastAppend = i + 1;
> + output.append('\\');
> + output.append(ch);
> + }
> + }
> + if (output == null) {
> + return input;
> + } else {
substring() is already used here.
> + 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",
The LocalString.properties change is missing from backport. See r1587887
BTW, maybe
s /must be contained with/must be contained within/
in message text.
> + 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));
Ditto, s/subSequence/substring/.
> + 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()));
> }
> }
> }
>
(...)
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org