From: markt <markt@13f79535-47bb-0310-9956-ffa450edef68>
Date: Tue, 19 Oct 2010 13:21:05 +0000 (+0000)
Subject: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49297
X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=20244985cfb863e04bae0f8c68f6a78de84141ba;p=tomcat7.0

Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49297
Enforce the rules in the JSP specification for parsing the attributes of custom and standard actions that require that the attribute names are unique within an element and that there is whitespace before the attribute name. The whitespace test can be disabled by setting the system property <code>org.apache.jasper.compiler.Parser.STRICT_WHITESPACE</code> to <code>false</code>

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1024229 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/java/org/apache/jasper/compiler/Parser.java b/java/org/apache/jasper/compiler/Parser.java
index fc30d333b..28c88dca1 100644
--- a/java/org/apache/jasper/compiler/Parser.java
+++ b/java/org/apache/jasper/compiler/Parser.java
@@ -28,6 +28,7 @@ import javax.servlet.jsp.tagext.TagLibraryInfo;
 
 import org.apache.jasper.JasperException;
 import org.apache.jasper.JspCompilationContext;
+import org.apache.jasper.util.UniqueAttributesImpl;
 import org.xml.sax.Attributes;
 import org.xml.sax.helpers.AttributesImpl;
 
@@ -74,6 +75,13 @@ class Parser implements TagConstants {
     private static final String JAVAX_BODY_CONTENT_TEMPLATE_TEXT =
         "JAVAX_BODY_CONTENT_TEMPLATE_TEXT";
 
+    /* System property that controls if the strict white space rules are
+     * applied.
+     */ 
+    private static final boolean STRICT_WHITESPACE = Boolean.valueOf(
+            System.getProperty(
+                    "org.apache.jasper.compiler.Parser.STRICT_WHITESPACE",
+                    "true")).booleanValue();
     /**
      * The constructor
      */
@@ -142,11 +150,23 @@ class Parser implements TagConstants {
      * Attributes ::= (S Attribute)* S?
      */
     Attributes parseAttributes() throws JasperException {
-        AttributesImpl attrs = new AttributesImpl();
+        UniqueAttributesImpl attrs = new UniqueAttributesImpl();
 
         reader.skipSpaces();
-        while (parseAttribute(attrs))
-            reader.skipSpaces();
+        int ws = 1;
+
+        try {
+            while (parseAttribute(attrs)) {
+                if (ws == 0 && STRICT_WHITESPACE) {
+                    err.jspError(reader.mark(),
+                            "jsp.error.attribute.nowhitespace");
+                }
+                ws = reader.skipSpaces();
+            }
+        } catch (IllegalArgumentException iae) {
+            // Duplicate attribute
+            err.jspError(reader.mark(), "jsp.error.attribute.duplicate");
+        }
 
         return attrs;
     }
diff --git a/java/org/apache/jasper/resources/LocalStrings.properties b/java/org/apache/jasper/resources/LocalStrings.properties
index eb0273bb4..11e07ec0a 100644
--- a/java/org/apache/jasper/resources/LocalStrings.properties
+++ b/java/org/apache/jasper/resources/LocalStrings.properties
@@ -347,6 +347,8 @@ jsp.error.attribute.noequal=equal symbol expected
 jsp.error.attribute.noquote=quote symbol expected
 jsp.error.attribute.unterminated=attribute for {0} is not properly terminated
 jsp.error.attribute.noescape=Attribute value {0} is quoted with {1} which must be escaped when used within the value
+jsp.error.attribute.nowhitespace=The JSP specification requires that an attribute name is preceded by whitespace
+jsp.error.attribute.duplicate=Attribute qualified names must be unique within an element
 jsp.error.missing.tagInfo=TagInfo object for {0} is missing from TLD
 jsp.error.deferredmethodsignaturewithoutdeferredmethod=Cannot specify a method signature if 'deferredMethod' is not 'true'
 jsp.error.deferredvaluetypewithoutdeferredvalue=Cannot specify a value type if 'deferredValue' is not 'true'
@@ -466,3 +468,6 @@ jsp.warning.noJarScanner=Warning: No org.apache.tomcat.JarScanner set in Servlet
 
 # JavacErrorDetail
 jsp.error.bug48498=Unable to display JSP extract. Probably due to an XML parser bug (see Tomcat bug 48498 for details).
+
+# UniqueAttributesImpl
+jsp.error.duplicateqname=An attribute with duplicate qualified name [{0}] was found. Attribute qualified names must be unique within an element.
\ No newline at end of file
diff --git a/java/org/apache/jasper/util/UniqueAttributesImpl.java b/java/org/apache/jasper/util/UniqueAttributesImpl.java
new file mode 100644
index 000000000..ce3809901
--- /dev/null
+++ b/java/org/apache/jasper/util/UniqueAttributesImpl.java
@@ -0,0 +1,87 @@
+/*
+ * 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
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package org.apache.jasper.util;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.jasper.compiler.Localizer;
+import org.xml.sax.Attributes;
+import org.xml.sax.helpers.AttributesImpl;
+
+/**
+ * Wraps the default attributes implementation and ensures that each attribute
+ * has a unique qname as required by the JSP specification.
+ */
+public class UniqueAttributesImpl extends AttributesImpl {
+
+    private Set<String> qNames = new HashSet<String>();
+
+    @Override
+    public void clear() {
+        qNames.clear();
+        super.clear();
+    }
+
+    @Override
+    public void setAttributes(Attributes atts) {
+        for (int i = 0; i < atts.getLength(); i++) {
+            if (!qNames.add(atts.getQName(i))) {
+                handleDuplicate(atts.getQName(i));
+            }
+        }
+        super.setAttributes(atts);
+    }
+
+    @Override
+    public void addAttribute(String uri, String localName, String qName,
+            String type, String value) {
+        if (qNames.add(qName)) {
+            super.addAttribute(uri, localName, qName, type, value);
+        } else {
+            handleDuplicate(qName);
+        }
+    }
+
+    @Override
+    public void setAttribute(int index, String uri, String localName,
+            String qName, String type, String value) {
+        qNames.remove(super.getQName(index));
+        if (qNames.add(qName)) {
+            super.setAttribute(index, uri, localName, qName, type, value);
+        } else {
+            handleDuplicate(qName);
+        }
+    }
+
+    @Override
+    public void removeAttribute(int index) {
+        qNames.remove(super.getQName(index));
+        super.removeAttribute(index);
+    }
+
+    @Override
+    public void setQName(int index, String qName) {
+        qNames.remove(super.getQName(index));
+        super.setQName(index, qName);
+    }
+
+    private void handleDuplicate(String qName) {
+        throw new IllegalArgumentException(
+                Localizer.getMessage("jsp.error.duplicateqname", qName));
+    }
+}
diff --git a/test/org/apache/jasper/compiler/TestParser.java b/test/org/apache/jasper/compiler/TestParser.java
index 4675fe2a7..145c2609f 100644
--- a/test/org/apache/jasper/compiler/TestParser.java
+++ b/test/org/apache/jasper/compiler/TestParser.java
@@ -18,11 +18,17 @@
 package org.apache.jasper.compiler;
 
 import java.io.File;
+import java.util.HashMap;
+import java.util.List;
 
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
 import org.apache.tomcat.util.buf.ByteChunk;
 
+/**
+ * Tests are duplicated in {@link TestParserNoStrictWhitespace} with the strict
+ * whitespace parsing disabled.
+ */
 public class TestParser extends TomcatBaseTest {
     
     public void testBug48627() throws Exception {
@@ -93,8 +99,7 @@ public class TestParser extends TomcatBaseTest {
     public void testBug48668b() throws Exception {
         Tomcat tomcat = getTomcatInstance();
 
-        File appDir = 
-            new File("test/webapp-3.0");
+        File appDir = new File("test/webapp-3.0");
         // app dir is relative to server home
         tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
         
@@ -107,6 +112,38 @@ public class TestParser extends TomcatBaseTest {
         assertEcho(result, "01-Hello world</p>#{foo2");
     }
 
+    public void testBug49297NoSpaceStrict() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp-3.0");
+        // app dir is relative to server home
+        tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
+        
+        tomcat.start();
+
+        int sc = getUrl("http://localhost:" + getPort() +
+                "/test/bug49297NoSpace.jsp", new ByteChunk(),
+                new HashMap<String,List<String>>());
+
+        assertEquals(500, sc);
+    }
+    
+    public void testBug49297DuplicateAttr() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp-3.0");
+        // app dir is relative to server home
+        tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
+        
+        tomcat.start();
+
+        int sc = getUrl("http://localhost:" + getPort() +
+                "/test/bug49297DuplicateAttr.jsp", new ByteChunk(),
+                new HashMap<String,List<String>>());
+
+        assertEquals(500, sc);
+    }
+    
     /** Assertion for text printed by tags:echo */
     private static void assertEcho(String result, String expected) {
         assertTrue(result.indexOf("<p>" + expected + "</p>") > 0);
diff --git a/test/org/apache/jasper/compiler/TestParserNoStrictWhitespace.java b/test/org/apache/jasper/compiler/TestParserNoStrictWhitespace.java
new file mode 100644
index 000000000..b1bba1f7b
--- /dev/null
+++ b/test/org/apache/jasper/compiler/TestParserNoStrictWhitespace.java
@@ -0,0 +1,163 @@
+/*
+ * 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
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+
+package org.apache.jasper.compiler;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.List;
+
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+/**
+ * Tests are duplicated in {@link TestParser} with the strict whitespace parsing
+ * enabled by default.
+ */
+public class TestParserNoStrictWhitespace extends TomcatBaseTest {
+    
+    @Override
+    public void setUp() throws Exception {
+        System.setProperty(
+                "org.apache.jasper.compiler.Parser.STRICT_WHITESPACE",
+                "false");
+        super.setUp();
+    }
+
+    public void testBug48627() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = 
+            new File("test/webapp-3.0");
+        // app dir is relative to server home
+        tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
+        
+        tomcat.start();
+
+        ByteChunk res = getUrl("http://localhost:" + getPort() +
+                "/test/bug48nnn/bug48627.jsp");
+        
+        String result = res.toString();
+        // Beware of the differences between escaping in JSP attributes and
+        // in Java Strings
+        assertEcho(result, "00-\\");
+        assertEcho(result, "01-\\");
+    }
+
+    public void testBug48668a() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = 
+            new File("test/webapp-3.0");
+        // app dir is relative to server home
+        tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
+        
+        tomcat.start();
+
+        ByteChunk res = getUrl("http://localhost:" + getPort() +
+                "/test/bug48nnn/bug48668a.jsp");
+        String result = res.toString();
+        assertEcho(result, "00-Hello world</p>#{foo.bar}");
+        assertEcho(result, "01-Hello world</p>${foo.bar}");
+        assertEcho(result, "10-Hello ${'foo.bar}");
+        assertEcho(result, "11-Hello ${'foo.bar}");
+        assertEcho(result, "12-Hello #{'foo.bar}");
+        assertEcho(result, "13-Hello #{'foo.bar}");
+        assertEcho(result, "14-Hello ${'foo}");
+        assertEcho(result, "15-Hello ${'foo}");
+        assertEcho(result, "16-Hello #{'foo}");
+        assertEcho(result, "17-Hello #{'foo}");
+        assertEcho(result, "18-Hello ${'foo.bar}");
+        assertEcho(result, "19-Hello ${'foo.bar}");
+        assertEcho(result, "20-Hello #{'foo.bar}");
+        assertEcho(result, "21-Hello #{'foo.bar}");
+        assertEcho(result, "30-Hello ${'foo}");
+        assertEcho(result, "31-Hello ${'foo}");
+        assertEcho(result, "32-Hello #{'foo}");
+        assertEcho(result, "33-Hello #{'foo}");
+        assertEcho(result, "34-Hello ${'foo}");
+        assertEcho(result, "35-Hello ${'foo}");
+        assertEcho(result, "36-Hello #{'foo}");
+        assertEcho(result, "37-Hello #{'foo}");
+        assertEcho(result, "40-Hello ${'foo}");
+        assertEcho(result, "41-Hello ${'foo}");
+        assertEcho(result, "42-Hello #{'foo}");
+        assertEcho(result, "43-Hello #{'foo}");
+        assertEcho(result, "50-Hello ${'foo}");
+        assertEcho(result, "51-Hello ${'foo}");
+        assertEcho(result, "52-Hello #{'foo}");
+        assertEcho(result, "53-Hello #{'foo}");
+    }
+
+    public void testBug48668b() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp-3.0");
+        // app dir is relative to server home
+        tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
+        
+        tomcat.start();
+
+        ByteChunk res = getUrl("http://localhost:" + getPort() +
+                "/test/bug48nnn/bug48668b.jsp");
+        String result = res.toString();
+        assertEcho(result, "00-Hello world</p>#{foo.bar}");
+        assertEcho(result, "01-Hello world</p>#{foo2");
+    }
+
+    public void testBug49297NoSpaceNotStrict() throws Exception {
+
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp-3.0");
+        // app dir is relative to server home
+        tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
+        
+        tomcat.start();
+
+        ByteChunk res = new ByteChunk();
+        int sc = getUrl("http://localhost:" + getPort() +
+                "/test/bug49297NoSpace.jsp", res,
+                new HashMap<String,List<String>>());
+
+
+        assertEquals(200, sc);
+        assertEcho(res.toString(), "Hello World");
+    }
+
+    public void testBug49297DuplicateAttr() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp-3.0");
+        // app dir is relative to server home
+        tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
+        
+        tomcat.start();
+
+        int sc = getUrl("http://localhost:" + getPort() +
+                "/test/bug49297DuplicateAttr.jsp", new ByteChunk(),
+                new HashMap<String,List<String>>());
+
+        assertEquals(500, sc);
+    }
+
+    /** Assertion for text printed by tags:echo */
+    private static void assertEcho(String result, String expected) {
+        assertTrue(result.indexOf("<p>" + expected + "</p>") > 0);
+    }
+}
diff --git a/test/webapp-3.0/WEB-INF/tags/echo.tag b/test/webapp-3.0/WEB-INF/tags/echo.tag
index fcadc9b3c..0f489c170 100644
--- a/test/webapp-3.0/WEB-INF/tags/echo.tag
+++ b/test/webapp-3.0/WEB-INF/tags/echo.tag
@@ -15,4 +15,5 @@
   limitations under the License.
 --%><%@ tag %><%@
 attribute name="echo" type="java.lang.String"%><%@
+attribute name="dummy" type="java.lang.String" required="false"%><%@
 tag body-content="empty" %><p>${echo}</p>
\ No newline at end of file
diff --git a/test/webapp-3.0/bug49297DuplicateAttr.jsp b/test/webapp-3.0/bug49297DuplicateAttr.jsp
new file mode 100644
index 000000000..9790c80e6
--- /dev/null
+++ b/test/webapp-3.0/bug49297DuplicateAttr.jsp
@@ -0,0 +1,23 @@
+<%--
+ 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
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  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.
+--%>
+<%@ taglib prefix="tags" tagdir="/WEB-INF/tags" %>
+<html>
+  <head><title>Bug 49297 duplicate attribute test case</title></head>
+  <body>
+    <tags:echo echo="Hello World" echo="error"/>
+  </body>
+</html>
\ No newline at end of file
diff --git a/test/webapp-3.0/bug49297NoSpace.jsp b/test/webapp-3.0/bug49297NoSpace.jsp
new file mode 100644
index 000000000..5a857c3fb
--- /dev/null
+++ b/test/webapp-3.0/bug49297NoSpace.jsp
@@ -0,0 +1,23 @@
+<%--
+ 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
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  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.
+--%>
+<%@ taglib prefix="tags" tagdir="/WEB-INF/tags" %>
+<html>
+  <head><title>Bug 49297 whitespace test case</title></head>
+  <body>
+    <tags:echo echo="Hello World"dummy="ignored"/>
+  </body>
+</html>
\ No newline at end of file
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 35f4e5654..4c5cf3019 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -49,6 +49,15 @@
   <subsection name="Jasper">
     <changelog>
       <fix>
+        <bug>49297</bug>: Enforce the rules in the JSP specification for parsing
+        the attributes of custom and standard actions that require that
+        the attribute names are unique within an element and that there is
+        whitespace before the attribute name. The whitespace test can be
+        disabled by setting the system property
+        <code>org.apache.jasper.compiler.Parser.STRICT_WHITESPACE</code> to
+        <code>false</code>. (markt)
+      </fix>
+      <fix>
         <bug>50105</bug>: When processing composite EL expressions use
         <code>Enum.name()</code> rather than <code>Enum.toString()</code> as
         required by the EL specification. (markt)
diff --git a/webapps/docs/config/systemprops.xml b/webapps/docs/config/systemprops.xml
index 5ce3c7ad2..33dffdd05 100644
--- a/webapps/docs/config/systemprops.xml
+++ b/webapps/docs/config/systemprops.xml
@@ -125,6 +125,13 @@
       <code>true</code> will be used.</p>
     </property>
 
+    <property name="org.apache.jasper.compiler. Parser.STRICT_WHITESPACE">
+      <p>If <code>false</code> the requirements for whitespace before an
+      attribute name will be relaxed so that the lack of whitespace will not
+      cause an error. If not specified, the specification compliant default of
+      <code>true</code> will be used.</p>
+    </property>
+
     <property name="org.apache.jasper.runtime. BodyContentImpl.LIMIT_BUFFER">
       <p>If <code>true</code>, any tag buffer that expands beyond
       <code>org.apache.jasper.Constants.DEFAULT_TAG_BUFFER_SIZE</code> will be