You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/03/19 13:41:35 UTC

svn commit: r1579214 - in /tomcat/trunk: java/org/apache/jasper/compiler/ test/org/apache/jasper/compiler/ test/webapp/WEB-INF/tags/ test/webapp/bug5nnnn/ webapps/docs/

Author: markt
Date: Wed Mar 19 12:41:34 2014
New Revision: 1579214

URL: http://svn.apache.org/r1579214
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
Do not escape values of dynamic tag attributes containing EL expressions
Patch by kkolinko

Added:
    tomcat/trunk/test/webapp/WEB-INF/tags/bug56265.tagx
      - copied, changed from r1579174, tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx
    tomcat/trunk/test/webapp/bug5nnnn/bug56265.jsp
      - copied, changed from r1579174, tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx
Modified:
    tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
    tomcat/trunk/java/org/apache/jasper/compiler/Validator.java
    tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
    tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx
    tomcat/trunk/webapps/docs/changelog.xml

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=1579214&r1=1579213&r2=1579214&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Wed Mar 19 12:41:34 2014
@@ -1839,7 +1839,7 @@ class Generator {
                         out.print(" + \"\\\"");
                     } else {
                         out.print(DOUBLE_QUOTE);
-                        out.print(attrs.getValue(i).replace("\"", """));
+                        out.print(jspAttrs[i].getValue().replace("\"", """));
                         out.print(DOUBLE_QUOTE);
                     }
                 }

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=1579214&r1=1579213&r2=1579214&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Validator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Validator.java Wed Mar 19 12:41:34 2014
@@ -1359,34 +1359,46 @@ class Validator {
                     result = new Node.JspAttribute(tai, qName, uri, localName,
                             value.substring(3, value.length() - 2), true, null,
                             dynamic);
-                } else if (pageInfo.isELIgnored()) {
-                    result = new Node.JspAttribute(tai, qName, uri, localName,
-                            value, false, null, dynamic);
                 } else {
-                    // The attribute can contain expressions but is not a
-                    // scriptlet expression; thus, we want to run it through
-                    // the expression interpreter
-
-                    // validate expression syntax if string contains
-                    // expression(s)
-                    ELNode.Nodes el = ELParser.parse(value, pageInfo
-                            .isDeferredSyntaxAllowedAsLiteral());
-
-                    if (el.containsEL()) {
+                    ELNode.Nodes el = null;
+                    if (!pageInfo.isELIgnored()) {
+                        // The attribute can contain expressions but is not a
+                        // scriptlet expression; thus, we want to run it through
+                        // the expression interpreter
+
+                        // validate expression syntax if string contains
+                        // expression(s)
+                        el = ELParser.parse(value,
+                                pageInfo.isDeferredSyntaxAllowedAsLiteral());
 
-                        validateFunctions(el, n);
+                        if (el.containsEL()) {
+                            validateFunctions(el, n);
+                        } else {
+                            el = null;
+                        }
+                    }
 
-                        if (n.getRoot().isXmlSyntax()) {
-                            // The non-EL elements need to be XML escaped
+                    if (n instanceof Node.UninterpretedTag &&
+                            n.getRoot().isXmlSyntax()) {
+                        // Attribute values of uninterpreted tags will have been
+                        // XML un-escaped during parsing. Since these attributes
+                        // are part of an uninterpreted tag the value needs to
+                        // be re-escaped before being included in the output.
+                        // The wrinkle is that the output of any EL must not be
+                        // re-escaped as that must be output as is.
+                        if (el != null) {
                             XmlEscapeNonELVisitor v = new XmlEscapeNonELVisitor();
                             el.visit(v);
-                            result = new Node.JspAttribute(tai, qName, uri,
-                                    localName, v.getText(), false, el, dynamic);
+                            value = v.getText();
                         } else {
-                            result = new Node.JspAttribute(tai, qName, uri,
-                                    localName, value, false, el, dynamic);
+                            value = xmlEscape(value);
                         }
+                    }
 
+                    result = new Node.JspAttribute(tai, qName, uri, localName,
+                            value, false, el, dynamic);
+
+                    if (el != null) {
                         ELContextImpl ctx =
                                 new ELContextImpl(expressionFactory);
                         ctx.setFunctionMapper(getFunctionMapper(el));
@@ -1399,10 +1411,6 @@ class Validator {
                                     "jsp.error.invalid.expression", value, e
                                             .toString());
                         }
-
-                    } else {
-                        result = new Node.JspAttribute(tai, qName, uri,
-                                localName, value, false, null, dynamic);
                     }
                 }
             } else {

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=1579214&r1=1579213&r2=1579214&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java (original)
+++ tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java Wed Mar 19 12:41:34 2014
@@ -14,7 +14,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.jasper.compiler;
 
 import java.io.File;
@@ -27,8 +26,11 @@ import static org.junit.Assert.assertTru
 import org.junit.Assert;
 import org.junit.Test;
 
+import org.apache.catalina.WebResourceRoot;
+import org.apache.catalina.core.StandardContext;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.catalina.webresources.StandardRoot;
 import org.apache.tomcat.util.buf.ByteChunk;
 
 /**
@@ -328,16 +330,54 @@ public class TestParser extends TomcatBa
 
         String result = res.toString();
 
-        Assert.assertTrue(result.contains(""1foo1"") ||
-                result.contains(""1foo1""));
-        Assert.assertTrue(result.contains(""2bar2"") ||
-                result.contains(""2bar2""));
-        Assert.assertTrue(result.contains(""3a&b3"") ||
-                result.contains(""3a&b3""));
-        Assert.assertTrue(result.contains(""4&4"") ||
-                result.contains(""4&4""));
-        Assert.assertTrue(result.contains(""5'5"") ||
-                result.contains(""5'5""));
+        Assert.assertTrue(result,
+                result.contains(""1foo1<&>"")
+             || result.contains(""1foo1<&>""));
+        Assert.assertTrue(result,
+                result.contains(""2bar2<&>"")
+             || result.contains(""2bar2<&>""));
+        Assert.assertTrue(result,
+                result.contains(""3a&b3"")
+             || result.contains(""3a&b3""));
+        Assert.assertTrue(result,
+                result.contains(""4&4"")
+             || result.contains(""4&4""));
+        Assert.assertTrue(result,
+                result.contains(""5'5"")
+             || result.contains(""5'5""));
+    }
+
+    @Test
+    public void testBug56265() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp");
+        // app dir is relative to server home
+        StandardContext ctxt = (StandardContext) tomcat.addWebapp(null,
+                "/test", appDir.getAbsolutePath());
+
+        // This test needs the JSTL libraries
+        File lib = new File("webapps/examples/WEB-INF/lib");
+        ctxt.setResources(new StandardRoot(ctxt));
+        ctxt.getResources().createWebResourceSet(
+                WebResourceRoot.ResourceSetType.POST, "/WEB-INF/lib",
+                lib.getAbsolutePath(), null, "/");
+
+        tomcat.start();
+
+        ByteChunk res = getUrl("http://localhost:" + getPort() +
+                "/test/bug5nnnn/bug56265.jsp");
+
+        String result = res.toString();
+
+        Assert.assertTrue(result,
+                result.contains("[1: [data-test]: [window.alert('Hello World <&>!')]]"));
+        Assert.assertTrue(result,
+                result.contains("[2: [data-test]: [window.alert('Hello World <&>!')]]"));
+        Assert.assertTrue(result,
+                result.contains("[3: [data-test]: [window.alert('Hello 'World <&>'!')]]"));
+        Assert.assertTrue(result,
+                result.contains("[4: [data-test]: [window.alert('Hello 'World <&>'!')]]"));
     }
 
     /** Assertion for text printed by tags:echo */

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=1579214&r1=1579213&r2=1579214&view=diff
==============================================================================
--- tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx (original)
+++ tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx Wed Mar 19 12:41:34 2014
@@ -17,8 +17,8 @@
 -->
 <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;1${'foo'}1&quot;)">foo</a>
