tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
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 GMT
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


Mime
View raw message