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/21 15:14:39 UTC
svn commit: r1560023 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/jasper/compiler/ test/org/apache/el/
test/org/apache/jasper/compiler/ test/webapp-3.0/bug5nnnn/
Author: markt
Date: Tue Jan 21 14:14:38 2014
New Revision: 1560023
URL: http://svn.apache.org/r1560023
Log:
Further work for https://issues.apache.org/bugzilla/show_bug.cgi?id=56029
Review from kkolinko
- Fix bug in parsing functions and whitespace
- More efficient resetting of StringBuidler
- Use String.substring() for token and whitespace
- avoid unnecessary use of trim()
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java
tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java
tomcat/tc7.0.x/trunk/test/org/apache/el/TestELInJsp.java
tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java
tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56029.jspx
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1560017
Modified: tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java?rev=1560023&r1=1560022&r2=1560023&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELNode.java Tue Jan 21 14:14:38 2014
@@ -122,14 +122,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;
}
@Override
@@ -145,6 +147,10 @@ abstract class ELNode {
return name;
}
+ public String getOriginalText() {
+ return originalText;
+ }
+
public void setUri(String uri) {
this.uri = uri;
}
Modified: tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java?rev=1560023&r1=1560022&r2=1560023&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/jasper/compiler/ELParser.java Tue Jan 21 14:14:38 2014
@@ -38,7 +38,7 @@ public class ELParser {
private Token curToken; // current token
private Token prevToken; // previous token
- private StringBuilder whiteSpace = new StringBuilder();
+ private String whiteSpace = "";
private ELNode.Nodes expr;
@@ -98,7 +98,9 @@ public class ELParser {
*
* @return An ELNode.Nodes representing the EL expression
*
- * TODO: Can this be refactored to use the standard EL implementation?
+ * 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() {
@@ -117,7 +119,7 @@ public class ELParser {
// Output whatever is in buffer
if (buf.length() > 0) {
ELexpr.add(new ELNode.ELText(buf.toString()));
- buf = new StringBuilder();
+ buf.setLength(0);
}
if (!parseFunction()) {
ELexpr.add(new ELNode.ELText(curToken.toString()));
@@ -140,12 +142,13 @@ public class ELParser {
* arguments
*/
private boolean parseFunction() {
- if (!(curToken instanceof Id) || isELReserved(curToken.toString()) ||
+ if (!(curToken instanceof Id) || isELReserved(curToken.toTrimmedString()) ||
prevToken instanceof Char && prevToken.toChar() == '.') {
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() - whiteSpace.length();
@@ -155,7 +158,7 @@ public class ELParser {
Token t2 = nextToken();
if (t2 instanceof Id) {
s1 = s2.trim();
- s2 = t2.toString();
+ s2 = t2.toTrimmedString();
if (hasNext()) {
curToken = nextToken();
}
@@ -163,7 +166,7 @@ public class ELParser {
}
}
if (curToken.toChar() == '(') {
- ELexpr.add(new ELNode.Function(s1, s2.trim()));
+ ELexpr.add(new ELNode.Function(s1, s2, expression.substring(start, index - 1)));
return true;
}
curToken = original;
@@ -248,28 +251,28 @@ public class ELParser {
}
private String getAndResetWhiteSpace() {
- String result = whiteSpace.toString();
- whiteSpace = new StringBuilder();
+ 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() {
prevToken = curToken;
- skipSpaces();
if (hasNextChar()) {
char ch = nextChar();
if (Character.isJavaIdentifierStart(ch)) {
- StringBuilder buf = new StringBuilder();
- buf.append(ch);
+ int start = index - 1;
while ((ch = peekChar()) != -1
&& Character.isJavaIdentifierPart(ch)) {
- buf.append(ch);
nextChar();
}
- return new Id(getAndResetWhiteSpace(), buf.toString());
+ return new Id(getAndResetWhiteSpace(), expression.substring(start, index));
}
if (ch == '\'' || ch == '"') {
@@ -313,13 +316,14 @@ public class ELParser {
*/
private void skipSpaces() {
+ int start = index;
while (hasNextChar()) {
char c = expression.charAt(index);
if (c > ' ')
break;
- whiteSpace.append(c);
index++;
}
+ whiteSpace = expression.substring(start, index);
}
private boolean hasNextChar() {
@@ -368,6 +372,10 @@ public class ELParser {
return whiteSpace;
}
+ String toTrimmedString() {
+ return "";
+ }
+
String getWhiteSpace() {
return whiteSpace;
}
@@ -388,6 +396,11 @@ public class ELParser {
public String toString() {
return whiteSpace + id;
}
+
+ @Override
+ String toTrimmedString() {
+ return id;
+ }
}
/*
@@ -411,6 +424,11 @@ public class ELParser {
public String toString() {
return whiteSpace + ch;
}
+
+ @Override
+ String toTrimmedString() {
+ return "" + ch;
+ }
}
/*
@@ -429,6 +447,11 @@ public class ELParser {
public String toString() {
return whiteSpace + value;
}
+
+ @Override
+ String toTrimmedString() {
+ return value;
+ }
}
public char getType() {
@@ -454,11 +477,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/tc7.0.x/trunk/test/org/apache/el/TestELInJsp.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/el/TestELInJsp.java?rev=1560023&r1=1560022&r2=1560023&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/el/TestELInJsp.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/el/TestELInJsp.java Tue Jan 21 14:14:38 2014
@@ -461,7 +461,7 @@ public class TestELInJsp extends TomcatB
String result = res.toString();
- Assert.assertTrue(result.contains("[1]"));
+ Assert.assertTrue(result.contains("[1]:[1]"));
}
Modified: tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java?rev=1560023&r1=1560022&r2=1560023&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/jasper/compiler/TestELParser.java Tue Jan 21 14:14:38 2014
@@ -124,7 +124,31 @@ public class TestELParser {
@Test
public void testTernary07() throws JasperException {
- doTestParser(" $ { do:it( a eq 1 ? true : false, y ) } ");
+ doTestParser(" ${ do:it( a eq 1 ? true : false, y ) } ");
+ }
+
+
+ @Test
+ public void testTernary08() throws JasperException {
+ doTestParser(" ${ do:it ( a eq 1 ? true : false, y ) } ");
+ }
+
+
+ @Test
+ public void testTernary09() throws JasperException {
+ doTestParser(" ${ do : it ( a eq 1 ? true : false, y ) } ");
+ }
+
+
+ @Test
+ public void testTernary10() throws JasperException {
+ doTestParser(" ${!empty my:link(foo)} ");
+ }
+
+
+ @Test
+ public void testTernaryBug56031() throws JasperException {
+ doTestParser("${my:link(!empty registration ? registration : '/test/registration')}");
}
Modified: tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56029.jspx
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56029.jspx?rev=1560023&r1=1560022&r2=1560023&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56029.jspx (original)
+++ tomcat/tc7.0.x/trunk/test/webapp-3.0/bug5nnnn/bug56029.jspx Tue Jan 21 14:14:38 2014
@@ -20,6 +20,7 @@
xmlns:fn="http://java.sun.com/jsp/jstl/functions">
<jsp:directive.page contentType="text/html; charset=UTF-8" session="false" />
<c:set var="list" value="%=new java.util.ArrayList() %" />
- <c:set var="limit" value="${1 + fn:length(list)}" />
- [${limit}]
+ <c:set var="limitA" value="${1 + fn:length(list)}" />
+ <c:set var="limitB" value="${1 + fn : length ( list ) }" />
+ [${limitA}]:[${limitB}]
</jsp:root>
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org