-<a href="#" onclick="window.alert(&quot;2bar2&quot;)">bar</a>
+<a href="#" onclick="window.alert(&quot;1${'foo'}1&lt;&amp;&gt;&quot;)">foo</a>
+<a href="#" onclick="window.alert(&quot;2bar2&lt;&amp;&gt;&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>

Copied: tomcat/trunk/test/webapp/WEB-INF/tags/bug56265.tagx (from r1579174, tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx)
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/webapp/WEB-INF/tags/bug56265.tagx?p2=tomcat/trunk/test/webapp/WEB-INF/tags/bug56265.tagx&p1=tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx&r1=1579174&r2=1579214&rev=1579214&view=diff
==============================================================================
--- tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx (original)
+++ tomcat/trunk/test/webapp/WEB-INF/tags/bug56265.tagx Wed Mar 19 12:41:34 2014
@@ -1,6 +1,6 @@
-<?xml version="1.0" encoding="ISO-8859-1"?>
+<?xml version="1.0" encoding="UTF-8" ?>
 <!--
- Licensed to the Apache Software Foundation (ASF) under one or more
+  Licensed to the Apache Software Foundation (ASF) under one or more
   contributor license agreements.  See the NOTICE file distributed with
   this work for additional information regarding copyright ownership.
   The ASF licenses this file to You under the Apache License, Version 2.0
