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