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 2023/01/18 20:15:00 UTC

[tomcat] 06/06: Refactor so CoyoteResponse only uses CharsetHolder

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 3944507b61e4db7aab588793cefef167ddce452a
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Jan 18 20:05:16 2023 +0000

    Refactor so CoyoteResponse only uses CharsetHolder
---
 .../apache/catalina/connector/OutputBuffer.java    | 13 ++--
 java/org/apache/catalina/connector/Response.java   | 76 ++++++++++++++--------
 .../apache/catalina/connector/ResponseFacade.java  | 11 +++-
 java/org/apache/coyote/Response.java               | 26 ++++++++
 4 files changed, 89 insertions(+), 37 deletions(-)

diff --git a/java/org/apache/catalina/connector/OutputBuffer.java b/java/org/apache/catalina/connector/OutputBuffer.java
index e3fd2b705c..cba68cee37 100644
--- a/java/org/apache/catalina/connector/OutputBuffer.java
+++ b/java/org/apache/catalina/connector/OutputBuffer.java
@@ -31,8 +31,8 @@ import jakarta.servlet.http.HttpServletResponse;
 import org.apache.coyote.ActionCode;
 import org.apache.coyote.CloseNowException;
 import org.apache.coyote.Response;
-import org.apache.tomcat.util.buf.B2CConverter;
 import org.apache.tomcat.util.buf.C2BConverter;
