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 2012/03/13 15:41:28 UTC

svn commit: r1300155 - in /tomcat/trunk: java/org/apache/catalina/connector/Response.java test/org/apache/catalina/connector/TestResponse.java

Author: markt
Date: Tue Mar 13 14:41:27 2012
New Revision: 1300155

URL: http://svn.apache.org/viewvc?rev=1300155&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52811
Make use of the newly added HttpParser to correctly process Content-Type headers
As well as the problem identified in the bug report, this fixes a couple of related issues:
- setting the content type via (set|add)Header bypassed some checks
- avoids parsing the content-type header multiple times

Modified:
    tomcat/trunk/java/org/apache/catalina/connector/Response.java
    tomcat/trunk/test/org/apache/catalina/connector/TestResponse.java

Modified: tomcat/trunk/java/org/apache/catalina/connector/Response.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Response.java?rev=1300155&r1=1300154&r2=1300155&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Response.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Response.java Tue Mar 13 14:41:27 2012
@@ -19,6 +19,7 @@ package org.apache.catalina.connector;
 
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.io.StringReader;
 import java.net.MalformedURLException;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
@@ -52,6 +53,9 @@ import org.apache.tomcat.util.buf.UEncod
 import org.apache.tomcat.util.http.FastHttpDateFormat;
 import org.apache.tomcat.util.http.MimeHeaders;
 import org.apache.tomcat.util.http.ServerCookie;
+import org.apache.tomcat.util.http.parser.AstMediaType;
+import org.apache.tomcat.util.http.parser.HttpParser;
+import org.apache.tomcat.util.http.parser.ParseException;
 import org.apache.tomcat.util.net.URL;
 import org.apache.tomcat.util.res.StringManager;
 
@@ -684,7 +688,6 @@ public class Response
      * @param type The new content type
      */
     @Override
-    @SuppressWarnings("deprecation") // isSpace (deprecated) cannot be replaced by isWhiteSpace
     public void setContentType(String type) {
 
         if (isCommitted()) {
@@ -696,39 +699,30 @@ public class Response
             return;
         }
 
-        // Ignore charset if getWriter() has already been called
-        if (usingWriter) {
-            if (type != null) {
-                int index = type.indexOf(";");
-                if (index != -1) {
-                    type = type.substring(0, index);
-                }
-            }
+        if (type == null) {
+            coyoteResponse.setContentType(null);
+            return;
         }
 
-        coyoteResponse.setContentType(type);
+        AstMediaType m = null;
+        HttpParser hp = new HttpParser(new StringReader(type));
+        try {
+             m = hp.MediaType();
+        } catch (ParseException e) {
+            // Invalid - Assume no charset and just pass through whatever
+            // the user provided.
+            coyoteResponse.setContentTypeNoCharset(type);
+            return;
+        }
 
-        // Check to see if content type contains charset
-        if (type != null) {
-            int index = type.indexOf(";");
-            if (index != -1) {
-                int len = type.length();
-                index++;
-                // N.B. isSpace (deprecated) cannot be replaced by isWhiteSpace
-                while (index < len && Character.isSpace(type.charAt(index))) {
-                    index++;
-                }
-                if (index+7 < len
-                        && type.charAt(index) == 'c'
-                        && type.charAt(index+1) == 'h'
-                        && type.charAt(index+2) == 'a'
-                        && type.charAt(index+3) == 'r'
-                        && type.charAt(index+4) == 's'
-                        && type.charAt(index+5) == 'e'
-                        && type.charAt(index+6) == 't'
-                        && type.charAt(index+7) == '=') {
-                    isCharacterEncodingSet = true;
-                }
+        coyoteResponse.setContentTypeNoCharset(m.toStringNoCharset());
+
+        String charset = m.getCharset();
+        if (charset != null) {
+            // Ignore charset if getWriter() has already been called
+            if (!usingWriter) {
+                coyoteResponse.setCharacterEncoding(charset);
+                isCharacterEncodingSet = true;
             }
         }
     }
@@ -1013,8 +1007,31 @@ public class Response
             return;
         }
 
+        char cc=name.charAt(0);
+        if (cc=='C' || cc=='c') {
+            if (checkSpecialHeader(name, value))
+            return;
+        }
+
         coyoteResponse.addHeader(name, value);
+    }
+
 
+    /**
+     * An extended version of this exists in {@link org.apache.coyote.Response}.
+     * This check is required here to ensure that the usingWriter checks in
+     * {@link #setContentType(String)} are applied since usingWriter is not
+     * visible to {@link org.apache.coyote.Response}
+     *
+     * Called from set/addHeader.
+     * Return true if the header is special, no need to set the header.
+     */
+    private boolean checkSpecialHeader(String name, String value) {
+        if (name.equalsIgnoreCase("Content-Type")) {
+            setContentType(value);
+            return true;
+        }
+        return false;
     }
 
 
@@ -1329,8 +1346,13 @@ public class Response
             return;
         }
 
