Author: markt Date: Sun Jul 9 20:25:21 2017 New Revision: 1801386 URL: http://svn.apache.org/viewvc?rev=1801386&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61264 Correct a regression in the refactoring to use Charset rather than String to store request character encoding that prevented getReader() throwing an UnsupportedEncodingException if the user agent specifies an unsupported character encoding.
Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java tomcat/trunk/java/org/apache/catalina/ssi/SSIServletExternalResolver.java tomcat/trunk/java/org/apache/coyote/Request.java tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1801386&r1=1801385&r2=1801386&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Sun Jul 9 20:25:21 2017 @@ -957,9 +957,9 @@ public class Request implements HttpServ */ @Override public String getCharacterEncoding() { - Charset charset = coyoteRequest.getCharset(); - if (charset != null) { - return charset.name(); + String characterEncoding = coyoteRequest.getCharacterEncoding(); + if (characterEncoding != null) { + return characterEncoding; } Context context = getContext(); @@ -972,7 +972,12 @@ public class Request implements HttpServ private Charset getCharset() { - Charset charset = coyoteRequest.getCharset(); + Charset charset = null; + try { + charset = coyoteRequest.getCharset(); + } catch (UnsupportedEncodingException e) { + // Ignore + } if (charset != null) { return charset; } Modified: tomcat/trunk/java/org/apache/catalina/ssi/SSIServletExternalResolver.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ssi/SSIServletExternalResolver.java?rev=1801386&r1=1801385&r2=1801386&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/ssi/SSIServletExternalResolver.java (original) +++ tomcat/trunk/java/org/apache/catalina/ssi/SSIServletExternalResolver.java Sun Jul 9 20:25:21 2017 @@ -18,6 +18,7 @@ package org.apache.catalina.ssi; import java.io.IOException; +import java.io.UnsupportedEncodingException; import java.net.URL; import java.net.URLConnection; import java.nio.charset.Charset; @@ -251,7 +252,11 @@ public class SSIServletExternalResolver // Get encoding settings from request / connector if // possible if (req instanceof Request) { - requestCharset = ((Request)req).getCoyoteRequest().getCharset(); + try { + requestCharset = ((Request)req).getCoyoteRequest().getCharset(); + } catch (UnsupportedEncodingException e) { + // Ignore + } Connector connector = ((Request)req).getConnector(); uriCharset = connector.getURICharset(); useBodyEncodingForURI = connector.getUseBodyEncodingForURI(); Modified: tomcat/trunk/java/org/apache/coyote/Request.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/Request.java?rev=1801386&r1=1801385&r2=1801386&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/Request.java (original) +++ tomcat/trunk/java/org/apache/coyote/Request.java Sun Jul 9 20:25:21 2017 @@ -131,6 +131,10 @@ public final class Request { private long contentLength = -1; private MessageBytes contentTypeMB = null; private Charset charset = null; + // Retain the original, user specified character encoding so it can be + // returned even if it is invalid + private String characterEncoding = null; + /** * Is there an expectation ? */ @@ -301,12 +305,32 @@ public final class Request { * call has been made to that method try to obtain if from the * content type. */ - public Charset getCharset() { - if (charset != null) { - return charset; + public String getCharacterEncoding() { + if (characterEncoding == null) { + characterEncoding = getCharsetFromContentType(getContentType()); } - charset = getCharsetFromContentType(getContentType()); + return characterEncoding; + } + + + /** + * Get the character encoding used for this request. + * + * @return The value set via {@link #setCharset(Charset)} or if no + * call has been made to that method try to obtain if from the + * content type. + * + * @throws UnsupportedEncodingException If the user agent has specified an + * invalid character encoding + */ + public Charset getCharset() throws UnsupportedEncodingException { + if (charset == null) { + getCharacterEncoding(); + if (characterEncoding != null) { + charset = B2CConverter.getCharset(characterEncoding); + } + } return charset; } @@ -314,6 +338,7 @@ public final class Request { public void setCharset(Charset charset) { this.charset = charset; + this.characterEncoding = charset.name(); } @@ -586,6 +611,7 @@ public final class Request { contentLength = -1; contentTypeMB = null; charset = null; + characterEncoding = null; expectation = false; headers.recycle(); trailerFields.clear(); @@ -647,7 +673,7 @@ public final class Request { * * @param contentType a content type header */ - private static Charset getCharsetFromContentType(String contentType) { + private static String getCharsetFromContentType(String contentType) { if (contentType == null) { return null; @@ -667,14 +693,6 @@ public final class Request { encoding = encoding.substring(1, encoding.length() - 1); } - Charset result = null; - - try { - result = B2CConverter.getCharset(encoding.trim()); - } catch (UnsupportedEncodingException e) { - // Ignore - } - - return result; + return encoding.trim(); } } Modified: tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java?rev=1801386&r1=1801385&r2=1801386&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java (original) +++ tomcat/trunk/test/org/apache/catalina/connector/TestRequest.java Sun Jul 9 20:25:21 2017 @@ -25,9 +25,12 @@ import java.io.PrintWriter; import java.net.HttpURLConnection; import java.net.URL; import java.util.ArrayList; +import java.util.Arrays; import java.util.Enumeration; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.TreeMap; import javax.servlet.ServletException; @@ -903,4 +906,66 @@ public class TestRequest extends TomcatB System.out.println(time); } + + + @Test + public void testGetReaderValidEncoding() throws Exception { + doTestGetReader("ISO-8859-1", true); + } + + + @Test + public void testGetReaderInvalidEbcoding() throws Exception { + doTestGetReader("X-Invalid", false); + } + + + private void doTestGetReader(String userAgentCharaceterEncoding, boolean expect200) + throws Exception { + + // Setup Tomcat instance + Tomcat tomcat = getTomcatInstance(); + + // No file system docBase required + Context ctx = tomcat.addContext("", null); + + Tomcat.addServlet(ctx, "servlet", new Bug61264GetReaderServlet()); + ctx.addServletMappingDecoded("/", "servlet"); + + tomcat.start(); + + byte[] body = "Test".getBytes(); + ByteChunk bc = new ByteChunk(); + Map<String,List<String>> reqHeaders = new HashMap<>(); + reqHeaders.put("Content-Type", + Arrays.asList(new String[] {"text/plain;charset=" + userAgentCharaceterEncoding})); + + int rc = postUrl(body, "http://localhost:" + getPort() + "/", bc, reqHeaders, null); + + if (expect200) { + assertEquals(200, rc); + } else { + assertEquals(500, rc); + } + } + + + private class Bug61264GetReaderServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + // This is intended for POST requests + resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + // Container will handle any errors + req.getReader(); + } + } } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1801386&r1=1801385&r2=1801386&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Sun Jul 9 20:25:21 2017 @@ -55,6 +55,13 @@ not take precedence over any settings in <code>/WEB-INF/web.xml</code>. (markt) </add> + <fix> + <bug>61264</bug>: Correct a regression in the refactoring to use + <code>Charset</code> rather than <code>String</code> to store request + character encoding that prevented <code>getReader()</code> throwing an + <code>UnsupportedEncodingException</code> if the user agent specifies + an unsupported character encoding. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org