Return-Path: Delivered-To: apmail-tomcat-dev-archive@www.apache.org Received: (qmail 53309 invoked from network); 16 Sep 2008 21:13:34 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 16 Sep 2008 21:13:34 -0000 Received: (qmail 38832 invoked by uid 500); 16 Sep 2008 21:13:27 -0000 Delivered-To: apmail-tomcat-dev-archive@tomcat.apache.org Received: (qmail 38630 invoked by uid 500); 16 Sep 2008 21:13:27 -0000 Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Tomcat Developers List" Delivered-To: mailing list dev@tomcat.apache.org Received: (qmail 38619 invoked by uid 99); 16 Sep 2008 21:13:26 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 16 Sep 2008 14:13:26 -0700 X-ASF-Spam-Status: No, hits=-1998.2 required=10.0 tests=ALL_TRUSTED,SUBJECT_DRUG_GAP_L X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 16 Sep 2008 21:12:36 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 17ACC23889B9; Tue, 16 Sep 2008 14:12:39 -0700 (PDT) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r696061 - in /tomcat/trunk/java/org/apache/jasper: Constants.java compiler/Compiler.java compiler/Generator.java compiler/JspDocumentParser.java compiler/JspUtil.java compiler/Parser.java compiler/ParserController.java compiler/Validator.java Date: Tue, 16 Sep 2008 21:12:38 -0000 To: dev@tomcat.apache.org From: markt@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20080916211239.17ACC23889B9@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: markt Date: Tue Sep 16 14:12:38 2008 New Revision: 696061 URL: http://svn.apache.org/viewvc?rev=696061&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=45451 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. Modified: tomcat/trunk/java/org/apache/jasper/Constants.java tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java tomcat/trunk/java/org/apache/jasper/compiler/Generator.java tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java tomcat/trunk/java/org/apache/jasper/compiler/Parser.java tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java tomcat/trunk/java/org/apache/jasper/compiler/Validator.java Modified: tomcat/trunk/java/org/apache/jasper/Constants.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/Constants.java?rev=696061&r1=696060&r2=696061&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/Constants.java (original) +++ tomcat/trunk/java/org/apache/jasper/Constants.java Tue Sep 16 14:12:38 2008 @@ -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: 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=696061&r1=696060&r2=696061&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Tue Sep 16 14:12:38 2008 @@ -145,8 +145,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()) { @@ -158,8 +178,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: tomcat/trunk/java/org/apache/jasper/compiler/Generator.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Generator.java?rev=696061&r1=696060&r2=696061&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Tue Sep 16 14:12:38 2008 @@ -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())"; @@ -2839,16 +2834,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: tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java?rev=696061&r1=696060&r2=696061&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java Tue Sep 16 14:12:38 2008 @@ -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: tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java?rev=696061&r1=696060&r2=696061&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java Tue Sep 16 14:12:38 2008 @@ -177,7 +177,7 @@ returnString = expression; } - return escapeXml(returnString.replace(Constants.ESC, '$')); + return escapeXml(returnString); } /** Modified: tomcat/trunk/java/org/apache/jasper/compiler/Parser.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Parser.java?rev=696061&r1=696060&r2=696061&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/Parser.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Parser.java Tue Sep 16 14:12:38 2008 @@ -266,6 +266,7 @@ 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) { @@ -287,13 +288,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; @@ -1339,11 +1349,8 @@ } char next = (char) reader.peekChar(); // Looking for \% or \$ or \# - // TODO: only recognize \$ or \# if isELIgnored is false, but since - // it can be set in a page directive, it cannot be determined - // here. Argh! (which is the way it should be since we shouldn't - // convolude multiple steps at once and create confusing parsers...) - if (next == '%' || next == '$' || next == '#') { + if (next == '%' || ((next == '$' || next == '#') && + !pageInfo.isELIgnored())) { ch = reader.nextChar(); } } Modified: tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java?rev=696061&r1=696060&r2=696061&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java Tue Sep 16 14:12:38 2008 @@ -104,6 +104,24 @@ } /** + * 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: 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=696061&r1=696060&r2=696061&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/Validator.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Validator.java Tue Sep 16 14:12:38 2008 @@ -1311,7 +1311,6 @@ } } else { - value = value.replace(Constants.ESC, '$'); result = new Node.JspAttribute(tai, qName, uri, localName, value, false, null, dynamic); } @@ -1666,15 +1665,13 @@ } } - public static void validate(Compiler compiler, Node.Nodes page) + public static void validateDirectives(Compiler compiler, Node.Nodes page) throws JasperException { - - /* - * Visit the page/tag directives first, as they are global to the page - * and are position independent. - */ page.visit(new DirectiveVisitor(compiler)); + } + 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(); --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org For additional commands, e-mail: dev-help@tomcat.apache.org