svn commit: r1539173 - in /tomcat/trunk: java/org/apache/jasper/compiler/ java/org/apache/jasper/runtime/ test/org/apache/jasper/compiler/ test/org/apache/jasper/runtime/ test/webapp/WEB-INF/tags/ test/webapp/bug5nnnn/

[markt]

Author: markt
Date: Tue Nov  5 22:46:35 2013
New Revision: 1539173

URL: http://svn.apache.org/r1539173
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55735
This fixes a regression caused by the fix to 
https://issues.apache.org/bugzilla/show_bug.cgi?id=55198
When processing JSP documents, attributes in UninterpretedTag nodes that 
contain EL should have the the non-EL components XML escaped and the output of 
the EL components should not be escaped.

Added:
    tomcat/trunk/test/org/apache/jasper/compiler/TesterValidator.java
      - copied, changed from r1538919, 
tomcat/trunk/test/org/apache/jasper/runtime/TesterPageContextImpl.java
Removed:
    tomcat/trunk/test/org/apache/jasper/runtime/TesterPageContextImpl.java
Modified:
    tomcat/trunk/java/org/apache/jasper/compiler/Validator.java
    tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java
    tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
    tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx
    tomcat/trunk/test/webapp/bug5nnnn/bug55198.jsp

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=1539173&r1=1539172&r2=1539173&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Validator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Validator.java Tue Nov  5 
22:46:35 2013
@@ -38,6 +38,7 @@ import javax.servlet.jsp.tagext.TagLibra
 import javax.servlet.jsp.tagext.ValidationMessage;
 
 import org.apache.jasper.JasperException;
+import org.apache.jasper.compiler.ELNode.Text;
 import org.apache.jasper.el.ELContextImpl;
 import org.xml.sax.Attributes;
 
@@ -1375,8 +1376,16 @@ class Validator {
 
                         validateFunctions(el, n);
 
-                        result = new Node.JspAttribute(tai, qName, uri,
-                                localName, value, false, el, dynamic);
+                        if (n.getRoot().isXmlSyntax()) {
+                            // The non-EL elements need to be XML escaped
+                            XmlEscapeNonELVisitor v = new 
XmlEscapeNonELVisitor();
+                            el.visit(v);
+                            result = new Node.JspAttribute(tai, qName, uri,
+                                    localName, v.getText(), false, el, 
dynamic);
+                        } else {
+                            result = new Node.JspAttribute(tai, qName, uri,
+                                    localName, value, false, el, dynamic);
+                        }
 
                         ELContextImpl ctx =
                                 new ELContextImpl(expressionFactory);
@@ -1412,6 +1421,16 @@ class Validator {
             return result;
         }
 
+
+        private static class XmlEscapeNonELVisitor extends 
ELParser.TextBuilder {
+
+            @Override
+            public void visit(Text n) throws JasperException {
+                output.append(xmlEscape(n.getText()));
+            }
+        }
+
+
         /*
          * Return an empty StringBuilder [not thread-safe]
          */
@@ -1860,4 +1879,67 @@ class Validator {
             errDisp.jspError(errMsg.toString());
         }
     }
+
+    protected static String xmlEscape(String s) {
+        if (s == null) {
+            return null;
+        }
+        int len = s.length();
+
+        /*
+         * Look for any "bad" characters, Escape "bad" character was found
+         */
+        // ASCII " 34 & 38 ' 39 < 60 > 62
+        for (int i = 0; i < len; i++) {
+            char c = s.charAt(i);
+            if (c >= '\"' && c <= '>' &&
+                    (c == '<' || c == '>' || c == '\'' || c == '&' || c == 
'"')) {
+                // need to escape them and then quote the whole string
+                StringBuilder sb = new StringBuilder((int) (len * 1.2));
+                sb.append(s, 0, i);
+                int pos = i + 1;
+                for (int j = i; j < len; j++) {
+                    c = s.charAt(j);
+                    if (c >= '\"' && c <= '>') {
+                        if (c == '<') {
+                            if (j > pos) {
+                                sb.append(s, pos, j);
+                            }
+                            sb.append("&lt;");
+                            pos = j + 1;
+                        } else if (c == '>') {
+                            if (j > pos) {
+                                sb.append(s, pos, j);
+                            }
+                            sb.append("&gt;");
+                            pos = j + 1;
+                        } else if (c == '\'') {
+                            if (j > pos) {
+                                sb.append(s, pos, j);
+                            }
+                            sb.append("&#039;"); // &apos;
+                            pos = j + 1;
+                        } else if (c == '&') {
+                            if (j > pos) {
+                                sb.append(s, pos, j);
+                            }
+                            sb.append("&amp;");
+                            pos = j + 1;
+                        } else if (c == '"') {
+                            if (j > pos) {
+                                sb.append(s, pos, j);
+                            }
+                            sb.append("&#034;"); // &quot;
+                            pos = j + 1;
+                        }
+                    }
+                }
+                if (pos < len) {
+                    sb.append(s, pos, len);
+                }
+                return sb.toString();
+            }
+        }
+        return s;
+    }
 }

