Author: markt
Date: Tue Jan 21 14:08:03 2014
New Revision: 1560017
URL: http://svn.apache.org/r1560017
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/trunk/java/org/apache/jasper/compiler/ELNode.java
tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java
tomcat/trunk/test/org/apache/el/TestELInJsp.java
tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java
tomcat/trunk/test/webapp/bug5nnnn/bug56029.jspx
Modified: tomcat/trunk/java/org/apache/jasper/compiler/ELNode.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/ELNode.java?rev=1560017&r1=1560016&r2=1560017&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/ELNode.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/ELNode.java Tue Jan 21
14:08:03 2014
@@ -122,14 +122,16 @@ abstract class ELNode {
private final String prefix;
private final 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/trunk/java/org/apache/jasper/compiler/ELParser.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java?rev=1560017&r1=1560016&r2=1560017&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java Tue Jan 21
14:08:03 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 final ELNode.Nodes expr;
@@ -96,7 +96,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() {
@@ -115,7 +117,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()));
@@ -138,12 +140,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();
@@ -153,7 +156,7 @@ public class ELParser {
Token t2 = nextToken();
if (t2 instanceof Id) {
s1 = s2.trim();
- s2 = t2.toString();
+ s2 = t2.toTrimmedString();
if (hasNext()) {
curToken = nextToken();
}
@@ -161,7 +164,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;
@@ -245,29 +248,29 @@ 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 (index < expression.length() &&
Character.isJavaIdentifierPart(
ch = expression.charAt(index))) {
- buf.append(ch);
nextChar();
}
- return new Id(getAndResetWhiteSpace(), buf.toString());
+ return new Id(getAndResetWhiteSpace(),
expression.substring(start, index));
}
if (ch == '\'' || ch == '"') {
@@ -311,13 +314,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() {
@@ -359,6 +363,10 @@ public class ELParser {
return whiteSpace;
}
+ String toTrimmedString() {
+ return "";
+ }
+
String getWhiteSpace() {
return whiteSpace;
}
@@ -379,6 +387,11 @@ public class ELParser {
public String toString() {
return whiteSpace + id;
}
+
+ @Override
+ String toTrimmedString() {
+ return id;
+ }
}
/*
@@ -402,6 +415,11 @@ public class ELParser {
public String toString() {
return whiteSpace + ch;
}
+
+ @Override
+ String toTrimmedString() {
+ return "" + ch;
+ }
}
/*
@@ -420,6 +438,11 @@ public class ELParser {
public String toString() {
return whiteSpace + value;
}
+
+ @Override
+ String toTrimmedString() {
+ return value;
+ }
}
public char getType() {
@@ -445,11 +468,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/trunk/test/org/apache/el/TestELInJsp.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/el/TestELInJsp.java?rev=1560017&r1=1560016&r2=1560017&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/el/TestELInJsp.java (original)
+++ tomcat/trunk/test/org/apache/el/TestELInJsp.java Tue Jan 21 14:08:03 2014
@@ -472,7 +472,7 @@ public class TestELInJsp extends TomcatB
String result = res.toString();
- Assert.assertTrue(result.contains("[1]"));
+ Assert.assertTrue(result.contains("[1]:[1]"));
}
Modified: tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java?rev=1560017&r1=1560016&r2=1560017&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java (original)
+++ tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java Tue Jan 21
14:08:03 2014
@@ -124,9 +124,28 @@ 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/trunk/test/webapp/bug5nnnn/bug56029.jspx
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/webapp/bug5nnnn/bug56029.jspx?rev=1560017&r1=1560016&r2=1560017&view=diff
==============================================================================
--- tomcat/trunk/test/webapp/bug5nnnn/bug56029.jspx (original)
+++ tomcat/trunk/test/webapp/bug5nnnn/bug56029.jspx Tue Jan 21 14:08:03 2014
@@ -1,25 +1,26 @@
-<?xml version="1.0" encoding="UTF-8" ?>
-<!--
- Licensed to the Apache Software Foundation (ASF) under one or more
- contributor license agreements. See the NOTICE file distributed with
- this work for additional information regarding copyright ownership.
- The ASF licenses this file to You under the Apache License, Version 2.0
- (the "License"); you may not use this file except in compliance with
- the License. You may obtain a copy of the License at
-
- http://www.apache.org/licenses/LICENSE-2.0
-
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
--->
-<jsp:root xmlns:jsp="http://java.sun.com/JSP/Page"; version="2.0"
- xmlns:c="http://java.sun.com/jsp/jstl/core";
- 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}]
+<?xml version="1.0" encoding="UTF-8" ?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<jsp:root xmlns:jsp="http://java.sun.com/JSP/Page"; version="2.0"
+ xmlns:c="http://java.sun.com/jsp/jstl/core";
+ 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="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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
