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/01/23 22:52:39 UTC
svn commit: r1560824 - in /tomcat/tc6.0.x/trunk: STATUS.txt
java/org/apache/jasper/compiler/ELNode.java
java/org/apache/jasper/compiler/ELParser.java
test/org/apache/jasper/compiler/TestELParser.java
Author: markt
Date: Thu Jan 23 21:52:38 2014
New Revision: 1560824
URL: http://svn.apache.org/r1560824
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56029
Regression in fix for BZ55198 broke parsing of some ternary expressions
Align tc6 implementation with that of trunk (diff to trunk is easy to review)
Modified:
tomcat/tc6.0.x/trunk/STATUS.txt
tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java
tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java
tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1560824&r1=1560823&r2=1560824&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Thu Jan 23 21:52:38 2014
@@ -30,13 +30,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=56029
- Regression in fix for BZ55198 broke parsing of some ternary expressions
- Align tc6 implementation with that of trunk (diff to trunk is easy to review)
- http://people.apache.org/~markt/patches/2014-01-21-ELParser-tc6-v1.patch
- +1: markt, kkolinko, remm
- -1:
-
* Correct Maven dependencies
http://people.apache.org/~markt/patches/2014-01-23-poms-tc6-v1.patch
+1: markt
Modified: tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java?rev=1560824&r1=1560823&r2=1560824&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java Thu Jan 23 21:52:38 2014
@@ -115,14 +115,16 @@ abstract class ELNode {
private String prefix;
private String name;
+ private final String originalText;
private String uri;
private FunctionInfo functionInfo;
private String methodName;
private String[] parameters;
- Function(String prefix, String name) {
+ Function(String prefix, String name, String originalText) {
this.prefix = prefix;
this.name = name;
+ this.originalText = originalText;
}
public void accept(Visitor v) throws JasperException {
@@ -137,6 +139,10 @@ abstract class ELNode {
return name;
}
+ public String getOriginalText() {
+ return originalText;
+ }
+
public void setUri(String uri) {
this.uri = uri;
}
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=1560824&r1=1560823&r2=1560824&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 Jan 23 21:52:38 2014
@@ -37,19 +37,18 @@ import org.apache.jasper.compiler.ELNode
public class ELParser {
private Token curToken; // current token
-
- private ELNode.Nodes expr;
+ private String whiteSpace = "";
+
+ private final ELNode.Nodes expr;
private ELNode.Nodes ELexpr;
private int index; // Current index of the expression
- private String expression; // The EL expression
+ private final String expression; // The EL expression
private char type;
- private boolean escapeBS; // is '\' an escape char in text outside EL?
-
private final boolean isDeferredSyntaxAllowedAsLiteral;
private static final String reservedWords[] = { "and", "div", "empty",
@@ -91,34 +90,41 @@ public class ELParser {
}
/**
- * Parse an EL expression string '${...}'
+ * Parse an EL expression string '${...}'. Currently only separates the EL
+ * into functions and everything else.
*
- * @return An ELNode.Nodes representing the EL expression TODO: Currently
- * only parsed into functions and text strings. This should be
- * rewritten for a full parser.
+ * @return An ELNode.Nodes representing the EL expression
+ *
+ * Note: This can not be refactored to use the standard EL implementation as
+ * the EL API does not provide the level of access required to the
+ * parsed expression.
*/
private ELNode.Nodes parseEL() {
- StringBuffer buf = new StringBuffer();
+ StringBuilder buf = new StringBuilder();
ELexpr = new ELNode.Nodes();
+ curToken = null;
while (hasNext()) {
curToken = nextToken();
if (curToken instanceof Char) {
if (curToken.toChar() == '}') {
break;
}
- buf.append(curToken.toChar());
+ buf.append(curToken.toString());
} else {
// Output whatever is in buffer
if (buf.length() > 0) {
ELexpr.add(new ELNode.ELText(buf.toString()));
- buf = new StringBuffer();
+ buf.setLength(0);
}
if (!parseFunction()) {
ELexpr.add(new ELNode.ELText(curToken.toString()));
}
}
}
+ if (curToken != null) {
+ buf.append(curToken.getWhiteSpace());
+ }
if (buf.length() > 0) {
ELexpr.add(new ELNode.ELText(buf.toString()));
}
@@ -132,30 +138,33 @@ public class ELParser {
* arguments
*/
private boolean parseFunction() {
- if (!(curToken instanceof Id) || isELReserved(curToken.toString())) {
+ if (!(curToken instanceof Id) || isELReserved(curToken.toTrimmedString())) {
return false;
}
String s1 = null; // Function prefix
- String s2 = curToken.toString(); // Function name
+ String s2 = curToken.toTrimmedString(); // Function name
+ int start = index - curToken.toString().length();
+ Token original = curToken;
if (hasNext()) {
- int mark = getIndex();
- Token t = nextToken();
- if (t.toChar() == ':') {
+ int mark = getIndex() - whiteSpace.length();
+ curToken = nextToken();
+ if (curToken.toChar() == ':') {
if (hasNext()) {
Token t2 = nextToken();
if (t2 instanceof Id) {
s1 = s2;
- s2 = t2.toString();
+ s2 = t2.toTrimmedString();
if (hasNext()) {
- t = nextToken();
+ curToken = nextToken();
}
}
}
}
- if (t.toChar() == '(') {
- ELexpr.add(new ELNode.Function(s1, s2));
+ if (curToken.toChar() == '(') {
+ ELexpr.add(new ELNode.Function(s1, s2, expression.substring(start, index - 1)));
return true;
}
+ curToken = original;
setIndex(mark);
}
return false;
@@ -190,15 +199,14 @@ public class ELParser {
*/
private String skipUntilEL() {
char prev = 0;
- StringBuffer buf = new StringBuffer();
+ StringBuilder buf = new StringBuilder();
while (hasNextChar()) {
char ch = nextChar();
if (prev == '\\') {
prev = 0;
if (ch == '\\') {
buf.append('\\');
- if (!escapeBS)
- prev = '\\';
+ prev = '\\';
} else if (ch == '$'
|| (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) {
buf.append(ch);
@@ -236,29 +244,36 @@ public class ELParser {
return hasNextChar();
}
+ private String getAndResetWhiteSpace() {
+ String result = whiteSpace;
+ whiteSpace = "";
+ return result;
+ }
+
/*
+ * Implementation note: This method assumes that it is always preceded by a
+ * call to hasNext() in order for whitespace handling to be correct.
+ *
* @return The next token in the EL expression buffer.
*/
private Token nextToken() {
- skipSpaces();
if (hasNextChar()) {
char ch = nextChar();
if (Character.isJavaIdentifierStart(ch)) {
- StringBuffer buf = new StringBuffer();
- buf.append(ch);
- while ((ch = peekChar()) != -1
- && Character.isJavaIdentifierPart(ch)) {
- buf.append(ch);
+ int start = index - 1;
+ while (index < expression.length() &&
+ Character.isJavaIdentifierPart(
+ ch = expression.charAt(index))) {
nextChar();
}
- return new Id(buf.toString());
+ return new Id(getAndResetWhiteSpace(), expression.substring(start, index));
}
if (ch == '\'' || ch == '"') {
return parseQuotedChars(ch);
} else {
// For now...
- return new Char(ch);
+ return new Char(getAndResetWhiteSpace(), ch);
}
}
return null;
@@ -269,7 +284,7 @@ public class ELParser {
* '\\', and ('\"', or "\'")
*/
private Token parseQuotedChars(char quote) {
- StringBuffer buf = new StringBuffer();
+ StringBuilder buf = new StringBuilder();
buf.append(quote);
while (hasNextChar()) {
char ch = nextChar();
@@ -286,7 +301,7 @@ public class ELParser {
buf.append(ch);
}
}
- return new QuotedString(buf.toString());
+ return new QuotedString(getAndResetWhiteSpace(), buf.toString());
}
/*
@@ -295,11 +310,14 @@ public class ELParser {
*/
private void skipSpaces() {
+ int start = index;
while (hasNextChar()) {
- if (expression.charAt(index) > ' ')
+ char c = expression.charAt(index);
+ if (c > ' ')
break;
index++;
}
+ whiteSpace = expression.substring(start, index);
}
private boolean hasNextChar() {
@@ -313,13 +331,6 @@ public class ELParser {
return expression.charAt(index++);
}
- private char peekChar() {
- if (index >= expression.length()) {
- return (char) -1;
- }
- return expression.charAt(index);
- }
-
private int getIndex() {
return index;
}
@@ -333,13 +344,28 @@ public class ELParser {
*/
private static class Token {
+ protected final String whiteSpace;
+
+ Token(String whiteSpace) {
+ this.whiteSpace = whiteSpace;
+ }
+
char toChar() {
return 0;
}
+ @Override
public String toString() {
+ return whiteSpace;
+ }
+
+ String toTrimmedString() {
return "";
}
+
+ String getWhiteSpace() {
+ return whiteSpace;
+ }
}
/*
@@ -348,11 +374,18 @@ public class ELParser {
private static class Id extends Token {
String id;
- Id(String id) {
+ Id(String whiteSpace, String id) {
+ super(whiteSpace);
this.id = id;
}
+ @Override
public String toString() {
+ return whiteSpace + id;
+ }
+
+ @Override
+ String toTrimmedString() {
return id;
}
}
@@ -364,16 +397,24 @@ public class ELParser {
private char ch;
- Char(char ch) {
+ Char(String whiteSpace, char ch) {
+ super(whiteSpace);
this.ch = ch;
}
+ @Override
char toChar() {
return ch;
}
+ @Override
public String toString() {
- return (new Character(ch)).toString();
+ return whiteSpace + ch;
+ }
+
+ @Override
+ String toTrimmedString() {
+ return "" + ch;
}
}
@@ -384,11 +425,18 @@ public class ELParser {
private String value;
- QuotedString(String v) {
+ QuotedString(String whiteSpace, String v) {
+ super(whiteSpace);
this.value = v;
}
+ @Override
public String toString() {
+ return whiteSpace + value;
+ }
+
+ @Override
+ String toTrimmedString() {
return value;
}
}
@@ -416,11 +464,7 @@ public class ELParser {
@Override
public void visit(Function n) throws JasperException {
- if (n.getPrefix() != null) {
- output.append(n.getPrefix());
- output.append(':');
- }
- output.append(n.getName());
+ output.append(n.getOriginalText());
output.append('(');
}
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=1560824&r1=1560823&r2=1560824&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 Jan 23 21:52:38 2014
@@ -74,6 +74,61 @@ public class TestELParser extends TestCa
}
+ public void testTernary01() throws JasperException {
+ doTestParser("${true?true:false}");
+ }
+
+
+ public void testTernary02() throws JasperException {
+ doTestParser("${a==1?true:false}");
+ }
+
+
+ public void testTernary03() throws JasperException {
+ doTestParser("${a eq1?true:false}");
+ }
+
+
+ public void testTernary04() throws JasperException {
+ doTestParser(" ${ a eq 1 ? true : false } ");
+ }
+
+
+ public void testTernary05() throws JasperException {
+ // Note this is invalid EL
+ doTestParser("${aeq1?true:false}");
+ }
+
+
+ public void testTernary06() throws JasperException {
+ doTestParser("${do:it(a eq1?true:false,y)}");
+ }
+
+
+ public void testTernary07() throws JasperException {
+ doTestParser(" ${ do:it( a eq 1 ? true : false, y ) } ");
+ }
+
+
+ public void testTernary08() throws JasperException {
+ doTestParser(" ${ do:it ( a eq 1 ? true : false, y ) } ");
+ }
+
+
+ public void testTernary09() throws JasperException {
+ doTestParser(" ${ do : it ( a eq 1 ? true : false, y ) } ");
+ }
+
+
+ public void testTernary10() throws JasperException {
+ doTestParser(" ${!empty my:link(foo)} ");
+ }
+
+
+ public void testTernaryBug56031() throws JasperException {
+ doTestParser("${my:link(!empty registration ? registration : '/test/registration')}");
+ }
+
private void doTestParser(String input) throws JasperException {
Nodes nodes = ELParser.parse(input, false);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org