Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48668
authorkkolinko <kkolinko@13f79535-47bb-0310-9956-ffa450edef68>
Sun, 7 Mar 2010 02:43:12 +0000 (02:43 +0000)
committerkkolinko <kkolinko@13f79535-47bb-0310-9956-ffa450edef68>
Sun, 7 Mar 2010 02:43:12 +0000 (02:43 +0000)
Fix remaining issues in BZ48668
The idea behind this change is to make ELParser aware about isDeferredAsLiteral option.
Before this change ELParser was used to parse an attribute regardless of isELIgnored or isDeferredSyntaxAllowedAsLiteral values. With this change we do not use ELParser when isELIgnored is true and ELParser does not parse '#{' in expressions when isDeferredSyntaxAllowedAsLiteral is true.
It simplified the code in many places.
Also, servlet specification version from web.xml and JSP specification version from TLD file are now taken into account when determining the default values for isELIgnored and isDeferredSyntaxAllowedAsLiteral. As far as I understand the code, previously only isELIgnored was determined by the servlet specification version.

TstParser.java, bug48668a.jsp:
I reenabled the tests that now pass with these changes applied.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@919914 13f79535-47bb-0310-9956-ffa450edef68

java/org/apache/jasper/compiler/Compiler.java
java/org/apache/jasper/compiler/ELParser.java
java/org/apache/jasper/compiler/JspConfig.java
java/org/apache/jasper/compiler/Validator.java
java/org/apache/jasper/resources/LocalStrings.properties
test/org/apache/jasper/compiler/TestParser.java
test/webapp/bug48668a.jsp

index 1b79f9e..9328781 100644 (file)
@@ -152,6 +152,20 @@ public abstract class Compiler {
                     JspUtil.booleanValue(
                             jspProperty.isErrorOnUndeclaredNamespace()));
         }
+        if (ctxt.getTagInfo() != null) {
+            try {
+                double libraryVersion = Double.parseDouble(ctxt.getTagInfo()
+                        .getTagLibrary().getRequiredVersion());
+                if (libraryVersion < 2.0) {
+                    pageInfo.setELIgnored(true);
+                }
+                if (libraryVersion < 2.1) {
+                    pageInfo.setDeferredSyntaxAllowedAsLiteral(true);
+                }
+            } catch (NumberFormatException ex) {
+                // ignored
+            }
+        }
 
         ctxt.checkOutputDir();
         String javaFileName = ctxt.getServletJavaFileName();