Modified: tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java?rev=1539173&r1=1539172&r2=1539173&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java (original)
+++ tomcat/trunk/java/org/apache/jasper/runtime/PageContextImpl.java Tue Nov  5 
22:46:35 2013
@@ -902,69 +902,6 @@ public class PageContextImpl extends Pag
         }
     }
 
-    protected static String XmlEscape(String s) {
-        if (s == null) {
-            return null;
-        }
-        int len = s.length();
-
-        /*
-         * Look for any "bad" characters, Escape "bad" character was found
-         */
-        // ASCII " 34 & 38 ' 39 < 60 > 62
-        for (int i = 0; i < len; i++) {
-            char c = s.charAt(i);
-            if (c >= '\"' && c <= '>' &&
-                    (c == '<' || c == '>' || c == '\'' || c == '&' || c == 
'"')) {
-                // need to escape them and then quote the whole string
-                StringBuilder sb = new StringBuilder((int) (len * 1.2));
-                sb.append(s, 0, i);
-                int pos = i + 1;
-                for (int j = i; j < len; j++) {
-                    c = s.charAt(j);
-                    if (c >= '\"' && c <= '>') {
-                        if (c == '<') {
-                            if (j > pos) {
-                                sb.append(s, pos, j);
-                            }
-                            sb.append("&lt;");
-                            pos = j + 1;
-                        } else if (c == '>') {
-                            if (j > pos) {
-                                sb.append(s, pos, j);
-                            }
-                            sb.append("&gt;");
-                            pos = j + 1;
-                        } else if (c == '\'') {
-                            if (j > pos) {
-                                sb.append(s, pos, j);
-                            }
-                            sb.append("&#039;"); // &apos;
-                            pos = j + 1;
-                        } else if (c == '&') {
-                            if (j > pos) {
-                                sb.append(s, pos, j);
-                            }
-                            sb.append("&amp;");
-                            pos = j + 1;
-                        } else if (c == '"') {
-                            if (j > pos) {
-                                sb.append(s, pos, j);
-                            }
-                            sb.append("&#034;"); // &quot;
-                            pos = j + 1;
-                        }
-                    }
-                }
-                if (pos < len) {
-                    sb.append(s, pos, len);
-                }
-                return sb.toString();
-            }
-        }
-        return s;
-    }
-
     /**
      * Proprietary method to evaluate EL expressions. XXX - This method should
      * go away once the EL interpreter moves out of JSTL and into its own
@@ -1014,9 +951,6 @@ public class PageContextImpl extends Pag
             ValueExpression ve = exprFactory.createValueExpression(ctx, 
expression, expectedType);
             retValue = ve.getValue(ctx);
         }
-        if (escape && retValue != null) {
-            retValue = XmlEscape(retValue.toString());
-        }
 
         return retValue;
     }

Modified: tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java?rev=1539173&r1=1539172&r2=1539173&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java (original)
+++ tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java Tue Nov  5 
22:46:35 2013
@@ -328,10 +328,16 @@ public class TestParser extends TomcatBa
 
         String result = res.toString();
 
-        Assert.assertTrue(result.contains("&quot;bar&quot;") ||
-                result.contains("&#034;bar&#034;"));
-        Assert.assertTrue(result.contains("&quot;foo&quot;") ||
-                result.contains("&#034;foo&#034;"));
+        Assert.assertTrue(result.contains("&quot;1foo1&quot;") ||
+                result.contains("&#034;1foo1&#034;"));
+        Assert.assertTrue(result.contains("&quot;2bar2&quot;") ||
+                result.contains("&#034;2bar2&#034;"));
+        Assert.assertTrue(result.contains("&quot;3a&amp;b3&quot;") ||
+                result.contains("&#034;3a&amp;b3&#034;"));
+        Assert.assertTrue(result.contains("&quot;4&4&quot;") ||
+                result.contains("&#034;4&4&#034;"));
+        Assert.assertTrue(result.contains("&quot;5&apos;5&quot;") ||
+                result.contains("&#034;5&apos;5&#034;"));
     }
 
     /** Assertion for text printed by tags:echo */

