Author: markt
Date: Wed Feb 24 09:17:14 2010
New Revision: 915729

URL: http://svn.apache.org/viewvc?rev=915729&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48627
Regression in re-working of EL parsing
Keep literals as literals
Also handle deferredSyntaxAllowedAsLiteral

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/AttributeParser.java
    tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Parser.java
    
tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestAttributeParser.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=915729&r1=915728&r2=915729&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Feb 24 09:17:14 2010
@@ -76,17 +76,6 @@
                if we remove the SCP auto feature, then there should be 
something to replace it with
                (http://ant.apache.org/manual/OptionalTasks/scp.html)  
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48627
-  Regression in re-working of EL parsing
-  Keep literals as literals
-  Also handle deferredSyntaxAllowedAsLiteral
-  JUnit test cases and TCK passes with this patch
-  OP confirms patch fixes issue
-  Combined patch file, from revs.904949, 905226, 906465:
-  https://issues.apache.org/bugzilla/attachment.cgi?id=24925
-  +1: kkolinko, markt, jfclere
-  -1:
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48050
   NamingContext.createSubcontext method returns Context with wrong name
   Based on a suggestion by gingyang.xu

Modified: 
tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/AttributeParser.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/AttributeParser.java?rev=915729&r1=915728&r2=915729&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/AttributeParser.java 
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/AttributeParser.java 
Wed Feb 24 09:17:14 2010
@@ -24,7 +24,7 @@
  * "\${1+1}". After unquoting, both appear as "${1+1}" but the first should
  * evaluate to "2" and the second to "${1+1}". Literal \, $ and # need special
  * treatment to ensure there is no ambiguity. The JSP attribute unquoting
- * covers \\, \", \', \$, \#, %\>, <\%, &apos; and &quot;
+ * covers \\, \", \', \$, \#, %\&gt;, &lt;\%, &amp;apos; and &amp;quot;
  */
 public class AttributeParser {
 
@@ -43,13 +43,16 @@
      *                      scripting expressions.
      * @param isELIgnored   Is expression language being ignored on the page
      *                      where the JSP attribute is defined.
+     * @param isDeferredSyntaxAllowedAsLiteral
+     *                      Are deferred expressions treated as literals?
      * @return              An unquoted JSP attribute that, if it contains
      *                      expression language can be safely passed to the EL
      *                      processor without fear of ambiguity.
      */
     public static String getUnquoted(String input, char quote,
-            boolean isELIgnored) {
+            boolean isELIgnored, boolean isDeferredSyntaxAllowedAsLiteral) {
         return (new AttributeParser(input, quote, isELIgnored,
+                isDeferredSyntaxAllowedAsLiteral,
                 STRICT_QUOTE_ESCAPING)).getUnquoted();
     }
 
@@ -62,15 +65,18 @@
      *                      scripting expressions.
      * @param isELIgnored   Is expression language being ignored on the page
      *                      where the JSP attribute is defined.
+     * @param isDeferredSyntaxAllowedAsLiteral
+     *                      Are deferred expressions treated as literals?
      * @param strict        The value to use for STRICT_QUOTE_ESCAPING.
      * @return              An unquoted JSP attribute that, if it contains
      *                      expression language can be safely passed to the EL
      *                      processor without fear of ambiguity.
      */
     protected static String getUnquoted(String input, char quote,
-            boolean isELIgnored, boolean strict) {
+            boolean isELIgnored, boolean isDeferredSyntaxAllowedAsLiteral,
+            boolean strict) {
         return (new AttributeParser(input, quote, isELIgnored,
-                strict)).getUnquoted();
+                isDeferredSyntaxAllowedAsLiteral, strict)).getUnquoted();
     }
 
     /* The quoted input string. */
@@ -83,6 +89,9 @@
      * treated as literals rather than quoted values. */
     private final boolean isELIgnored;
     
+    /* Are deferred expression treated as literals */
+    private final boolean isDeferredSyntaxAllowedAsLiteral;
+    
     /* Overrides the STRICT_QUOTE_ESCAPING. Used for Unit tests only. */
     private final boolean strict;
     