index ce17448..4a64b56 100644 (file)
@@ -45,13 +45,16 @@ public class ELParser {
 
     private boolean escapeBS; // is '\' an escape char in text outside EL?
 
+    private final boolean isDeferredSyntaxAllowedAsLiteral;
+
     private static final String reservedWords[] = { "and", "div", "empty",
             "eq", "false", "ge", "gt", "instanceof", "le", "lt", "mod", "ne",
             "not", "null", "or", "true" };
 
-    public ELParser(String expression) {
+    public ELParser(String expression, boolean isDeferredSyntaxAllowedAsLiteral) {
         index = 0;
         this.expression = expression;
+        this.isDeferredSyntaxAllowedAsLiteral = isDeferredSyntaxAllowedAsLiteral;
         expr = new ELNode.Nodes();
     }
 
@@ -61,10 +64,14 @@ public class ELParser {
      * @param expression
      *            The input expression string of the form Char* ('${' Char*
      *            '}')* Char*
+     * @param isDeferredSyntaxAllowedAsLiteral
+     *                      Are deferred expressions treated as literals?
      * @return Parsed EL expression in ELNode.Nodes
      */
-    public static ELNode.Nodes parse(String expression) {
-        ELParser parser = new ELParser(expression);
+    public static ELNode.Nodes parse(String expression,
+            boolean isDeferredSyntaxAllowedAsLiteral) {
+        ELParser parser = new ELParser(expression,
+                isDeferredSyntaxAllowedAsLiteral);
         while (parser.hasNextChar()) {
             String text = parser.skipUntilEL();
             if (text.length() > 0) {
@@ -188,11 +195,13 @@ public class ELParser {
                     buf.append('\\');
                     if (!escapeBS)
                         prev = '\\';
-                } else if (ch == '$' || ch == '#') {
+                } else if (ch == '$'
+                        || (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) {
                     buf.append(ch);
                 }
                 // else error!
-            } else if (prev == '$' || prev == '#') {
+            } else if (prev == '$'
+                    || (!isDeferredSyntaxAllowedAsLiteral && prev == '#')) {
                 if (ch == '{') {
                     this.type = prev;
                     prev = 0;
@@ -201,7 +210,8 @@ public class ELParser {
                 buf.append(prev);
                 prev = 0;
             }
-            if (ch == '\\' || ch == '$' || ch == '#') {
+            if (ch == '\\' || ch == '$'
+                    || (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) {
                 prev = ch;
             } else {
                 buf.append(ch);
index 944cda6..7024b3d 100644 (file)
@@ -45,14 +45,14 @@ public class JspConfig {
     private ServletContext ctxt;
     private volatile boolean initialized = false;
 
-    private String defaultIsXml = null;                // unspecified
+    private final static String defaultIsXml = null;           // unspecified
     private String defaultIsELIgnored = null;  // unspecified
-    private String defaultIsScriptingInvalid = null;
+    private final static String defaultIsScriptingInvalid = null;
     private String defaultDeferedSyntaxAllowedAsLiteral = null;
-    private String defaultTrimDirectiveWhitespaces = null;
-    private String defaultDefaultContentType = null;
-    private String defaultBuffer = null;
-    private String defaultErrorOnUndeclaredNamespace = "false";
+    private final static String defaultTrimDirectiveWhitespaces = null;
+    private final static String defaultDefaultContentType = null;
+    private final static String defaultBuffer = null;
+    private final static String defaultErrorOnUndeclaredNamespace = "false";
     private JspProperty defaultJspProperty;
 
     public JspConfig(ServletContext ctxt) {
@@ -87,8 +87,12 @@ public class JspConfig {
             if (webApp == null
                     || getVersion(webApp) < 2.4) {
                 defaultIsELIgnored = "true";
+                defaultDeferedSyntaxAllowedAsLiteral = "true";
                 return;
             }
+            if (getVersion(webApp) < 2.5) {
+                defaultDeferedSyntaxAllowedAsLiteral = "true";
+            }
             TreeNode jspConfig = webApp.findChild("jsp-config");
             if (jspConfig == null) {
                 return;
index 3270a25..0f0220c 100644 (file)
@@ -418,8 +418,6 @@ class Validator {
 
         private ErrorDispatcher err;
 
-        private TagInfo tagInfo;
-
         private ClassLoader loader;
 
         private final StringBuilder buf = new StringBuilder(32);
@@ -510,7 +508,6 @@ class Validator {
         ValidateVisitor(Compiler compiler) {
             this.pageInfo = compiler.getPageInfo();
             this.err = compiler.getErrorDispatcher();
-            this.tagInfo = compiler.getCompilationContext().getTagInfo();
             this.loader = compiler.getCompilationContext().getClassLoader();
         }
 
@@ -724,10 +721,7 @@ class Validator {
 
             // JSP.2.2 - '#{' not allowed in template text
             if (n.getType() == '#') {
-                if (!pageInfo.isDeferredSyntaxAllowedAsLiteral()
-                        && (tagInfo == null 
-                                || ((tagInfo != null) && !(tagInfo.getTagLibrary().getRequiredVersion().equals("2.0")
-                                        || tagInfo.getTagLibrary().getRequiredVersion().equals("1.2"))))) {
+                if (!pageInfo.isDeferredSyntaxAllowedAsLiteral()) {
                     err.jspError(n, "jsp.error.el.template.deferred");
                 } else {
                     return;
@@ -738,7 +732,8 @@ class Validator {
             StringBuilder expr = this.getBuffer();
             expr.append(n.getType()).append('{').append(n.getText())
                     .append('}');
-            ELNode.Nodes el = ELParser.parse(expr.toString());
+            ELNode.Nodes el = ELParser.parse(expr.toString(), pageInfo
+                    .isDeferredSyntaxAllowedAsLiteral());
 
             // validate/prepare expression
             prepareExpression(el, n, expr.toString());
@@ -1073,10 +1068,6 @@ class Validator {
             TagAttributeInfo[] tldAttrs = tagInfo.getAttributes();
             Attributes attrs = n.getAttributes();
 
-            boolean checkDeferred = !pageInfo.isDeferredSyntaxAllowedAsLiteral()
-                && !(tagInfo.getTagLibrary().getRequiredVersion().equals("2.0")
-                        || tagInfo.getTagLibrary().getRequiredVersion().equals("1.2"));
-
             for (int i = 0; attrs != null && i < attrs.getLength(); i++) {
                 boolean found = false;
                 
@@ -1084,54 +1075,55 @@ class Validator {
                         || (!n.getRoot().isXmlSyntax() && attrs.getValue(i).startsWith("<%=")));
                 boolean elExpression = false;
                 boolean deferred = false;
-                boolean deferredValueIsLiteral = false;
 
                 ELNode.Nodes el = null;
-                if (!runtimeExpression) {
-                    el = ELParser.parse(attrs.getValue(i));
+                if (!runtimeExpression && !pageInfo.isELIgnored()) {
+                    el = ELParser.parse(attrs.getValue(i), pageInfo
+                            .isDeferredSyntaxAllowedAsLiteral());
                     Iterator<ELNode> nodes = el.iterator();
                     while (nodes.hasNext()) {
                         ELNode node = nodes.next();
                         if (node instanceof ELNode.Root) {
                             if (((ELNode.Root) node).getType() == '$') {
+                                if (elExpression && deferred) {
+                                    err.jspError(n,
+                                            "jsp.error.attribute.deferredmix");
+                                }
                                 elExpression = true;
-                            } else if (checkDeferred && ((ELNode.Root) node).getType() == '#') {
+                            } else if (((ELNode.Root) node).getType() == '#') {
+                                if (elExpression && !deferred) {
+                                    err.jspError(n,
+                                            "jsp.error.attribute.deferredmix");
+                                }
                                 elExpression = true;
                                 deferred = true;
-                                if (pageInfo.isELIgnored()) {
-                                    deferredValueIsLiteral = true;
-                                }
                             }
                         }
                     }
                 }
 
-                boolean expression = runtimeExpression 
-                    || (elExpression  && (!pageInfo.isELIgnored() || (!"true".equalsIgnoreCase(pageInfo.getIsELIgnored()) && checkDeferred && deferred)));
-                
+                boolean expression = runtimeExpression || elExpression;
+
                 for (int j = 0; tldAttrs != null && j < tldAttrs.length; j++) {
                     if (attrs.getLocalName(i).equals(tldAttrs[j].getName())
                             && (attrs.getURI(i) == null
                                     || attrs.getURI(i).length() == 0 || attrs
                                     .getURI(i).equals(n.getURI()))) {
-                        
-                        if (tldAttrs[j].canBeRequestTime()
-                                || tldAttrs[j].isDeferredMethod() || tldAttrs[j].isDeferredValue()) { // JSP 2.1
+
+                        TagAttributeInfo tldAttr = tldAttrs[j];
+                        if (tldAttr.canBeRequestTime()
+                                || tldAttr.isDeferredMethod() || tldAttr.isDeferredValue()) { // JSP 2.1
                             
                             if (!expression) {
-                                
-                                if (deferredValueIsLiteral && !pageInfo.isDeferredSyntaxAllowedAsLiteral()) {
-                                    err.jspError(n, "jsp.error.attribute.custom.non_rt_with_expr",
-                                            tldAttrs[j].getName());
-                                }
-                                
+
                                 String expectedType = null;
-                                if (tldAttrs[j].isDeferredMethod()) {
+                                if (tldAttr.isDeferredMethod()) {
                                     // The String literal must be castable to what is declared as type
                                     // for the attribute
-                                    String m = tldAttrs[j].getMethodSignature();
+                                    String m = tldAttr.getMethodSignature();
                                     if (m != null) {
-                                        int rti = m.trim().indexOf(' ');
+                                        m = m.trim();
+                                        int rti = m.indexOf(' ');
                                         if (rti > 0) {
                                             expectedType = m.substring(0, rti).trim();
                                         }
@@ -1144,13 +1136,13 @@ class Validator {
                                         // of void - JSP.2.3.4
                                         err.jspError(n,
                                                 "jsp.error.literal_with_void",
-                                                tldAttrs[j].getName());
+                                                tldAttr.getName());
                                     }
                                 }
-                                if (tldAttrs[j].isDeferredValue()) {
+                                if (tldAttr.isDeferredValue()) {
                                     // The String literal must be castable to what is declared as type
                                     // for the attribute
-                                    expectedType = tldAttrs[j].getExpectedTypeName();
+                                    expectedType = tldAttr.getExpectedTypeName();
                                 }
                                 if (expectedType != null) {
                                     Class<?> expectedClass = String.class;
@@ -1159,7 +1151,7 @@ class Validator {
                                     } catch (ClassNotFoundException e) {
                                         err.jspError
                                             (n, "jsp.error.unknown_attribute_type",
-                                             tldAttrs[j].getName(), expectedType);
+                                             tldAttr.getName(), expectedType);
                                     }
                                     // Check casting
                                     try {
@@ -1167,31 +1159,31 @@ class Validator {
                                     } catch (Exception e) {
                                         err.jspError
                                             (n, "jsp.error.coerce_to_type",
-                                             tldAttrs[j].getName(), expectedType, attrs.getValue(i));
+                                             tldAttr.getName(), expectedType, attrs.getValue(i));
                                     }
                                 }
 
-                                jspAttrs[i] = new Node.JspAttribute(tldAttrs[j],
+                                jspAttrs[i] = new Node.JspAttribute(tldAttr,
                                         attrs.getQName(i), attrs.getURI(i), attrs
                                                 .getLocalName(i),
                                         attrs.getValue(i), false, null, false);
                             } else {
-                                
-                                if (deferred && !tldAttrs[j].isDeferredMethod() && !tldAttrs[j].isDeferredValue()) {
+
+                                if (deferred && !tldAttr.isDeferredMethod() && !tldAttr.isDeferredValue()) {
                                     // No deferred expressions allowed for this attribute
                                     err.jspError(n, "jsp.error.attribute.custom.non_rt_with_expr",
-                                            tldAttrs[j].getName());
+                                            tldAttr.getName());
                                 }
-                                if (!deferred && !tldAttrs[j].canBeRequestTime()) {
+                                if (!deferred && !tldAttr.canBeRequestTime()) {
                                     // Only deferred expressions are allowed for this attribute
                                     err.jspError(n, "jsp.error.attribute.custom.non_rt_with_expr",
-                                            tldAttrs[j].getName());
+                                            tldAttr.getName());
                                 }
                                 
                                 if (elExpression) {
                                     // El expression
                                     validateFunctions(el, n);
-                                    jspAttrs[i] = new Node.JspAttribute(tldAttrs[j],
+                                    jspAttrs[i] = new Node.JspAttribute(tldAttr,
                                             attrs.getQName(i), attrs.getURI(i), 
                                             attrs.getLocalName(i),
                                             attrs.getValue(i), false, el, false);
@@ -1206,7 +1198,7 @@ class Validator {
                                     }
                                 } else {
                                     // Runtime expression
-                                    jspAttrs[i] = getJspAttribute(tldAttrs[j],
+                                    jspAttrs[i] = getJspAttribute(tldAttr,
                                             attrs.getQName(i), attrs.getURI(i),
                                             attrs.getLocalName(i), attrs
                                             .getValue(i), n, false);
@@ -1218,9 +1210,9 @@ class Validator {
                             // Make sure its value does not contain any.
                             if (expression) {
                                 err.jspError(n, "jsp.error.attribute.custom.non_rt_with_expr",
-                                                tldAttrs[j].getName());
+                                                tldAttr.getName());
                             }
-                            jspAttrs[i] = new Node.JspAttribute(tldAttrs[j],
+                            jspAttrs[i] = new Node.JspAttribute(tldAttr,
                                     attrs.getQName(i), attrs.getURI(i), attrs
                                             .getLocalName(i),
                                     attrs.getValue(i), false, null, false);
@@ -1340,6 +1332,9 @@ class Validator {
                     result = new Node.JspAttribute(tai, qName, uri, localName,
                             value.substring(3, value.length() - 2), true, null,
                             dynamic);
+                } else if (pageInfo.isELIgnored()) {
+                    result = new Node.JspAttribute(tai, qName, uri, localName,
+                            value, false, null, dynamic);
                 } else {
                     // The attribute can contain expressions but is not a
                     // scriptlet expression; thus, we want to run it through
@@ -1347,22 +1342,10 @@ class Validator {
 
                     // validate expression syntax if string contains
                     // expression(s)
-                    ELNode.Nodes el = ELParser.parse(value);
-                    
-                    boolean deferred = false;
-                    Iterator<ELNode> nodes = el.iterator();
-                    while (nodes.hasNext()) {
-                        ELNode node = nodes.next();
-                        if (node instanceof ELNode.Root) {
-                            if (((ELNode.Root) node).getType() == '#') {
-                                deferred = true;
-                            }
-                        }
-                    }
+                    ELNode.Nodes el = ELParser.parse(value, pageInfo
+                            .isDeferredSyntaxAllowedAsLiteral());
 
-                    if (el.containsEL() && !pageInfo.isELIgnored()
-                            && ((!pageInfo.isDeferredSyntaxAllowedAsLiteral() && deferred)
-                                    || !deferred)) {
+                    if (el.containsEL()) {
 
                         validateFunctions(el, n);
 
@@ -1421,7 +1404,8 @@ class Validator {
             boolean elExpression = false;
 
             if (!runtimeExpression && !pageInfo.isELIgnored()) {
-                Iterator<ELNode> nodes = ELParser.parse(value).iterator();
+                Iterator<ELNode> nodes = ELParser.parse(value,
+                        pageInfo.isDeferredSyntaxAllowedAsLiteral()).iterator();
                 while (nodes.hasNext()) {
                     ELNode node = nodes.next();
                     if (node instanceof ELNode.Root) {
index e48b112..0ca8b0f 100644 (file)
@@ -372,6 +372,7 @@ jsp.error.prolog_pagedir_encoding_mismatch=Page-encoding specified in XML prolog
 jsp.error.prolog_config_encoding_mismatch=Page-encoding specified in XML prolog ({0}) is different from that specified in jsp-property-group ({1})
 jsp.error.attribute.custom.non_rt_with_expr=According to TLD or attribute directive in tag file, attribute {0} does not accept any expressions
 jsp.error.attribute.standard.non_rt_with_expr=The {0} attribute of the {1} standard action does not accept any expressions
+jsp.error.attribute.deferredmix=Cannot use both ${} and #{} EL expressions in the same attribute value
 jsp.error.scripting.variable.missing_name=Unable to determine scripting variable name from attribute {0}
 jasper.error.emptybodycontent.nonempty=According to TLD, tag {0} must be empty, but is not
 jsp.error.tagfile.nameNotUnique=The value of {0} and the value of {1} in line {2} are the same.
index 0a6b33b..4db2cba 100644 (file)
@@ -58,17 +58,16 @@ public class TestParser extends TomcatBaseTest {
         ByteChunk res = getUrl("http://localhost:" + getPort() +
                 "/test/bug48668a.jsp");
         String result = res.toString();
-        System.out.println(result);
         assertEcho(result, "00-Hello world</p>#{foo.bar}");
         assertEcho(result, "01-Hello world</p>${foo.bar}");
         assertEcho(result, "10-Hello ${'foo.bar}");
         assertEcho(result, "11-Hello ${'foo.bar}");
-        //assertEcho(result, "12-Hello #{'foo.bar}");
-        //assertEcho(result, "13-Hello #{'foo.bar}");
+        assertEcho(result, "12-Hello #{'foo.bar}");
+        assertEcho(result, "13-Hello #{'foo.bar}");
         assertEcho(result, "14-Hello ${'foo}");
         assertEcho(result, "15-Hello ${'foo}");
-        //assertEcho(result, "16-Hello #{'foo}");
-        //assertEcho(result, "17-Hello #{'foo}");
+        assertEcho(result, "16-Hello #{'foo}");
+        assertEcho(result, "17-Hello #{'foo}");
         assertEcho(result, "18-Hello ${'foo.bar}");
         assertEcho(result, "19-Hello ${'foo.bar}");
         assertEcho(result, "20-Hello #{'foo.bar}");
index b95958a..dd8bb7c 100644 (file)
 
     <p>10-<tags:bug48668 expr="Hello ${'foo.bar}" /></p>
     <p>11-Hello <tags:bug48668 expr="${'foo.bar}" /></p>
-<%--<p>12-<tags:bug48668 expr="Hello #{'foo.bar}" /></p>--%>
-<%--<p>13-Hello <tags:bug48668 expr="#{'foo.bar}" /></p>--%>
+    <p>12-<tags:bug48668 expr="Hello #{'foo.bar}" /></p>
+    <p>13-Hello <tags:bug48668 expr="#{'foo.bar}" /></p>
 
     <p>14-<tags:bug48668 expr="Hello ${'foo" />}</p>
     <p>15-Hello <tags:bug48668 expr="${'foo" />}</p>
-<%--<p>16-<tags:bug48668 expr="Hello #{'foo" />}</p>--%>
-<%--<p>17-Hello <tags:bug48668 expr="#{'foo" />}</p>--%>
+    <p>16-<tags:bug48668 expr="Hello #{'foo" />}</p>
+    <p>17-Hello <tags:bug48668 expr="#{'foo" />}</p>
 
     <p>18-<tags:bug48668 ><jsp:attribute name="expr">Hello ${'foo.bar}</jsp:attribute></tags:bug48668></p>
     <p>19-Hello <tags:bug48668 ><jsp:attribute name="expr">${'foo.bar}</jsp:attribute></tags:bug48668></p>