Author: remy.maucherat(a)jboss.com Date: 2008-09-19 11:22:38 -0400 (Fri, 19 Sep 2008) New Revision: 788 Modified: trunk/java/org/apache/jasper/Constants.java trunk/java/org/apache/jasper/compiler/Compiler.java trunk/java/org/apache/jasper/compiler/Generator.java trunk/java/org/apache/jasper/compiler/JspDocumentParser.java trunk/java/org/apache/jasper/compiler/JspUtil.java trunk/java/org/apache/jasper/compiler/Parser.java trunk/java/org/apache/jasper/compiler/ParserController.java trunk/java/org/apache/jasper/compiler/Validator.java trunk/webapps/docs/changelog.xml Log: - 45451: Rework escaping of EL. Modified: trunk/java/org/apache/jasper/Constants.java =================================================================== --- trunk/java/org/apache/jasper/Constants.java 2008-09-19 02:14:12 UTC (rev 787) +++ trunk/java/org/apache/jasper/Constants.java 2008-09-19 15:22:38 UTC (rev 788) @@ -181,13 +181,6 @@ System.getProperty("org.apache.jasper.Constants.TEMP_VARIABLE_NAME_PREFIX", "_jspx_temp"); /** - * A replacement char for "\$". - * XXX This is a hack to avoid changing EL interpreter to recognize "\$" - */ - public static final char ESC = '\u001b'; - public static final String ESCStr = "'\\u001b'"; - - /** * Has security been turned on? */ public static final boolean IS_SECURITY_ENABLED = Modified: trunk/java/org/apache/jasper/compiler/Compiler.java =================================================================== --- trunk/java/org/apache/jasper/compiler/Compiler.java 2008-09-19 02:14:12 UTC (rev 787) +++ trunk/java/org/apache/jasper/compiler/Compiler.java 2008-09-19 15:22:38 UTC (rev 788) @@ -146,8 +146,28 @@ ServletWriter writer = null; try { + /* + * The setting of isELIgnored changes the behaviour of the parser + * in subtle ways. To add to the 'fun', isELIgnored can be set in + * any file that forms part of the translation unit so setting it + * in a file included towards the end of the translation unit can + * change how the parser should have behaved when parsing content + * up to the point where isELIgnored was set. Arghh! + * Previous attempts to hack around this have only provided partial + * solutions. We now use two passes to parse the translation unit. + * The first just parses the directives and the second parses the + * whole translation unit once we know how isELIgnored has been set. + * TODO There are some possible optimisations of this process. + */ // Parse the file ParserController parserCtl = new ParserController(ctxt, this); + + // Pass 1 - the directives + Node.Nodes directives = + parserCtl.parseDirectives(ctxt.getJspFile()); + Validator.validateDirectives(this, directives); + + // Pass 2 - the whole translation unit pageNodes = parserCtl.parse(ctxt.getJspFile()); if (ctxt.isPrototypeMode()) { @@ -159,8 +179,9 @@ return null; } - // Validate and process attributes - Validator.validate(this, pageNodes); + // Validate and process attributes - don't re-validate the + // directives we validated in pass 1 + Validator.validateExDirectives(this, pageNodes); if (log.isDebugEnabled()) { t2 = System.currentTimeMillis(); Modified: trunk/java/org/apache/jasper/compiler/Generator.java =================================================================== --- trunk/java/org/apache/jasper/compiler/Generator.java 2008-09-19 02:14:12 UTC (rev 787) +++ trunk/java/org/apache/jasper/compiler/Generator.java 2008-09-19 15:22:38 UTC (rev 788) @@ -806,13 +806,8 @@ } return v; } else if (attr.isELInterpreterInput()) { - boolean replaceESC = v.indexOf(Constants.ESC) > 0; v = JspUtil.interpreterCall(this.isTagFile, v, expectedType, attr.getEL().getMapName(), false); - // XXX ESC replacement hack - if (replaceESC) { - v = "(" + v + ").replace(" + Constants.ESCStr + ", '$')"; - } if (encode) { return "org.apache.jasper.runtime.JspRuntimeLibrary.URLEncode(" + v + ", request.getCharacterEncoding())"; @@ -2843,16 +2838,10 @@ attrValue = sb.toString(); } else { // run attrValue through the expression interpreter - boolean replaceESC = attrValue.indexOf(Constants.ESC) > 0; String mapName = (attr.getEL() != null) ? attr.getEL() .getMapName() : null; attrValue = JspUtil.interpreterCall(this.isTagFile, attrValue, c[0], mapName, false); - // XXX hack: Replace ESC with '$' - if (replaceESC) { - attrValue = "(" + attrValue + ").replace(" - + Constants.ESCStr + ", '$')"; - } } } else { attrValue = convertString(c[0], attrValue, localName, Modified: trunk/java/org/apache/jasper/compiler/JspDocumentParser.java =================================================================== --- trunk/java/org/apache/jasper/compiler/JspDocumentParser.java 2008-09-19 02:14:12 UTC (rev 787) +++ trunk/java/org/apache/jasper/compiler/JspDocumentParser.java 2008-09-19 15:22:38 UTC (rev 788) @@ -580,6 +580,9 @@ lastCh = ch; } } else if (lastCh == '\\' && (ch == '$' || ch == '#')) { + if (pageInfo.isELIgnored()) { + ttext.write('\\'); + } ttext.write(ch); ch = 0; // Not start of EL anymore } else { Modified: trunk/java/org/apache/jasper/compiler/JspUtil.java =================================================================== --- trunk/java/org/apache/jasper/compiler/JspUtil.java 2008-09-19 02:14:12 UTC (rev 787) +++ trunk/java/org/apache/jasper/compiler/JspUtil.java 2008-09-19 15:22:38 UTC (rev 788) @@ -191,7 +191,7 @@ returnString = expression; } - return escapeXml(returnString.replace(Constants.ESC, '$')); + return escapeXml(returnString); } /** Modified: trunk/java/org/apache/jasper/compiler/Parser.java =================================================================== --- trunk/java/org/apache/jasper/compiler/Parser.java 2008-09-19 02:14:12 UTC (rev 787) +++ trunk/java/org/apache/jasper/compiler/Parser.java 2008-09-19 15:22:38 UTC (rev 788) @@ -27,7 +27,6 @@ import javax.servlet.jsp.tagext.TagInfo; import javax.servlet.jsp.tagext.TagLibraryInfo; -import org.apache.jasper.Constants; import org.apache.jasper.JasperException; import org.apache.jasper.JspCompilationContext; import org.xml.sax.Attributes; @@ -264,18 +263,19 @@ private String parseQuoted(Mark start, String tx, char quote) throws JasperException { StringBuffer buf = new StringBuffer(); + boolean possibleEL = tx.contains("${"); int size = tx.length(); int i = 0; while (i < size) { char ch = tx.charAt(i); if (ch == '&') { if (i + 5 < size && tx.charAt(i + 1) == 'a' - && tx.charAt(i + 2) == 'p' && tx.charAt(i + 3) == 'o' + && tx.charAt(i + 2) == 'p' && tx.charAt(i + 3) == 'o' && tx.charAt(i + 4) == 's' && tx.charAt(i + 5) == ';') { buf.append('\''); i += 6; } else if (i + 5 < size && tx.charAt(i + 1) == 'q' - && tx.charAt(i + 2) == 'u' && tx.charAt(i + 3) == 'o' + && tx.charAt(i + 2) == 'u' && tx.charAt(i + 3) == 'o' && tx.charAt(i + 4) == 't' && tx.charAt(i + 5) == ';') { buf.append('"'); i += 6; @@ -285,13 +285,22 @@ } } else if (ch == '\\' && i + 1 < size) { ch = tx.charAt(i + 1); - if (ch == '\\' || ch == '\"' || ch == '\'' || ch == '>') { + if (ch == '\\' || ch == '\"' || ch == '\'') { + if (pageInfo.isELIgnored() || !possibleEL) { + // EL is not enabled or no chance of EL + // Unescape these now + buf.append(ch); + i += 2; + } else { + // EL is enabled and ${ appears in value + // EL processing will escape these + buf.append('\\'); + buf.append(ch); + i += 2; + } + } else if (ch == '>') { buf.append(ch); i += 2; - } else if (ch == '$') { - // Replace "\$" with some special char. XXX hack! - buf.append(Constants.ESC); - i += 2; } else { buf.append('\\'); ++i; Modified: trunk/java/org/apache/jasper/compiler/ParserController.java =================================================================== --- trunk/java/org/apache/jasper/compiler/ParserController.java 2008-09-19 02:14:12 UTC (rev 787) +++ trunk/java/org/apache/jasper/compiler/ParserController.java 2008-09-19 15:22:38 UTC (rev 788) @@ -104,6 +104,23 @@ } /** + * Parses the directives of a JSP page or tag file. This is invoked by the + * compiler. + * + * @param inFileName The path to the JSP page or tag file to be parsed. + */ + public Node.Nodes parseDirectives(String inFileName) + throws FileNotFoundException, JasperException, IOException { + // If we're parsing a packaged tag file or a resource included by it + // (using an include directive), ctxt.getTagFileJar() returns the + // JAR file from which to read the tag file or included resource, + // respectively. + isTagFile = ctxt.isTagFile(); + directiveOnly = true; + return doParse(inFileName, null, ctxt.getTagFileJarUrl()); + } + + /** * Processes an include directive with the given path. * * @param inFileName The path to the resource to be included. Modified: trunk/java/org/apache/jasper/compiler/Validator.java =================================================================== --- trunk/java/org/apache/jasper/compiler/Validator.java 2008-09-19 02:14:12 UTC (rev 787) +++ trunk/java/org/apache/jasper/compiler/Validator.java 2008-09-19 15:22:38 UTC (rev 788) @@ -1337,7 +1337,6 @@ } } else { - value = value.replace(Constants.ESC, '$'); result = new Node.JspAttribute(tai, qName, uri, localName, value, false, null, dynamic); } @@ -1692,16 +1691,14 @@ } } - public static void validate(Compiler compiler, Node.Nodes page) - throws JasperException { - - /* - * Visit the page/tag directives first, as they are global to the page - * and are position independent. - */ + public static void validateDirectives(Compiler compiler, Node.Nodes page) + throws JasperException { page.visit(new DirectiveVisitor(compiler)); + } - // Determine the default output content type + public static void validateExDirectives(Compiler compiler, Node.Nodes page) + throws JasperException { +// Determine the default output content type PageInfo pageInfo = compiler.getPageInfo(); String contentType = pageInfo.getContentType(); Modified: trunk/webapps/docs/changelog.xml =================================================================== --- trunk/webapps/docs/changelog.xml 2008-09-19 02:14:12 UTC (rev 787) +++ trunk/webapps/docs/changelog.xml 2008-09-19 15:22:38 UTC (rev 788) @@ -54,6 +54,11 @@ <fix> Fix bad EL exception cast. (rjung) </fix> + <fix> + <bug>45451</bug>: Testing for this threw up all sorts of other failures around use of \${...} These + should all now be fixed. The two pass parsing means we can do away with the previous 'replace with + unused unicode character' trick. (markt) + </fix> </changelog> </subsection> </section>