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:45:55 UTC
svn commit: r1300161 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/connector/Response.java
java/org/apache/coyote/Response.java
test/org/apache/catalina/connector/TestResponse.java
Author: markt
Date: Tue Mar 13 14:45:54 2012
New Revision: 1300161
URL: http://svn.apache.org/viewvc?rev=1300161&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/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java
tomcat/tc7.0.x/trunk/java/org/apache/coyote/Response.java
tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Mar 13 14:45:54 2012
@@ -1 +1 @@
-/tomcat/trunk:1156115-1157160,1157162-1157859,1157862-1157942,1157945-1160347,1160349-1163716,1163718-1166689,1166691-1174340,1174342-1175596,1175598-1175611,1175613-1175932,1175934-1177783,1177850,1177862,1177978,1178209,1178228,1178233,1178449,1178542,1178681,1178684,1178721,1179268,1179274,1180261,1180865,1180891,1180894,1180907,1181028,1181123,1181125,1181136,1181291,1181743,1182796,1183078,1183105,1183142,1183328,1183339-1183340,1183492-1183494,1183605,1184917,1184919,1185018,1185020,1185200,1185588,1185626,1185756,1185758,1186011,1186042-1186045,1186104,1186123,1186137,1186153,1186254,1186257,1186377-1186379,1186479-1186480,1186712,1186743,1186750,1186763,1186890-1186892,1186894,1186949,1187018,1187027-1187028,1187381,1187753,1187755,1187775,1187801,1187806,1187809,1187827,1188301,1188303-1188305,1188399,1188822,1188930-1188931,1189116,1189129,1189183,1189240,1189256,1189386,1189413-1189414,1189477,1189685,1189805,1189857,1189864,1189882,1190034,1190185,1190279,1190339
,1190371,1190388-1190389,1190474,1190481,1194915,1195222-1195223,1195531,1195899,1195905,1195943,1195949,1195953,1195955,1195965,1195968,1196175,1196212,1196223,1196304-1196305,1196735,1196825,1196827,1197158,1197261,1197263,1197299-1197300,1197305,1197339-1197340,1197343,1197382,1197386-1197387,1197480,1197578,1198497,1198528,1198552,1198602,1198604,1198607,1198622,1198640,1198696,1198707,1199418,1199432,1199436,1199513,1199529,1199980,1199996,1200056,1200089,1200106-1200107,1200263,1200316,1200320,1200398-1200399,1200445-1200446,1200555,1200627,1200696,1200725,1200937,1200941,1201069,1201087,1201180,1201235-1201237,1201508,1201521,1201542,1201545-1201546,1201548,1201555-1201556,1201568,1201576,1201608,1201921-1201922,1201931,1202035,1202039,1202271,1202565,1202578,1202705,1202828,1202860,1203047-1203052,1203078,1203091,1203253,1203278,1204182,1204856,1204867,1204936,1204938,1204982,1205033,1205065,1205082,1205097,1205112,1206200,1207692,1208046,1208073,1208096,1208114,1208
145,1208772,1209194,1209277-1209278,1209686-1209731,1210894,1212091,1212095,1212099,1212118,1213469,1213906,1214853,1214855,1214864,1215115,1215118-1215119,1215121,1220293,1220295,1221038,1221842,1222189,1222201,1222276,1222300,1222690,1222850,1222852,1222855,1224607,1224617,1224648-1224652,1224657,1224662-1224663,1224682,1224801,1224910,1225000,1225219,1225343,1225465,1225627,1225629,1225634,1226069,1226158-1226159,1226177,1226196,1226214-1226215,1226385,1226394,1226500,1226537-1226538,1226546,1226551,1226975,1228196,1228360,1228376,1228724,1228908,1228918,1228920,1228922,1228929,1228969,1229307,1229536,1229549,1229724,1229726-1229731,1229997,1230539,1230711,1230729,1230762-1230763,1230765,1230955,1230957,1231285,1231290,1231308,1231310,1231337,1231460-1231461,1231542-1231543,1231546-1231547,1231620-1231621,1231624-1231625,1231630,1231654-1231655,1231738,1231740,1231762-1231763,1231856,1231886,1231923,1231947,1232345,1232368,1232380,1232447,1232760,1232813,1232842-1232843,1
232869,1233413,1233423,1233426,1234143,1234567,1235207,1236906-1236907,1236914,1237146,1237154-1237156,1237332,1237334,1237425,1237427,1237604,1237975,1237981,1237985,1238070,1238073,1239024,1239048,1239050,1239060,1239135,1239483,1239485,1240101,1240106,1240109,1240112,1240114,1240116,1240118,1240121,1240329,1240697,1240795,1240821,1240842,1240857,1241087,1241160,1241908-1241909,1241982,1242099,1242110,1242371,1242434,1242495,1242947,1243034,1243038,1244302,1244511,1244567,1244718-1244719,1244935-1244938,1245274,1245449,1245849,1290875,1292334,1292338,1292345-1292347,1293155,1293831-1293832,1295998,1297014-1297015,1297017,1297158,1297177,1297202,1297209,1297213,1297717,1297722,1297729,1297768,1297778,1297818,1297828,1297979,1297987,1298121,1298140,1298590,1298592,1298628-1298629,1298794,1298983-1298984,1299020,1299034,1299819,1300154
+/tomcat/trunk:1156115-1157160,1157162-1157859,1157862-1157942,1157945-1160347,1160349-1163716,1163718-1166689,1166691-1174340,1174342-1175596,1175598-1175611,1175613-1175932,1175934-1177783,1177850,1177862,1177978,1178209,1178228,1178233,1178449,1178542,1178681,1178684,1178721,1179268,1179274,1180261,1180865,1180891,1180894,1180907,1181028,1181123,1181125,1181136,1181291,1181743,1182796,1183078,1183105,1183142,1183328,1183339-1183340,1183492-1183494,1183605,1184917,1184919,1185018,1185020,1185200,1185588,1185626,1185756,1185758,1186011,1186042-1186045,1186104,1186123,1186137,1186153,1186254,1186257,1186377-1186379,1186479-1186480,1186712,1186743,1186750,1186763,1186890-1186892,1186894,1186949,1187018,1187027-1187028,1187381,1187753,1187755,1187775,1187801,1187806,1187809,1187827,1188301,1188303-1188305,1188399,1188822,1188930-1188931,1189116,1189129,1189183,1189240,1189256,1189386,1189413-1189414,1189477,1189685,1189805,1189857,1189864,1189882,1190034,1190185,1190279,1190339
,1190371,1190388-1190389,1190474,1190481,1194915,1195222-1195223,1195531,1195899,1195905,1195943,1195949,1195953,1195955,1195965,1195968,1196175,1196212,1196223,1196304-1196305,1196735,1196825,1196827,1197158,1197261,1197263,1197299-1197300,1197305,1197339-1197340,1197343,1197382,1197386-1197387,1197480,1197578,1198497,1198528,1198552,1198602,1198604,1198607,1198622,1198640,1198696,1198707,1199418,1199432,1199436,1199513,1199529,1199980,1199996,1200056,1200089,1200106-1200107,1200263,1200316,1200320,1200398-1200399,1200445-1200446,1200555,1200627,1200696,1200725,1200937,1200941,1201069,1201087,1201180,1201235-1201237,1201508,1201521,1201542,1201545-1201546,1201548,1201555-1201556,1201568,1201576,1201608,1201921-1201922,1201931,1202035,1202039,1202271,1202565,1202578,1202705,1202828,1202860,1203047-1203052,1203078,1203091,1203253,1203278,1204182,1204856,1204867,1204936,1204938,1204982,1205033,1205065,1205082,1205097,1205112,1206200,1207692,1208046,1208073,1208096,1208114,1208
145,1208772,1209194,1209277-1209278,1209686-1209731,1210894,1212091,1212095,1212099,1212118,1213469,1213906,1214853,1214855,1214864,1215115,1215118-1215119,1215121,1220293,1220295,1221038,1221842,1222189,1222201,1222276,1222300,1222690,1222850,1222852,1222855,1224607,1224617,1224648-1224652,1224657,1224662-1224663,1224682,1224801,1224910,1225000,1225219,1225343,1225465,1225627,1225629,1225634,1226069,1226158-1226159,1226177,1226196,1226214-1226215,1226385,1226394,1226500,1226537-1226538,1226546,1226551,1226975,1228196,1228360,1228376,1228724,1228908,1228918,1228920,1228922,1228929,1228969,1229307,1229536,1229549,1229724,1229726-1229731,1229997,1230539,1230711,1230729,1230762-1230763,1230765,1230955,1230957,1231285,1231290,1231308,1231310,1231337,1231460-1231461,1231542-1231543,1231546-1231547,1231620-1231621,1231624-1231625,1231630,1231654-1231655,1231738,1231740,1231762-1231763,1231856,1231886,1231923,1231947,1232345,1232368,1232380,1232447,1232760,1232813,1232842-1232843,1
232869,1233413,1233423,1233426,1234143,1234567,1235207,1236906-1236907,1236914,1237146,1237154-1237156,1237332,1237334,1237425,1237427,1237604,1237975,1237981,1237985,1238070,1238073,1239024,1239048,1239050,1239060,1239135,1239483,1239485,1240101,1240106,1240109,1240112,1240114,1240116,1240118,1240121,1240329,1240697,1240795,1240821,1240842,1240857,1241087,1241160,1241908-1241909,1241982,1242099,1242110,1242371,1242434,1242495,1242947,1243034,1243038,1244302,1244511,1244567,1244718-1244719,1244935-1244938,1245274,1245449,1245849,1290875,1292334,1292338,1292345-1292347,1293155,1293831-1293832,1295998,1297014-1297015,1297017,1297158,1297177,1297202,1297209,1297213,1297717,1297722,1297729,1297768,1297778,1297818,1297828,1297979,1297987,1298121,1298140,1298590,1298592,1298628-1298629,1298794,1298983-1298984,1299020,1299034,1299819,1300154-1300155
Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java?rev=1300161&r1=1300160&r2=1300161&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java Tue Mar 13 14:45:54 2012
@@ -20,6 +20,7 @@ package org.apache.catalina.connector;
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintWriter;
+import java.io.StringReader;
import java.net.MalformedURLException;
import java.security.AccessController;
import java.security.PrivilegedAction;
@@ -53,6 +54,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;
@@ -781,7 +785,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()) {
@@ -793,39 +796,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;
}
}
}
@@ -1125,8 +1119,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;
}
@@ -1441,8 +1458,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/tc7.0.x/trunk/java/org/apache/coyote/Response.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/Response.java?rev=1300161&r1=1300160&r2=1300161&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/Response.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/Response.java Tue Mar 13 14:45:54 2012
@@ -18,10 +18,14 @@
package org.apache.coyote;
import java.io.IOException;
+import java.io.StringReader;
import java.util.Locale;
import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.http.MimeHeaders;
+import org.apache.tomcat.util.http.parser.AstMediaType;
+import org.apache.tomcat.util.http.parser.HttpParser;
+import org.apache.tomcat.util.http.parser.ParseException;
/**
* Response object.
@@ -424,71 +428,38 @@ public final class Response {
*
* @param type the content type
*/
- @SuppressWarnings("deprecation")
public void setContentType(String type) {
- int semicolonIndex = -1;
-
if (type == null) {
this.contentType = null;
return;
}
- /*
- * Remove the charset param (if any) from the Content-Type, and use it
- * to set the response encoding.
- * The most recent response encoding setting will be appended to the
- * response's Content-Type (as its charset param) by getContentType();
- */
- boolean hasCharset = false;
- int len = type.length();
- int index = type.indexOf(';');
- while (index != -1) {
- semicolonIndex = index;
- index++;
- // Yes, isSpace() is deprecated but it does exactly what we need
- while (index < len && Character.isSpace(type.charAt(index))) {
- index++;
- }
- if (index+8 < 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) == '=') {
- hasCharset = true;
- break;
- }
- index = type.indexOf(';', index);
- }
-
- if (!hasCharset) {
+ 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.
this.contentType = type;
return;
}
- this.contentType = type.substring(0, semicolonIndex);
- String tail = type.substring(index+8);
- int nextParam = tail.indexOf(';');
- String charsetValue = null;
- if (nextParam != -1) {
- this.contentType += tail.substring(nextParam);
- charsetValue = tail.substring(0, nextParam);
- } else {
- charsetValue = tail;
- }
+ this.contentType = m.toStringNoCharset();
+
+ String charsetValue = m.getCharset().trim();
- // The charset value may be quoted, but must not contain any quotes.
if (charsetValue != null && charsetValue.length() > 0) {
- charsetSet=true;
- charsetValue = charsetValue.replace('"', ' ');
- this.characterEncoding = charsetValue.trim();
+ charsetSet = true;
+ this.characterEncoding = charsetValue;
}
}
+ public void setContentTypeNoCharset(String type) {
+ this.contentType = type;
+ }
+
public String getContentType() {
String ret = contentType;
Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java?rev=1300161&r1=1300160&r2=1300161&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java Tue Mar 13 14:45:54 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