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 2010/07/10 23:13:23 UTC

svn commit: r962917 - in /tomcat/trunk: java/org/apache/catalina/filters/ test/org/apache/catalina/filters/ webapps/docs/ webapps/docs/config/

Author: markt
Date: Sat Jul 10 21:13:23 2010
New Revision: 962917

URL: http://svn.apache.org/viewvc?rev=962917&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49478
Add support for user specified character sets to the AddDefaultCharsetFilter. Based on a patch by Felix Schumacher.

Added:
    tomcat/trunk/test/org/apache/catalina/filters/TestAddCharSetFilter.java   (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/filters/AddDefaultCharsetFilter.java
    tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties
    tomcat/trunk/webapps/docs/changelog.xml
    tomcat/trunk/webapps/docs/config/filter.xml

Modified: tomcat/trunk/java/org/apache/catalina/filters/AddDefaultCharsetFilter.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/AddDefaultCharsetFilter.java?rev=962917&r1=962916&r2=962917&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/AddDefaultCharsetFilter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/filters/AddDefaultCharsetFilter.java Sat Jul 10 21:13:23 2010
@@ -17,10 +17,9 @@
 
 package org.apache.catalina.filters;
 
-
 import java.io.IOException;
+import java.nio.charset.Charset;
 
-import javax.servlet.Filter;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
@@ -29,24 +28,54 @@ import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpServletResponseWrapper;
 
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+
 
 /**
  * Filter that explicitly sets the default character set for media subtypes of
- * the "text" type to ISO-8859-1. RFC2616 explicitly states that browsers must
- * use ISO-8859-1 in these circumstances. However, browsers may attempt to
+ * the "text" type to ISO-8859-1, or another user defined character set. RFC2616
+ * explicitly states that browsers must use ISO-8859-1 if no character set is
+ * defined for media with subtype "text". However, browsers may attempt to
  * auto-detect the character set. This may be exploited by an attacker to
  * perform an XSS attack. Internet Explorer has this behaviour by default. Other
- * browsers have an option to enable it.
+ * browsers have an option to enable it.<br/>
  * 
  * This filter prevents the attack by explicitly setting a character set. Unless
  * the provided character set is explicitly overridden by the user - in which
  * case they deserve everything they get - the browser will adhere to an
  * explicitly set character set, thus preventing the XSS attack.
  */
-public class AddDefaultCharsetFilter implements Filter {
+public class AddDefaultCharsetFilter extends FilterBase {
+
+    private static final Log log =
+        LogFactory.getLog(AddDefaultCharsetFilter.class);
+
+    private static final String DEFAULT_ENCODING = "ISO-8859-1";
+    
+    private String encoding;
+
+    public void setEncoding(String encoding) {
+        this.encoding = encoding;
+    }
+
+    @Override
+    protected Log getLogger() {
+        return log;
+    }
 
-    public void destroy() {
-        // NOOP
+    @Override
+    public void init(FilterConfig filterConfig) throws ServletException {
+        super.init(filterConfig);
+        if (encoding == null || encoding.length() == 0 ||
+                encoding.equalsIgnoreCase("default")) {
+            encoding = DEFAULT_ENCODING;
+        } else if (encoding.equalsIgnoreCase("system")) {
+            encoding = Charset.defaultCharset().name();
+        } else if (!Charset.isSupported(encoding)) {
+            throw new IllegalArgumentException(sm.getString(
+                    "addDefaultCharset.unsupportedCharset", encoding));
+        }
     }
 
     public void doFilter(ServletRequest request, ServletResponse response,
@@ -55,40 +84,46 @@ public class AddDefaultCharsetFilter imp
         // Wrap the response
         if (response instanceof HttpServletResponse) {
             ResponseWrapper wrapped =
-                new ResponseWrapper((HttpServletResponse)response);
+                new ResponseWrapper((HttpServletResponse)response, encoding);
             chain.doFilter(request, wrapped);
         } else {
             chain.doFilter(request, response);
         }
     }
 
-    public void init(FilterConfig filterConfig) throws ServletException {
-        // NOOP
-    }
-
     /**
-     * Wrapper that adds the default character set for text media types if no
-     * character set is specified.
+     * Wrapper that adds a character set for text media types if no character
+     * set is specified.
      */
-    public class ResponseWrapper extends HttpServletResponseWrapper {
+    public static class ResponseWrapper extends HttpServletResponseWrapper {
+
+        private String encoding;
+        
+        public ResponseWrapper(HttpServletResponse response, String encoding) {
+            super(response);
+            this.encoding = encoding;
+        }
 
         @Override
         public void setContentType(String ct) {
             
-            if (ct != null && ct.startsWith("text/") &&
-                    ct.indexOf("charset=") < 0) {
-                // Use getCharacterEncoding() in case the charset has already
-                // been set by a separate call.
-                super.setContentType(ct + ";charset=" + getCharacterEncoding());
+            if (ct != null && ct.startsWith("text/")) {
+                if (ct.indexOf("charset=") < 0) {
+                    super.setContentType(ct + ";charset=" + encoding);
+                } else {
+                    super.setContentType(ct);
+                    encoding = getCharacterEncoding();
+                }
             } else {
                 super.setContentType(ct);
             }
 
         }
 
-        public ResponseWrapper(HttpServletResponse response) {
-            super(response);
+        @Override
+        public void setCharacterEncoding(String charset) {
+            super.setCharacterEncoding(charset);
+            encoding = charset;
         }
-        
     }
 }

Modified: tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties?rev=962917&r1=962916&r2=962917&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties Sat Jul 10 21:13:23 2010
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+addDefaultCharset.unsupportedCharset=Specified character set [{0}] is not supported
 csrfPrevention.invalidRandomClass=Unable to create Random source using class [{0}]
 filterbase.noSuchProperty=The property "{0}" is not defined for filters of type "{1}"
 

Added: tomcat/trunk/test/org/apache/catalina/filters/TestAddCharSetFilter.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/filters/TestAddCharSetFilter.java?rev=962917&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/filters/TestAddCharSetFilter.java (added)
+++ tomcat/trunk/test/org/apache/catalina/filters/TestAddCharSetFilter.java Sat Jul 10 21:13:23 2010
@@ -0,0 +1,147 @@
+/*
+ *  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.catalina.filters;
+
+import java.io.IOException;
+import java.nio.charset.Charset;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.deploy.FilterDef;
+import org.apache.catalina.deploy.FilterMap;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+public class TestAddCharSetFilter extends TomcatBaseTest {
+
+    public void testNoneSpecifiedMode1() throws Exception {
+        doTest(null, "ISO-8859-1");
+    }
+
+    public void testNoneSpecifiedMode2() throws Exception {
+        doTest(null, "ISO-8859-2", 2);
+    }
+
+    public void testNoneSpecifiedMode3() throws Exception {
+        doTest(null, "ISO-8859-3", 3);
+    }
+
+    public void testDefault() throws Exception {
+        doTest("default", "ISO-8859-1");
+    }
+
+    public void testDefaultMixedCase() throws Exception {
+        doTest("dEfAuLt", "ISO-8859-1");
+    }
+
+    public void testSystem() throws Exception {
+        doTest("system", Charset.defaultCharset().name());
+    }
+
+    public void testSystemMixedCase() throws Exception {
+        doTest("SyStEm", Charset.defaultCharset().name());
+    }
+
+    public void testUTF8() throws Exception {
+        doTest("utf-8", "utf-8");
+    }
+
+
+    private void doTest(String encoding, String expected) throws Exception {
+        doTest(encoding, expected, 1);
+    }
+    
+    private void doTest(String encoding, String expected, int mode)
+            throws Exception {
+        // Setup Tomcat instance
+        Tomcat tomcat = getTomcatInstance();
+        
+        // Must have a real docBase - just use temp
+        Context ctx = 
+            tomcat.addContext("/", System.getProperty("java.io.tmpdir"));
+
+        // Add the Servlet
+        CharsetServlet servlet = new CharsetServlet(mode);
+        Tomcat.addServlet(ctx, "servlet", servlet);
+        ctx.addServletMapping("/", "servlet");
+        
+        // Add the Filter
+        FilterDef filterDef = new FilterDef();
+        filterDef.setFilterClass(AddDefaultCharsetFilter.class.getName());
+        filterDef.setFilterName("filter");
+        if (encoding != null) {
+            filterDef.addInitParameter("encoding", encoding);
+        }
+        ctx.addFilterDef(filterDef);
+        FilterMap filterMap = new FilterMap();
+        filterMap.setFilterName("filter");
+        filterMap.addServletName("servlet");
+        ctx.addFilterMap(filterMap);
+        
+        tomcat.start();
+
+        Map<String, List<String>> headers = new HashMap<String, List<String>>();
+        getUrl("http://localhost:" + getPort() + "/", new ByteChunk(), headers);
+        
+        List<String> ctHeaders = headers.get("Content-Type");
+        assertEquals(1, ctHeaders.size());
+        String ct = ctHeaders.get(0);
+        assertEquals("text/plain;charset=" + expected, ct);
+    }
+
+    private static class CharsetServlet extends HttpServlet {
+        private static final long serialVersionUID = 1L;
+        private static final String OUTPUT = "OK";
+        
+        private final int mode;
+        
+        public CharsetServlet(int mode) {
+            this.mode = mode;
+        }
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            
+            switch (mode) {
+                case 1:
+                    resp.setContentType("text/plain");
+                    break;
+                case 2:
+                    resp.setContentType("text/plain;charset=ISO-8859-2"); 
+                    break;
+                case 3:
+                    resp.setContentType("text/plain");
+                    resp.setCharacterEncoding("ISO-8859-3");
+                    break;
+                default:
+                    resp.setContentType("text/plain;charset=ISO-8859-4");
+                    break;
+            }
+            resp.getWriter().print(OUTPUT);
+        }
+    }
+}

Propchange: tomcat/trunk/test/org/apache/catalina/filters/TestAddCharSetFilter.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=962917&r1=962916&r2=962917&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Sat Jul 10 21:13:23 2010
@@ -83,6 +83,11 @@
         Use a LockOutRealm in the default configuration to prevent attempts to
         guess user passwords by brute-force. (markt)
       </add>
+      <add>
+        <bug>49478</bug>: Add support for user specified character sets to the
+        <code>AddDefaultCharsetFilter</code>. Based on a patch by Felix
+        Schumacher. (markt)
+      </add>
       <fix>
         <bug>49503</bug>: Make sure connectors bind to their associated ports
         sufficiently early to allow jsvc and the

Modified: tomcat/trunk/webapps/docs/config/filter.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/filter.xml?rev=962917&r1=962916&r2=962917&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/filter.xml (original)
+++ tomcat/trunk/webapps/docs/config/filter.xml Sat Jul 10 21:13:23 2010
@@ -79,8 +79,20 @@
 
   <subsection name="Initialisation parameters">
 
-    <p>The Add Default Character Set Filter does not support any initialization
-    parameters.</p>
+    <p>The Add Default Character Set Filter supports the following initialization
+    parameters:</p>
+
+    <attributes>
+
+      <attribute name="encoding" required="false">
+        <p>Name of the character set which should be set, if no other character set 
+        was set explicitly by a Servlet. This parameter has two special values 
+        <code>default</code> and <code>system</code>. A value of <code>system</code>
+        uses the JVM wide default character set, which is usually set by locale.
+        A value of <code>default</code> will use <strong>ISO-8859-1</strong>.</p>
+      </attribute>
+
+    </attributes>
 
   </subsection>
 



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