@@ -15,12 +15,10 @@
   See the License for the specific language governing permissions and
   limitations under the License.
 -->
-<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;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>
+<jsp:root xmlns:jsp="http://java.sun.com/JSP/Page" version="2.0"
+ xmlns:c="http://java.sun.com/jsp/jstl/core">
+  <jsp:directive.tag body-content="empty" dynamic-attributes="attMap" />
+  <c:forEach var="e" items="${attMap}">
+    <jsp:text>[${e.key}]: [${e.value}]</jsp:text>
+  </c:forEach>
+</jsp:root>
\ No newline at end of file

Copied: tomcat/trunk/test/webapp/bug5nnnn/bug56265.jsp (from r1579174, tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx)
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/webapp/bug5nnnn/bug56265.jsp?p2=tomcat/trunk/test/webapp/bug5nnnn/bug56265.jsp&p1=tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx&r1=1579174&r2=1579214&rev=1579214&view=diff
==============================================================================
--- tomcat/trunk/test/webapp/WEB-INF/tags/bug55198.tagx (original)
+++ tomcat/trunk/test/webapp/bug5nnnn/bug56265.jsp Wed Mar 19 12:41:34 2014
@@ -1,5 +1,4 @@
-<?xml version="1.0" encoding="ISO-8859-1"?>
-<!--
+<%--
  Licensed to the Apache Software Foundation (ASF) under one or more
   contributor license agreements.  See the NOTICE file distributed with
   this work for additional information regarding copyright ownership.
@@ -14,13 +13,18 @@
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
--->
-<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;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>
+--%>
+<%@ taglib prefix="tags" tagdir="/WEB-INF/tags" %>
+<%
+request.setAttribute("text", "World <&>");
+request.setAttribute("textQuote", "'World <&>'");
+%>
+<html>
+  <head><title>Bug 56265 test case</title></head>
+  <body>
+    <p>[1: <tags:bug56265 data-test="window.alert('Hello World <&>!')"/>]</p>
+    <p>[2: <tags:bug56265 data-test="window.alert('Hello ${text}!')"/>]</p>
+    <p>[3: <tags:bug56265 data-test="window.alert('Hello &apos;World <&>&apos;!')"/>]</p>
+    <p>[4: <tags:bug56265 data-test="window.alert('Hello ${textQuote}!')"/>]</p>
+  </body>
+</html>
\ No newline at end of file

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1579214&r1=1579213&r2=1579214&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Mar 19 12:41:34 2014
@@ -206,6 +206,10 @@
         Update to the Eclipse JDT Compiler P20140317-1600 which adds support for
         Java 8 syntax to JSPs. (markt)
       </update>
+      <fix>
+        <bug>56265</bug>: Do not escape values of dynamic tag attributes
+        containing EL expressions. (kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="WebSocket">



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