From 8937ec41dedbd1c13c5bb27c9864adcfae2d9363 Mon Sep 17 00:00:00 2001 From: kkolinko Date: Sun, 7 Mar 2010 02:43:12 +0000 Subject: [PATCH] Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48668 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 | 14 +++ java/org/apache/jasper/compiler/ELParser.java | 22 ++-- java/org/apache/jasper/compiler/JspConfig.java | 16 +-- java/org/apache/jasper/compiler/Validator.java | 114 +++++++++------------ .../jasper/resources/LocalStrings.properties | 1 + test/org/apache/jasper/compiler/TestParser.java | 9 +- test/webapp/bug48668a.jsp | 8 +- 7 files changed, 98 insertions(+), 86 deletions(-) diff --git a/java/org/apache/jasper/compiler/Compiler.java b/java/org/apache/jasper/compiler/Compiler.java index 1b79f9eb8..93287814b 100644 --- a/java/org/apache/jasper/compiler/Compiler.java +++ b/java/org/apache/jasper/compiler/Compiler.java @@ -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(); diff --git a/java/org/apache/jasper/compiler/ELParser.java b/java/org/apache/jasper/compiler/ELParser.java index ce17448d3..4a64b5603 100644 --- a/java/org/apache/jasper/compiler/ELParser.java +++ b/java/org/apache/jasper/compiler/ELParser.java @@ -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); diff --git a/java/org/apache/jasper/compiler/JspConfig.java b/java/org/apache/jasper/compiler/JspConfig.java index 944cda60e..7024b3d5e 100644 --- a/java/org/apache/jasper/compiler/JspConfig.java +++ b/java/org/apache/jasper/compiler/JspConfig.java @@ -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; diff --git a/java/org/apache/jasper/compiler/Validator.java b/java/org/apache/jasper/compiler/Validator.java index 3270a254c..0f0220c6a 100644 --- a/java/org/apache/jasper/compiler/Validator.java +++ b/java/org/apache/jasper/compiler/Validator.java @@ -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 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 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 nodes = ELParser.parse(value).iterator(); + Iterator nodes = ELParser.parse(value, + pageInfo.isDeferredSyntaxAllowedAsLiteral()).iterator(); while (nodes.hasNext()) { ELNode node = nodes.next(); if (node instanceof ELNode.Root) { diff --git a/java/org/apache/jasper/resources/LocalStrings.properties b/java/org/apache/jasper/resources/LocalStrings.properties index e48b11276..0ca8b0f0d 100644 --- a/java/org/apache/jasper/resources/LocalStrings.properties +++ b/java/org/apache/jasper/resources/LocalStrings.properties @@ -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. diff --git a/test/org/apache/jasper/compiler/TestParser.java b/test/org/apache/jasper/compiler/TestParser.java index 0a6b33ba1..4db2cba6d 100644 --- a/test/org/apache/jasper/compiler/TestParser.java +++ b/test/org/apache/jasper/compiler/TestParser.java @@ -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

#{foo.bar}"); assertEcho(result, "01-Hello world

${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}"); diff --git a/test/webapp/bug48668a.jsp b/test/webapp/bug48668a.jsp index b95958aaf..dd8bb7c03 100644 --- a/test/webapp/bug48668a.jsp +++ b/test/webapp/bug48668a.jsp @@ -24,13 +24,13 @@

10-

11-Hello

-<%--

12-

--%> -<%--

13-Hello

--%> +

12-

+

13-Hello

14-}

15-Hello }

-<%--

16-}

--%> -<%--

17-Hello }

--%> +

16-}

+

17-Hello }

18-Hello ${'foo.bar}

19-Hello ${'foo.bar}

-- 2.11.0