Copied: tomcat/trunk/test/org/apache/jasper/compiler/TesterValidator.java (from 
r1538919, 
tomcat/trunk/test/org/apache/jasper/runtime/TesterPageContextImpl.java)
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/jasper/compiler/TesterValidator.java?p2=tomcat/trunk/test/org/apache/jasper/compiler/TesterValidator.java&p1=tomcat/trunk/test/org/apache/jasper/runtime/TesterPageContextImpl.java&r1=1538919&r2=1539173&rev=1539173&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/jasper/runtime/TesterPageContextImpl.java 
(original)
+++ tomcat/trunk/test/org/apache/jasper/compiler/TesterValidator.java Tue Nov  
5 22:46:35 2013
@@ -14,15 +14,15 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.jasper.runtime;
+package org.apache.jasper.compiler;
 
 import org.junit.Assert;
 import org.junit.Test;
 
 /**
- * Performance tests for {@link PageContextImpl}.
+ * Performance tests for {@link Validator}.
  */
-public class TesterPageContextImpl {
+public class TesterValidator {
 
     private static String[] bug53867TestData = new String[] {
             "Hello World!",
@@ -42,7 +42,7 @@ public class TesterPageContextImpl {
 
         for (int j = 0; j < bug53867TestData.length; j++) {
             Assert.assertEquals(doTestBug53867OldVersion(bug53867TestData[j]),
-                    PageContextImpl.XmlEscape(bug53867TestData[j]));
+                    Validator.xmlEscape(bug53867TestData[j]));
         }
 
         for (int i = 0; i < 100; i++) {
@@ -52,7 +52,7 @@ public class TesterPageContextImpl {
         }
         for (int i = 0; i < 100; i++) {
             for (int j = 0; j < bug53867TestData.length; j++) {
-                PageContextImpl.XmlEscape(bug53867TestData[j]);
+                Validator.xmlEscape(bug53867TestData[j]);
             }
         }
 
@@ -68,7 +68,7 @@ public class TesterPageContextImpl {
         start = System.currentTimeMillis();
         for (int i = 0; i < count; i++) {
             for (int j = 0; j < bug53867TestData.length; j++) {
-                PageContextImpl.XmlEscape(bug53867TestData[j]);
+                Validator.xmlEscape(bug53867TestData[j]);
             }
         }
         System.out.println(

Modified: tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx?rev=1539173&r1=1539172&r2=1539173&view=diff
==============================================================================
--- tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx (original)
+++ tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx Tue Nov  5 22:46:35 2013
@@ -17,7 +17,10 @@
 -->
 <jsp:root xmlns:jsp="http://java.sun.com/JSP/Page"; version="2.0">
 <jsp:directive.tag body-content="scriptless" />
-<a href="#" onclick="window.alert(&quot;${'foo'}&quot;)">foo</a>
-<a href="#" onclick="window.alert(&quot;bar&quot;)">bar</a>
+<a href="#" onclick="window.alert(&quot;1${'foo'}1&quot;)">foo</a>
+<a href="#" onclick="window.alert(&quot;2bar2&quot;)">bar</a>
+<a href="#" onclick="window.alert(&quot;3${text}3&quot;)">foo</a>
+<a href="#" onclick="window.alert(&quot;4${'&amp;'}4&quot;)">foo</a>
+<a href="#" onclick="window.alert(&quot;5${'&amp;apos;'}5&quot;)">foo</a>
 <jsp:doBody />
 </jsp:root>

Modified: tomcat/trunk/test/webapp/bug5nnnn/bug55198.jsp
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/webapp/bug5nnnn/bug55198.jsp?rev=1539173&r1=1539172&r2=1539173&view=diff
==============================================================================
--- tomcat/trunk/test/webapp/bug5nnnn/bug55198.jsp (original)
+++ tomcat/trunk/test/webapp/bug5nnnn/bug55198.jsp Tue Nov  5 22:46:35 2013
@@ -15,6 +15,9 @@
   limitations under the License.
 --%>
 <%@ taglib prefix="tags" tagdir="/WEB-INF/tags" %>
+<%
+request.setAttribute("text", "a&amp;b");
+%>
 <html>
   <head><title>Bug 55198 test case</title></head>
   <body>



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