This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 12ac01b Fix BZ 64938 Clarify expected behaviour of
setCharacterEncoding(null)
12ac01b is described below
commit 12ac01b99599404809772f263a26d554e3ccba75
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Feb 23 12:49:59 2021 +0000
Fix BZ 64938 Clarify expected behaviour of setCharacterEncoding(null)
Also covers setContentType(null) and setLocale(null)
https://bz.apache.org/bugzilla/show_bug.cgi?id=64938
---
.../apache/catalina/connector/OutputBuffer.java | 5 +
java/org/apache/catalina/connector/Response.java | 42 ++-
java/org/apache/coyote/Response.java | 6 +-
.../apache/catalina/connector/TestResponse.java | 345 +++++++++++++++++++++
test/org/apache/tomcat/unittest/TesterContext.java | 23 +-
webapps/docs/changelog.xml | 7 +
6 files changed, 409 insertions(+), 19 deletions(-)
diff --git a/java/org/apache/catalina/connector/OutputBuffer.java
b/java/org/apache/catalina/connector/OutputBuffer.java
index 14c9452..958a506 100644
--- a/java/org/apache/catalina/connector/OutputBuffer.java
+++ b/java/org/apache/catalina/connector/OutputBuffer.java
@@ -591,6 +591,11 @@ public class OutputBuffer extends Writer {
}
if (charset == null) {
+ if (coyoteResponse.getCharacterEncoding() != null) {
+ // setCharacterEncoding() was called with an invalid character
set
+ // Trigger an UnsupportedEncodingException
+ charset =
B2CConverter.getCharset(coyoteResponse.getCharacterEncoding());
+ }
if (enc == null) {
charset = org.apache.coyote.Constants.DEFAULT_BODY_CHARSET;
} else {
diff --git a/java/org/apache/catalina/connector/Response.java
b/java/org/apache/catalina/connector/Response.java
index 860cca9..0d59924 100644
--- a/java/org/apache/catalina/connector/Response.java
+++ b/java/org/apache/catalina/connector/Response.java
@@ -752,6 +752,12 @@ public class Response implements HttpServletResponse {
if (type == null) {
getCoyoteResponse().setContentType(null);
+ try {
+ getCoyoteResponse().setCharacterEncoding(null);
+ } catch (IllegalArgumentException e) {
+ // Can never happen when calling with null
+ }
+ isCharacterEncodingSet = false;
return;
}
@@ -811,7 +817,11 @@ public class Response implements HttpServletResponse {
log.warn(sm.getString("coyoteResponse.encoding.invalid", charset),
e);
return;
}
- isCharacterEncodingSet = true;
+ if (charset == null) {
+ isCharacterEncodingSet = false;
+ } else {
+ isCharacterEncodingSet = true;
+ }
}
@@ -845,16 +855,24 @@ public class Response implements HttpServletResponse {
return;
}
- // In some error handling scenarios, the context is unknown
- // (e.g. a 404 when a ROOT context is not present)
- Context context = getContext();
- if (context != null) {
- String charset = context.getCharset(locale);
- if (charset != null) {
- try {
- getCoyoteResponse().setCharacterEncoding(charset);
- } catch (IllegalArgumentException e) {
- log.warn(sm.getString("coyoteResponse.encoding.invalid",
charset), e);
+ if (locale == null) {
+ try {
+ getCoyoteResponse().setCharacterEncoding(null);
+ } catch (IllegalArgumentException e) {
+ // Impossible when calling with null
+ }
+ } else {
+ // In some error handling scenarios, the context is unknown
+ // (e.g. a 404 when a ROOT context is not present)
+ Context context = getContext();
+ if (context != null) {
+ String charset = context.getCharset(locale);
+ if (charset != null) {
+ try {
+ getCoyoteResponse().setCharacterEncoding(charset);
+ } catch (IllegalArgumentException e) {
+
log.warn(sm.getString("coyoteResponse.encoding.invalid", charset), e);
+ }
}
}
}
@@ -1583,7 +1601,7 @@ public class Response implements HttpServletResponse {
if (!file.startsWith(contextPath)) {
return false;
}
- String tok = ";" +
SessionConfig.getSessionUriParamName(request.getContext()) + "=" +
+ String tok = ";" +
SessionConfig.getSessionUriParamName(request.getContext()) + "=" +
session.getIdInternal();
if( file.indexOf(tok, contextPath.length()) >= 0 ) {
return false;
diff --git a/java/org/apache/coyote/Response.java
b/java/org/apache/coyote/Response.java
index be8eb00..9e7eaf3 100644
--- a/java/org/apache/coyote/Response.java
+++ b/java/org/apache/coyote/Response.java
@@ -437,6 +437,8 @@ public final class Response {
public void setLocale(Locale locale) {
if (locale == null) {
+ this.locale = null;
+ this.contentLanguage = null;
return;
}
@@ -469,15 +471,17 @@ public final class Response {
return;
}
if (characterEncoding == null) {
+ this.charset = null;
+ this.characterEncoding = null;
return;
}
+ this.characterEncoding = characterEncoding;
try {
this.charset = B2CConverter.getCharset(characterEncoding);
} catch (UnsupportedEncodingException e) {
throw new IllegalArgumentException(e);
}
- this.characterEncoding = characterEncoding;
}
diff --git a/test/org/apache/catalina/connector/TestResponse.java
b/test/org/apache/catalina/connector/TestResponse.java
index 14f78f3..620c532 100644
--- a/test/org/apache/catalina/connector/TestResponse.java
+++ b/test/org/apache/catalina/connector/TestResponse.java
@@ -18,7 +18,10 @@ package org.apache.catalina.connector;
import java.io.IOException;
import java.io.PrintWriter;
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.StandardCharsets;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import javax.servlet.ServletException;
@@ -636,6 +639,348 @@ public class TestResponse extends TomcatBaseTest {
}
+ private static final String ISO_8859_1 =
StandardCharsets.ISO_8859_1.name();
+ private static final String UTF_8 = StandardCharsets.UTF_8.name();
+ private static final String UNKNOWN = "unknown";
+ private static final String TEXT = "text/plain";
+ private static final String TEXT_ISO_8859_1 = TEXT + ";charset=" +
ISO_8859_1;
+ private static final String TEXT_UTF_8 = TEXT + ";charset=" + UTF_8;
+ private static final String TEXT_UNKNOWN = TEXT + ";charset=" + UNKNOWN;
+ private static final Locale UNDETERMINED = new Locale("und");
+
+ @Test
+ public void testSetCharacterEncoding01() {
+ Response response = setupResponse();
+
+ // Check default
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetCharacterEncoding02() {
+ Response response = setupResponse();
+
+ // Check multiple calls
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setCharacterEncoding(UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setCharacterEncoding(ISO_8859_1);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetCharacterEncoding03() throws IOException {
+ Response response = setupResponse();
+
+ // Check after getWriter()
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setCharacterEncoding(UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.getWriter();
+ response.setCharacterEncoding(ISO_8859_1);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetCharacterEncoding04() throws IOException {
+ Response response = setupResponse();
+
+ // Check after commit
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setCharacterEncoding(UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.flushBuffer();
+ response.setCharacterEncoding(ISO_8859_1);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetCharacterEncoding05() {
+ Response response = setupResponse();
+
+ // Check calling with null
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setCharacterEncoding(UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setCharacterEncoding(null);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ @Test(expected = UnsupportedEncodingException.class)
+ public void testSetCharacterEncoding06() throws IOException {
+ Response response = setupResponse();
+
+ // Check calling with an unknown character set and writer
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setCharacterEncoding(UNKNOWN);
+ Assert.assertEquals(UNKNOWN, response.getCharacterEncoding());
+ response.getWriter();
+ }
+
+
+ @Test
+ public void testSetCharacterEncoding07() throws IOException {
+ Response response = setupResponse();
+
+ // Check calling with an unknown character set
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setCharacterEncoding(UNKNOWN);
+ Assert.assertEquals(UNKNOWN, response.getCharacterEncoding());
+ response.getOutputStream();
+ }
+
+
+ @Test
+ public void testSetCharacterEncoding08() {
+ Response response = setupResponse();
+
+ // Check multiple calls with different methods
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setCharacterEncoding(UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setContentType(TEXT_ISO_8859_1);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setContentType(TEXT_UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setCharacterEncoding(ISO_8859_1);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetContentType01() {
+ Response response = setupResponse();
+
+ // Check multiple calls
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setContentType(TEXT_UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setContentType(TEXT_ISO_8859_1);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetContentType02() throws IOException {
+ Response response = setupResponse();
+
+ // Check after getWriter()
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setContentType(TEXT_UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.getWriter();
+ response.setContentType(TEXT_ISO_8859_1);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetContentType03() throws IOException {
+ Response response = setupResponse();
+
+ // Check after commit
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setContentType(TEXT_UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.flushBuffer();
+ response.setContentType(TEXT_ISO_8859_1);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetContentType04() {
+ Response response = setupResponse();
+
+ // Check calling with null
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setContentType(TEXT_UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setContentType(null);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ @Test(expected = UnsupportedEncodingException.class)
+ public void testSetContentType05() throws IOException {
+ Response response = setupResponse();
+
response.getContext().addLocaleEncodingMappingParameter(Locale.UK.toLanguageTag(),
UNKNOWN);
+
+ // Check calling with an unknown character set and writer
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setContentType(TEXT_UNKNOWN);
+ Assert.assertEquals(UNKNOWN, response.getCharacterEncoding());
+ response.getWriter();
+ }
+
+
+ @Test
+ public void testSetContentType06() throws IOException {
+ Response response = setupResponse();
+
+ // Check calling with an unknown character set
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setContentType(TEXT_UNKNOWN);
+ Assert.assertEquals(UNKNOWN, response.getCharacterEncoding());
+ response.getOutputStream();
+ }
+
+
+ @Test
+ public void testSetLocale01() {
+ Response response = setupResponse();
+
+ // Check multiple calls
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setLocale(Locale.CHINESE);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setLocale(Locale.ENGLISH);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetLocale02() throws IOException {
+ Response response = setupResponse();
+
+ // Check after getWriter()
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setLocale(Locale.CHINESE);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.getWriter();
+ response.setLocale(Locale.ENGLISH);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetLocale03() throws IOException {
+ Response response = setupResponse();
+
+ // Check after commit
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setLocale(Locale.CHINESE);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.flushBuffer();
+ response.setLocale(Locale.ENGLISH);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetLocale04() {
+ Response response = setupResponse();
+
+ // Check calling with null
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setLocale(Locale.CHINESE);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setLocale(null);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ @Test(expected = UnsupportedEncodingException.class)
+ public void testSetLocale05() throws IOException {
+ Response response = setupResponse();
+
+ // Check calling with an unknown character set and writer
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setLocale(UNDETERMINED);
+ Assert.assertEquals(UNKNOWN, response.getCharacterEncoding());
+ response.getWriter();
+ }
+
+
+ @Test
+ public void testSetLocale06() throws IOException {
+ Response response = setupResponse();
+
+ // Check calling with an unknown character set
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ response.setLocale(UNDETERMINED);
+ Assert.assertEquals(UNKNOWN, response.getCharacterEncoding());
+ response.getOutputStream();
+ }
+
+
+ @Test
+ public void testSetLocale07() {
+ Response response = setupResponse();
+
+ // Check setLocale() is over-ridden by setCharacterEncoding
+
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+
+ // setLocale doesn't change previous value
+ response.setCharacterEncoding(UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setLocale(Locale.ENGLISH);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+
+ // Reset
+ response.setCharacterEncoding(null);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+
+ // setLocale is over-ridden by setCharacterEncoding
+ response.setLocale(Locale.CHINESE);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setCharacterEncoding(ISO_8859_1);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ @Test
+ public void testSetLocale08() {
+ Response response = setupResponse();
+
+ // Check setLocale() is over-ridden by setContentType
+
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+
+ // setLocale doesn't change previous value
+ response.setContentType(TEXT_UTF_8);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setLocale(Locale.ENGLISH);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+
+ // Reset
+ response.setContentType(null);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+
+ // setLocale is over-ridden by setContentTpe
+ response.setLocale(Locale.CHINESE);
+ Assert.assertEquals(UTF_8, response.getCharacterEncoding());
+ response.setContentType(TEXT_ISO_8859_1);
+ Assert.assertEquals(ISO_8859_1, response.getCharacterEncoding());
+ }
+
+
+ private Response setupResponse() {
+ Connector connector = new Connector();
+ org.apache.coyote.Response cResponse = new
org.apache.coyote.Response();
+ Response response = new Response();
+ response.setConnector(connector);
+ response.setCoyoteResponse(cResponse);
+ Request request = new Request();
+ request.setConnector(connector);
+ org.apache.coyote.Request cRequest = new org.apache.coyote.Request();
+ request.setCoyoteRequest(cRequest);
+ Context context = new TesterContext();
+ request.getMappingData().context = context;
+ response.setRequest(request);
+
context.addLocaleEncodingMappingParameter(Locale.ENGLISH.getLanguage(),
ISO_8859_1);
+
context.addLocaleEncodingMappingParameter(Locale.CHINESE.getLanguage(), UTF_8);
+
context.addLocaleEncodingMappingParameter(UNDETERMINED.toLanguageTag(),
UNKNOWN);
+ return response;
+ }
+
+
private static final class Bug52811Servlet extends HttpServlet {
private static final long serialVersionUID = 1L;
diff --git a/test/org/apache/tomcat/unittest/TesterContext.java
b/test/org/apache/tomcat/unittest/TesterContext.java
index 7a24e8e..68f13d5 100644
--- a/test/org/apache/tomcat/unittest/TesterContext.java
+++ b/test/org/apache/tomcat/unittest/TesterContext.java
@@ -24,6 +24,7 @@ import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import javax.management.ObjectName;
import javax.servlet.ServletContainerInitializer;
@@ -364,11 +365,6 @@ public class TesterContext implements Context {
}
@Override
- public String getCharset(Locale locale) {
- return null;
- }
-
- @Override
public URL getConfigFile() {
return null;
}
@@ -738,9 +734,24 @@ public class TesterContext implements Context {
// NO-OP
}
+ private final Map<String,String> localEncodingMap = new
ConcurrentHashMap<>();
+
@Override
public void addLocaleEncodingMappingParameter(String locale, String
encoding) {
- // NO-OP
+ localEncodingMap.put(locale, encoding);
+ }
+ @Override
+ public String getCharset(Locale locale) {
+ // Match full language_country_variant first, then language_country,
+ // then language only
+ String charset = localEncodingMap.get(locale.toString());
+ if (charset == null) {
+ charset = localEncodingMap.get(locale.getLanguage() + "_" +
locale.getCountry());
+ if (charset == null) {
+ charset = localEncodingMap.get(locale.getLanguage());
+ }
+ }
+ return charset;
}
@Override
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f7fac87..eca9372 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -121,6 +121,13 @@
<code>WriteListener.onWritePossible()</code> and
<code>ReadListener.onDataAvailable()</code>. (markt)
</add>
+ <fix>
+ <bug>64938</bug>: Align the behaviour when <code>null</code> is passed
+ to the <code>ServletResponse</code> methods
+ <code>setCharacterEncoding()</code>, <code>setContentType()</code> and
+ <code>setLocale()</code> with the recent clarification from the Jakarta
+ Servlet project of the expected behaviour in these cases. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
