Author: kkolinko Date: Sun Mar 7 02:43:12 2010 New Revision: 919914 URL: http://svn.apache.org/viewvc?rev=919914&view=rev Log: 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. Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java tomcat/trunk/java/org/apache/jasper/compiler/JspConfig.java tomcat/trunk/java/org/apache/jasper/compiler/Validator.java tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java tomcat/trunk/test/webapp/bug48668a.jsp Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java?rev=919914&r1=919913&r2=919914&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Sun Mar 7 02:43:12 2010 @@ -152,6 +152,20 @@ 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(); 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=919914&r1=919913&r2=919914&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java Sun Mar 7 02:43:12 2010 @@ -45,13 +45,16 @@ 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 @@ * @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 @@ 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 @@ buf.append(prev); prev = 0; } - if (ch == '\\' || ch == '$' || ch == '#') { + if (ch == '\\' || ch == '$' + || (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) { prev = ch; } else { buf.append(ch); Modified: tomcat/trunk/java/org/apache/jasper/compiler/JspConfig.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/JspConfig.java?rev=919914&r1=919913&r2=919914&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/JspConfig.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/JspConfig.java Sun Mar 7 02:43:12 2010 @@ -45,14 +45,14 @@ 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 @@ 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; Modified: tomcat/trunk/java/org/apache/jasper/compiler/Validator.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Validator.java?rev=919914&r1=919913&r2=919914&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/Validator.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Validator.java Sun Mar 7 02:43:12 2010 @@ -418,8 +418,6 @@ private ErrorDispatcher err; - private TagInfo tagInfo; - private ClassLoader loader; private final StringBuilder buf = new StringBuilder(32); @@ -510,7 +508,6 @@ 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 @@ // 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 @@ 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 @@ 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 @@ || (!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 @@ // 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 @@ } 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 @@ } 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 @@ } } 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 @@ // 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 @@ 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 @@ // 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 @@ 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) { Modified: tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties?rev=919914&r1=919913&r2=919914&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties Sun Mar 7 02:43:12 2010 @@ -372,6 +372,7 @@ 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. Modified: tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java?rev=919914&r1=919913&r2=919914&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java (original) +++ tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java Sun Mar 7 02:43:12 2010 @@ -58,17 +58,16 @@ 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}"); Modified: tomcat/trunk/test/webapp/bug48668a.jsp URL: http://svn.apache.org/viewvc/tomcat/trunk/test/webapp/bug48668a.jsp?rev=919914&r1=919913&r2=919914&view=diff ============================================================================== --- tomcat/trunk/test/webapp/bug48668a.jsp (original) +++ tomcat/trunk/test/webapp/bug48668a.jsp Sun Mar 7 02:43:12 2010 @@ -24,13 +24,13 @@ <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> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org