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 2017/07/09 20:25:21 UTC

svn commit: r1801386 - in /tomcat/trunk: java/org/apache/catalina/connector/ java/org/apache/catalina/ssi/ java/org/apache/coyote/ test/org/apache/catalina/connector/ webapps/docs/

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