Author: markt
Date: Wed Jun 24 20:12:03 2009
New Revision: 788167

URL: http://svn.apache.org/viewvc?rev=788167&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=36923
If EL is disabled, treat ${ as template text
This patch includes a port of the work from TC6 that removed the need for the 
text replacement hack as part of the EL processing.

Modified:
    tomcat/container/tc5.5.x/webapps/docs/changelog.xml
    tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/Constants.java
    tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Compiler.java
    tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Generator.java
    
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspDocumentParser.java
    tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspUtil.java
    tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Parser.java
    
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/ParserController.java
    tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Validator.java

Modified: tomcat/container/tc5.5.x/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/webapps/docs/changelog.xml?rev=788167&r1=788166&r2=788167&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/webapps/docs/changelog.xml (original)
+++ tomcat/container/tc5.5.x/webapps/docs/changelog.xml Wed Jun 24 20:12:03 2009
@@ -163,6 +163,9 @@
   <subsection name="Jasper">
     <changelog>
       <fix>
+        <bug>36923</bug>: Parse deactivated EL expressions correctly. (markt) 
+      </fix>
+      <fix>
         <bug>37515</bug>: Add options for Java 1.6 and 1.7 to the JDT compiler.
         (markt)
       </fix>

Modified: tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/Constants.java
URL: 
http://svn.apache.org/viewvc/tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/Constants.java?rev=788167&r1=788166&r2=788167&view=diff
==============================================================================
--- tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/Constants.java (original)
+++ tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/Constants.java Wed Jun 24 
20:12:03 2009
@@ -201,6 +201,7 @@
      * Replacement char for "\$". This is the first unicode character in the
      * private use area.
      * XXX This is a hack to avoid changing EL interpreter to recognize "\$"
+     * @deprecated
      */
     public static final char HACK_CHAR = '\ue000';
     
@@ -208,6 +209,7 @@
      * Replacement string for "\$". This is the first unicode character in the
      * private use area.
      * XXX This is a hack to avoid changing EL interpreter to recognize "\$"
+     * @deprecated
      */
     public static final String HACK_STR = "'\\ue000'";
     

Modified: 
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Compiler.java
URL: 
http://svn.apache.org/viewvc/tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Compiler.java?rev=788167&r1=788166&r2=788167&view=diff
==============================================================================
--- tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Compiler.java 
(original)
+++ tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Compiler.java 
Wed Jun 24 20:12:03 2009
@@ -151,8 +151,28 @@
             // Reset the temporary variable counter for the generator.
             JspUtil.resetTemporaryVariableName();
 
+            /*
+             * 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()) {
@@ -163,8 +183,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/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Generator.java
URL: 
http://svn.apache.org/viewvc/tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Generator.java?rev=788167&r1=788166&r2=788167&view=diff
==============================================================================
--- tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Generator.java 
(original)
+++ tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Generator.java 
Wed Jun 24 20:12:03 2009
@@ -761,7 +761,6 @@
                 }
                 return v;
             } else if (attr.isELInterpreterInput()) {
-                boolean replaceESC = v.indexOf(Constants.HACK_CHAR) > 0;
                 v =
                     JspUtil.interpreterCall(
                         this.isTagFile,
@@ -769,10 +768,6 @@
                         expectedType,
                         attr.getEL().getMapName(),
                         false);
-                // XXX hack replacement
-                if (replaceESC) {
-                    v = "(" + v + ").replace(" + Constants.HACK_STR + ", '$')";
-                }
                 if (encode) {
                     return 
"org.apache.jasper.runtime.JspRuntimeLibrary.URLEncode("
                         + v
@@ -1896,17 +1891,6 @@
                 return;
             }
 
-            // Replace marker for \$ sequence with correct sequence
-            if (text.indexOf(Constants.HACK_CHAR) > 0) {
-                if (pageInfo.isELIgnored()) {
-                    text = text.replaceAll(String.valueOf(Constants.HACK_CHAR),
-                            "\\\\\\$");
-                    textSize++;
-                } else {
-                    text = text.replace(Constants.HACK_CHAR, '$');
-                }
-            }
-
             if (textSize <= 3) {
                // Special case small text strings
                n.setBeginJavaLine(out.getJavaLine());
@@ -2752,7 +2736,6 @@
                 }
             } else if (attr.isELInterpreterInput()) {
                 // run attrValue through the expression interpreter
-                boolean replaceESC = attrValue.indexOf(Constants.HACK_CHAR) > 
0;
                 attrValue =
                     JspUtil.interpreterCall(
                         this.isTagFile,
@@ -2760,15 +2743,6 @@
                         c[0],
                         attr.getEL().getMapName(),
                         false);
-                // XXX hack: Replace ESC with '$'
-                if (replaceESC) {
-                    attrValue =
-                        "("
-                            + attrValue
-                            + ").replace("
-                            + Constants.HACK_STR
-                            + ", '$')";
-                }
             } else {
                 attrValue =
                     convertString(

Modified: 
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspDocumentParser.java
URL: 
http://svn.apache.org/viewvc/tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspDocumentParser.java?rev=788167&r1=788166&r2=788167&view=diff
==============================================================================
--- 
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspDocumentParser.java
 (original)
+++ 
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspDocumentParser.java
 Wed Jun 24 20:12:03 2009
@@ -577,6 +577,9 @@
                         lastCh = ch;
                     }
                 } else if (lastCh == '\\' && ch == '$') {
+                    if (pageInfo.isELIgnored()) {
+                        ttext.write('\\');
+                    }
                     ttext.write('$');
                     ch = 0;  // Not start of EL anymore
                 } else {

Modified: 
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspUtil.java
URL: 
http://svn.apache.org/viewvc/tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspUtil.java?rev=788167&r1=788166&r2=788167&view=diff
==============================================================================
--- tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspUtil.java 
(original)
+++ tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/JspUtil.java Wed 
Jun 24 20:12:03 2009
@@ -187,7 +187,7 @@
             returnString = expression;
         }
 
-        return escapeXml(returnString.replace(Constants.HACK_CHAR, '$'));
+        return escapeXml(returnString);
     }
 
     /**

Modified: tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Parser.java
URL: 
http://svn.apache.org/viewvc/tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Parser.java?rev=788167&r1=788166&r2=788167&view=diff
==============================================================================
--- tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Parser.java 
(original)
+++ tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Parser.java Wed 
Jun 24 20:12:03 2009
@@ -118,20 +118,19 @@
         root.setJspConfigPageEncoding(jspConfigPageEnc);
         root.setIsDefaultPageEncoding(isDefaultPageEncoding);
 
-        if (directivesOnly) {
-            parser.parseTagFileDirectives(root);
-            return new Node.Nodes(root);
-        }
-
-        // For the Top level page, add inlcude-prelude and include-coda
+        // For the Top level page, add include-prelude and include-coda
         PageInfo pageInfo = pc.getCompiler().getPageInfo();
-        if (parent == null) {
+        if (parent == null && !isTagFile) {
             parser.addInclude(root, pageInfo.getIncludePrelude());
         }
-        while (reader.hasMoreInput()) {
-            parser.parseElements(root);
+        if (directivesOnly) {
+            parser.parseFileDirectives(root);
+        } else {
+            while (reader.hasMoreInput()) {
+                parser.parseElements(root);
+            }
         }
-        if (parent == null) {
+        if (parent == null && !isTagFile) {
             parser.addInclude(root, pageInfo.getIncludeCoda());
         }
 
@@ -294,10 +293,6 @@
                 if (ch == '\\' || ch == '\"' || ch == '\'' || ch == '>') {
                     buf.append(ch);
                     i += 2;
-                } else if (ch == '$') {
-                    // Replace "\$" with some special char.  XXX hack!
-                    buf.append(Constants.HACK_CHAR);
-                    i += 2;
                 } else {
                     buf.append('\\');
                     ++i;
@@ -1415,7 +1410,7 @@
                 reader.pushChar();
                 break;
             }
-            else if( ch == '$' ) {
+            else if( ch == '$' && !pageInfo.isELIgnored()) {
                 if (!reader.hasMoreInput()) {
                     ttext.write('$');
                     break;
@@ -1436,15 +1431,10 @@
                     break;
                 }
                 // Look for \% or \$
-                // Only recognize \$ if isELIgnored is false, but since it can
-                // be set in a page directive, it cannot be determined yet.
+                // Only recognize \$ if isELIgnored is false
                 char next = (char)reader.peekChar();
-                if (next == '%') {
+                if (next == '%' || (next == '$' && !pageInfo.isELIgnored())) {
                     ch = reader.nextChar();
-                } else if(next == '$') {
-                    // Skip the $ and use a hack to flag this sequence
-                    reader.nextChar();
-                    ch = Constants.HACK_CHAR;
                 }
             }
             ttext.write(ch);
@@ -1582,7 +1572,7 @@
             parseXMLScriptlet(parent);
         } else if (reader.matches("<jsp:text")) {
             parseXMLTemplateText(parent);
-        } else if (reader.matches("${")) {
+        } else if (reader.matches("${") && !pageInfo.isELIgnored()) {
             parseELExpression(parent);
         } else if (reader.matches("<jsp:")) {
             parseStandardAction(parent);
@@ -1927,7 +1917,7 @@
         return JAVAX_BODY_CONTENT_TEMPLATE_TEXT;
     }
 
-    private void parseTagFileDirectives(Node parent)
+    private void parseFileDirectives(Node parent)
         throws JasperException
     {
         reader.setSingleFile(true);

Modified: 
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/ParserController.java
URL: 
http://svn.apache.org/viewvc/tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/ParserController.java?rev=788167&r1=788166&r2=788167&view=diff
==============================================================================
--- 
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/ParserController.java
 (original)
+++ 
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/ParserController.java
 Wed Jun 24 20:12:03 2009
@@ -102,6 +102,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: 
tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Validator.java
URL: 
http://svn.apache.org/viewvc/tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Validator.java?rev=788167&r1=788166&r2=788167&view=diff
==============================================================================
--- tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Validator.java 
(original)
+++ tomcat/jasper/tc5.5.x/src/share/org/apache/jasper/compiler/Validator.java 
Wed Jun 24 20:12:03 2009
@@ -1118,7 +1118,6 @@
                                                        value, false, el,
                                                        dynamic);
                     } else {
-                        value = value.replace(Constants.HACK_CHAR, '$');
                         result = new Node.JspAttribute(qName, uri, localName,
                                                        value, false, null,
                                                        dynamic);
@@ -1437,15 +1436,13 @@
         }
     }
 
-    public static void validate(Compiler compiler,
+    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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to