@@ -109,12 +118,15 @@
      * @param strict
      */
     private AttributeParser(String input, char quote,
-            boolean isELIgnored, boolean strict) {
+            boolean isELIgnored, boolean isDeferredSyntaxAllowedAsLiteral,
+            boolean strict) {
         this.input = input;
         this.quote = quote;
         // If quote is null this is a scriptign expressions and any EL syntax
         // should be ignored
         this.isELIgnored = isELIgnored || (quote == 0);
+        this.isDeferredSyntaxAllowedAsLiteral =
+            isDeferredSyntaxAllowedAsLiteral;
         this.strict = strict;
         this.type = getType(input);
         this.size = input.length();
@@ -151,22 +163,27 @@
             char ch = nextChar();
             if (!isELIgnored && ch == '\\') {
                 if (type == 0) {
-                    type = '$';
+                    result.append("\\");
+                } else {
+                    result.append(type);
+                    result.append("{'\\\\'}");
                 }
-                result.append(type);
-                result.append("{'\\\\'}");
             } else if (!isELIgnored && ch == '$' && lastChEscaped){
                 if (type == 0) {
-                    type = '$';
+                    result.append("\\$");
+                } else {
+                    result.append(type);
+                    result.append("{'$'}");
                 }
-                result.append(type);
-                result.append("{'$'}");
             } else if (!isELIgnored && ch == '#' && lastChEscaped){
+                // Note if isDeferredSyntaxAllowedAsLiteral==true, \# will
+                // not be treated as an escape
                 if (type == 0) {
-                    type = '$';
+                    result.append("\\#");
+                } else {
+                    result.append(type);
+                    result.append("{'#'}");
                 }
-                result.append(type);
-                result.append("{'#'}");
             } else if (ch == type){
                 if (i < size) {
                     char next = input.charAt(i);
@@ -197,8 +214,8 @@
     private void parseEL() {
         boolean endEL = false;
         boolean insideLiteral = false;
+        char literalQuote = 0;
         while (i < size && !endEL) {
-            char literalQuote = '\'';
             char ch = nextChar();
             if (ch == '\'' || ch == '\"') {
                 if (insideLiteral) {
@@ -261,7 +278,10 @@
         } else if (ch == '\\' && i + 1 < size) {
             ch = input.charAt(i + 1);
             if (ch == '\\' || ch == '\"' || ch == '\'' ||
-                    (!isELIgnored && (ch == '$' || ch == '#'))) {
+                    (!isELIgnored &&
+                            (ch == '$' ||
+                                    (!isDeferredSyntaxAllowedAsLiteral &&
+                                            ch == '#')))) {
                 i += 2;
                 lastChEscaped = true;
             } else {
@@ -311,13 +331,13 @@
         int j = 0;
         int len = value.length();
         char current;
-        
+
         while (j < len) {
             current = value.charAt(j);
             if (current == '\\') {
                 // Escape character - skip a character
                 j++;
-            } else if (current == '#') {
+            } else if (current == '#' && !isDeferredSyntaxAllowedAsLiteral) {
                 if (j < (len -1) && value.charAt(j + 1) == '{') {
                     return '#';
                 }

Modified: tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Parser.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Parser.java?rev=915729&r1=915728&r2=915729&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Parser.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Parser.java Wed Feb 24 
09:17:14 2010
@@ -247,7 +247,8 @@
                 quote = watch.charAt(0);
             }
             ret = AttributeParser.getUnquoted(reader.getText(start, stop),
-                    quote, pageInfo.isELIgnored());
+                    quote, pageInfo.isELIgnored(),
+                    pageInfo.isDeferredSyntaxAllowedAsLiteral());
         } catch (IllegalArgumentException iae) {
             err.jspError(start, iae.getMessage());
         }

Modified: 
tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestAttributeParser.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestAttributeParser.java?rev=915729&r1=915728&r2=915729&view=diff
==============================================================================
--- 
tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestAttributeParser.java 
(original)
+++ 
tomcat/tc6.0.x/trunk/test/org/apache/jasper/compiler/TestAttributeParser.java 
Wed Feb 24 09:17:14 2010
@@ -134,9 +134,24 @@
         // Quoting <% and %>
         assertEquals("hello <% world", evalAttr("hello <\\% world", '\"'));
         assertEquals("hello %> world", evalAttr("hello %> world", '\"'));
+
+        // Test that the end of literal in EL expression is recognized in
+        // parseEL(), be it quoted with single or double quotes. That is, that
+        // AttributeParser correctly switches between parseLiteral and parseEL
+        // methods.
+        //
+        // The test is based on the difference in how the '\' character is 
printed:
+        // when in parseLiteral \\${ will be printed as ${'\'}${, but if we 
are still
+        // inside of parseEL it will be printed as \${, thus preventing the EL
+        // expression that follows from being evaluated.
+        //
+        assertEquals("foo\\bar\\baz", 
evalAttr("${\'foo\'}\\\\${\'bar\'}\\\\${\'baz\'}", '\"'));
+        assertEquals("foo\\bar\\baz", 
evalAttr("${\'foo\'}\\\\${\\\"bar\\\"}\\\\${\'baz\'}", '\"'));
+        assertEquals("foo\\bar\\baz", 
evalAttr("${\\\"foo\\\"}\\\\${\'bar\'}\\\\${\\\"baz\\\"}", '\"'));
+        assertEquals("foo\\bar\\baz", 
evalAttr("${\"foo\"}\\\\${\\\'bar\\\'}\\\\${\"baz\"}", '\''));
     }
 
-    public void testScriptExpressiinLiterals() {
+    public void testScriptExpressionLiterals() {
         assertEquals(" \"hello world\" ", parseScriptExpression(
                 " \"hello world\" ", (char) 0));
         assertEquals(" \"hello \\\"world\" ", parseScriptExpression(
@@ -149,13 +164,15 @@
         ctx.setFunctionMapper(new FMapper());
         ExpressionFactoryImpl exprFactory = new ExpressionFactoryImpl();
         ValueExpression ve = exprFactory.createValueExpression(ctx,
-                AttributeParser.getUnquoted(expression, quote, false, false),
+                AttributeParser.getUnquoted(expression, quote, false, false,
+                        false),
                 String.class);
         return (String) ve.getValue(ctx);
     }
     
     private String parseScriptExpression(String expression, char quote) {
-        return AttributeParser.getUnquoted(expression, quote, false, false);
+        return AttributeParser.getUnquoted(expression, quote, false, false,
+                false);
     }
 
     public static class FMapper extends FunctionMapper {

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=915729&r1=915728&r2=915729&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Wed Feb 24 09:17:14 2010
@@ -152,6 +152,11 @@
         regression in the first fix. (kkolinko) 
       </fix>
       <fix>
+        <bug>48627</bug>: Fix regression in re-factored EL parsing. Keep
+        literals as literals and handle deferredSyntaxAllowedAsLiteral.
+        (kkolinko)
+      </fix>
+      <fix>
         <bug>48668</bug>: When parsing JSPs only parse EL as EL if EL is 
enabled
         else strings such as ${ will be silently dropped. (markt)
       </fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to