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/23 10:34:30 UTC

svn commit: r1580478 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/jasper/compiler/Generator.java java/org/apache/jasper/compiler/Validator.java webapps/docs/changelog.xml

Author: markt
Date: Sun Mar 23 09:34:30 2014
New Revision: 1580478

URL: http://svn.apache.org/r1580478
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
Don't escape values for dynamic tag attributes

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
    tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.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=1580478&r1=1580477&r2=1580478&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sun Mar 23 09:34:30 2014
@@ -64,14 +64,6 @@ PATCHES PROPOSED TO BACKPORT:
          Thus I am OK with this change.
   -1:
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56265
-  Don't escape values for dynamic tag attributes
-  http://people.apache.org/~markt/patches/2014-03-19-bug56265-tc6-v1.patch
-  +1: markt, kkolinko, remm
-  -1:
-    kkolinko: Beware that changelog part of this patch merges into a wrong
-    place. This change belongs to Jasper, not Catalina.
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56275
   Fix memory leak if a Filter throws an exception during its destroy() method
   http://people.apache.org/~markt/patches/2014-03-19-bug56275-tc6-v1.patch

Modified: tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java?rev=1580478&r1=1580477&r2=1580478&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java Sun Mar 23 09:34:30 2014
@@ -1838,7 +1838,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/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.java?rev=1580478&r1=1580477&r2=1580478&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Validator.java Sun Mar 23 09:34:30 2014
@@ -1331,34 +1331,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();
                         ctx.setFunctionMapper(getFunctionMapper(el));
 
@@ -1370,10 +1382,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/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1580478&r1=1580477&r2=1580478&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Sun Mar 23 09:34:30 2014
@@ -103,6 +103,10 @@
         <code>JspWriter.DEFAULT_BUFFER</code>. Based on a patch by Eugene Chung.
         (markt)
       </fix>
+      <fix>
+        <bug>56265</bug>: Do not escape values of dynamic tag attributes
+        containing EL expressions. (kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Web applications">
@@ -236,7 +240,7 @@
         aware, validating parser will be used when processing <code>*.tld</code>
         and <code>web.xml</code> files if the system property
         <code>org.apache.catalina.STRICT_SERVLET_COMPLIANCE</code> is set to
-        <code>true</code>. (markt) 
+        <code>true</code>. (markt)
       </fix>
       <fix>
         Fix CVE-2014-0033:



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