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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