-        coyoteResponse.setHeader(name, value);
+        char cc=name.charAt(0);
+        if (cc=='C' || cc=='c') {
+            if (checkSpecialHeader(name, value))
+            return;
+        }
 
+        coyoteResponse.setHeader(name, value);
     }
 
 

Modified: tomcat/trunk/test/org/apache/catalina/connector/TestResponse.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/connector/TestResponse.java?rev=1300155&r1=1300154&r2=1300155&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/connector/TestResponse.java (original)
+++ tomcat/trunk/test/org/apache/catalina/connector/TestResponse.java Tue Mar 13 14:41:27 2012
@@ -19,6 +19,7 @@ package org.apache.catalina.connector;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -96,4 +97,91 @@ public class TestResponse extends Tomcat
         }
 
     }
+
+
+    /**
+     * Tests an issue noticed during the investigation of BZ 52811.
+     */
+    @Test
+    public void testCharset() throws Exception {
+        // Setup Tomcat instance
+        Tomcat tomcat = getTomcatInstance();
+
+        // Must have a real docBase - just use temp
+        File docBase = new File(System.getProperty("java.io.tmpdir"));
+        Context ctx = tomcat.addContext("", docBase.getAbsolutePath());
+
+        Tomcat.addServlet(ctx, "servlet", new CharsetServlet());
+        ctx.addServletMapping("/", "servlet");
+
+        tomcat.start();
+
+        ByteChunk bc = getUrl("http://localhost:" + getPort() + "/");
+
+        assertEquals("OK", bc.toString());
+    }
+
+    private static final class CharsetServlet extends HttpServlet {
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            PrintWriter pw = resp.getWriter();
+            resp.setHeader("Content-Type", "text/plain;charset=UTF-8");
+
+            // Should be ISO-8859-1 because getWriter() was called before
+            // setHeader()
+            if (resp.getCharacterEncoding().equals("ISO-8859-1")) {
+                pw.print("OK");
+            } else {
+                pw.print("FAIL: " + resp.getCharacterEncoding());
+            }
+        }
+
+    }
+
+
+    @Test
+    public void testBug52811() throws Exception {
+        // Setup Tomcat instance
+        Tomcat tomcat = getTomcatInstance();
+
+        // Must have a real docBase - just use temp
+        File docBase = new File(System.getProperty("java.io.tmpdir"));
+        Context ctx = tomcat.addContext("", docBase.getAbsolutePath());
+
+        Tomcat.addServlet(ctx, "servlet", new Bug52811Servlet());
+        ctx.addServletMapping("/", "servlet");
+
+        tomcat.start();
+
+        ByteChunk bc = getUrl("http://localhost:" + getPort() + "/");
+
+        assertEquals("OK", bc.toString());
+    }
+
+    private static final class Bug52811Servlet extends HttpServlet {
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+
+            resp.setContentType("multipart/related;" +
+                    "boundary=1_4F50BD36_CDF8C28;" +
+                    "Start=\"<31671603.smil>\";" +
+                    "Type=\"application/smil;charset=UTF-8\"");
+
+            // Should be ISO-8859-1 because the charset in the above is part
+            // of the Type parameter
+            PrintWriter pw = resp.getWriter();
+            if (resp.getCharacterEncoding().equals("ISO-8859-1")) {
+                pw.print("OK");
+            } else {
+                pw.print("FAIL: " + resp.getCharacterEncoding());
+            }
+        }
+
+    }
 }



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