+import org.apache.tomcat.util.buf.CharsetHolder;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
@@ -551,15 +551,14 @@ public class OutputBuffer extends Writer {
         Charset charset = null;
 
         if (coyoteResponse != null) {
-            charset = coyoteResponse.getCharset();
+            CharsetHolder charsetHolder = coyoteResponse.getCharsetHolder();
+            // setCharacterEncoding() was called with an invalid character set
+            // Trigger an UnsupportedEncodingException
+            charsetHolder.validate();
+            charset = charsetHolder.getCharset();
         }
 
         if (charset == null) {
-            if (coyoteResponse.getCharacterEncoding() != null) {
-                // setCharacterEncoding() was called with an invalid character set
-                // Trigger an UnsupportedEncodingException
-                charset = B2CConverter.getCharset(coyoteResponse.getCharacterEncoding());
-            }
             charset = org.apache.coyote.Constants.DEFAULT_BODY_CHARSET;
         }
 
diff --git a/java/org/apache/catalina/connector/Response.java b/java/org/apache/catalina/connector/Response.java
index 38ddd83640..6c47a21f40 100644
--- a/java/org/apache/catalina/connector/Response.java
+++ b/java/org/apache/catalina/connector/Response.java
@@ -50,6 +50,7 @@ import org.apache.coyote.ContinueResponseTiming;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.buf.CharChunk;
+import org.apache.tomcat.util.buf.CharsetHolder;
 import org.apache.tomcat.util.buf.UEncoder;
 import org.apache.tomcat.util.buf.UEncoder.SafeCharsSet;
 import org.apache.tomcat.util.buf.UriUtil;
@@ -486,22 +487,19 @@ public class Response implements HttpServletResponse {
      */
     @Override
     public String getCharacterEncoding() {
-        String charset = getCoyoteResponse().getCharacterEncoding();
-        if (charset != null) {
-            return charset;
-        }
-
-        Context context = getContext();
-        String result = null;
-        if (context != null) {
-            result =  context.getResponseCharacterEncoding();
+        String charset = getCoyoteResponse().getCharsetHolder().getName();
+        if (charset == null) {
+            Context context = getContext();
+            if (context != null) {
+                charset = context.getResponseCharacterEncoding();
+            }
         }
 
-        if (result == null) {
-            result = org.apache.coyote.Constants.DEFAULT_BODY_CHARSET.name();
+        if (charset == null) {
+            charset = org.apache.coyote.Constants.DEFAULT_BODY_CHARSET.name();
         }
 
-        return result;
+        return charset;
     }
 
 
@@ -720,11 +718,7 @@ public class Response implements HttpServletResponse {
 
         if (type == null) {
             getCoyoteResponse().setContentType(null);
-            try {
-                getCoyoteResponse().setCharacterEncoding(null);
-            } catch (UnsupportedEncodingException e) {
-                // Can never happen when calling with null
-            }
+            getCoyoteResponse().setCharsetHolder(CharsetHolder.EMPTY);
             isCharacterEncodingSet = false;
             return;
         }
@@ -750,8 +744,9 @@ public class Response implements HttpServletResponse {
 
             // Ignore charset if getWriter() has already been called
             if (!usingWriter) {
+                getCoyoteResponse().setCharsetHolder(CharsetHolder.getInstance(m[1]));
                 try {
-                    getCoyoteResponse().setCharacterEncoding(m[1]);
+                    getCoyoteResponse().getCharsetHolder().validate();
                 } catch (UnsupportedEncodingException e) {
                     log.warn(sm.getString("coyoteResponse.encoding.invalid", m[1]), e);
                 }
@@ -767,10 +762,10 @@ public class Response implements HttpServletResponse {
      * of the request. This method must be called prior to reading
      * request parameters or reading input using getReader().
      *
-     * @param charset String containing the name of the character encoding.
+     * @param encoding String containing the name of the character encoding.
      */
     @Override
-    public void setCharacterEncoding(String charset) {
+    public void setCharacterEncoding(String encoding) {
 
         if (isCommitted()) {
             return;
@@ -787,12 +782,40 @@ public class Response implements HttpServletResponse {
             return;
         }
 
+        getCoyoteResponse().setCharsetHolder(CharsetHolder.getInstance(encoding));
         try {
-            getCoyoteResponse().setCharacterEncoding(charset);
+            getCoyoteResponse().getCharsetHolder().validate();
         } catch (UnsupportedEncodingException e) {
-            log.warn(sm.getString("coyoteResponse.encoding.invalid", charset), e);
+            log.warn(sm.getString("coyoteResponse.encoding.invalid", encoding), e);
+            return;
+        }
+        if (encoding == null) {
+            isCharacterEncodingSet = false;
+        } else {
+            isCharacterEncodingSet = true;
+        }
+    }
+
+
+    @Override
+    public void setCharacterEncoding(Charset charset) {
+
+        if (isCommitted()) {
+            return;
+        }
+
+        // Ignore any call from an included servlet
+        if (included) {
             return;
         }
+
+        // Ignore any call made after the getWriter has been invoked
+        // The default should be used
+        if (usingWriter) {
+            return;
+        }
+
+        getCoyoteResponse().setCharsetHolder(CharsetHolder.getInstance(charset));
         if (charset == null) {
             isCharacterEncodingSet = false;
         } else {
@@ -832,11 +855,7 @@ public class Response implements HttpServletResponse {
         }
 
         if (locale == null) {
-            try {
-                getCoyoteResponse().setCharacterEncoding(null);
-            } catch (UnsupportedEncodingException e) {
-                // Impossible when calling with null
-            }
+            getCoyoteResponse().setCharsetHolder(CharsetHolder.EMPTY);
         } else {
             // In some error handling scenarios, the context is unknown
             // (e.g. a 404 when a ROOT context is not present)
@@ -844,8 +863,9 @@ public class Response implements HttpServletResponse {
             if (context != null) {
                 String charset = context.getCharset(locale);
                 if (charset != null) {
+                    getCoyoteResponse().setCharsetHolder(CharsetHolder.getInstance(charset));
                     try {
-                        getCoyoteResponse().setCharacterEncoding(charset);
+                        getCoyoteResponse().getCharsetHolder().validate();
                     } catch (UnsupportedEncodingException e) {
                         log.warn(sm.getString("coyoteResponse.encoding.invalid", charset), e);
                     }
diff --git a/java/org/apache/catalina/connector/ResponseFacade.java b/java/org/apache/catalina/connector/ResponseFacade.java
index a6adbb5851..3d2f71cc41 100644
--- a/java/org/apache/catalina/connector/ResponseFacade.java
+++ b/java/org/apache/catalina/connector/ResponseFacade.java
@@ -18,6 +18,7 @@ package org.apache.catalina.connector;
 
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.nio.charset.Charset;
 import java.util.Collection;
 import java.util.Locale;
 import java.util.Map;
@@ -357,9 +358,15 @@ public class ResponseFacade implements HttpServletResponse {
 
 
     @Override
-    public void setCharacterEncoding(String arg0) {
+    public void setCharacterEncoding(String encoding) {
         checkFacade();
-        response.setCharacterEncoding(arg0);
+        response.setCharacterEncoding(encoding);
+    }
+
+    @Override
+    public void setCharacterEncoding(Charset charset) {
+        checkFacade();
+        response.setCharacterEncoding(charset);
     }
 
     @Override
diff --git a/java/org/apache/coyote/Response.java b/java/org/apache/coyote/Response.java
index 1c9008765b..8401edb64b 100644
--- a/java/org/apache/coyote/Response.java
+++ b/java/org/apache/coyote/Response.java
@@ -496,7 +496,10 @@ public final class Response {
      *
      * @throws UnsupportedEncodingException If the specified name is not
      *         recognised
+     *
+     * @deprecated Unused. Will be removed in Tomcat 11.
      */
+    @Deprecated
     public void setCharacterEncoding(String characterEncoding) throws UnsupportedEncodingException {
         if (isCommitted()) {
             return;
@@ -507,19 +510,42 @@ public final class Response {
     }
 
 
+    /**
+     * Returns the current character set.
+     *
+     * @return The current character set
+     *
+     * @deprecated Unused. Will be removed in Tomcat 11.
+     */
+    @Deprecated
     public Charset getCharset() {
         return charsetHolder.getCharset();
     }
 
 
     /**
+     * Returns the name of the current encoding.
+     *
      * @return The name of the current encoding
+     *
+     * @deprecated Unused. Will be removed in Tomcat 11.
      */
+    @Deprecated
     public String getCharacterEncoding() {
         return charsetHolder.getName();
     }
 
 
+    public CharsetHolder getCharsetHolder() {
+        return charsetHolder;
+    }
+
+
+    public void setCharsetHolder(CharsetHolder charsetHolder) {
+        this.charsetHolder = charsetHolder;
+    }
+
+
     /**
      * Sets the content type.
      *


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org