Bug 61916 - AddDefaultCharsetFilter wouldn't work when setting response header through response.setHeader()
Summary: AddDefaultCharsetFilter wouldn't work when setting response header through re...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.23
Hardware: PC All
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-19 06:34 UTC by Fuwei Chin
Modified: 2018-01-16 22:33 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Fuwei Chin 2017-12-19 06:34:20 UTC
Overview:
AddDefaultCharsetFilter wouldn't work if one set response header 'content-type' through setHeader() instead of setContentType().
I tried to debug the webapp by adding a breakpoint in be beginning of method AddDefaultCharsetFilter.ResponseWrapper#setContentType(String), while the CXF framework did't invoke setContentType().
So I think that cxf may set response header Content-Type by invoking setHeader() but there is not a overridden method setHeader() in class AddDefaultCharsetFilter.ResponseWrapper

Steps to Reproduce:
1) Adding a maven dependecy in pom.xml
<dependency>
	<groupId>org.apache.tomcat</groupId>
	<artifactId>tomcat-catalina</artifactId>
	<version>8.5.23</version>
	<scope>provided</scope>
</dependency>
2) Adding a filter registeration in web.xml
<filter>
	<filter-name>AddDefaultCharset</filter-name>
	<filter-class>org.apache.catalina.filters.AddDefaultCharsetFilter</filter-class>
	<init-param>
		<param-name>encoding</param-name>
		<param-value>UTF-8</param-value>
	</init-param>
</filter>
<filter-mapping>
	<filter-name>AddDefaultCharset</filter-name>
	<servlet-name>cxf</servlet-name>
</filter-mapping>
3) Adding a method in cxf jax-rs service bean UserResource.java
@Path("hello")
@Produces(MediaType.TEXT_PLAIN)
public Response hello(){
	return Response.ok("hello").build();
}
4) request the hello service

Actual Results:
Content-Type: text/plain

Expected Results:
Content-Type: text/plain;charset=UTF-8

Build Date & Hardware:
	
Additional Builds and Platforms:

Additional Information:
Comment 1 Mark Thomas 2018-01-04 17:32:51 UTC
Thanks for the report. I fixed addHeader() as well as setHeader()

Fixed in:
- trunk for 9.0.3 onwards
- 8.5.x for 8.5.25 onwards
- 8.0.x for 8.0.49 onwards
- 7.0.x for 7.0.84 onwards
Comment 2 Konstantin Kolinko 2018-01-16 17:31:20 UTC
Implementation of addHeader() in r1820138 is broken: it needs to call super.addHeader().

This concerns 9.0.3 (currently being voted as a release candidate).


A fix is trivial, but maybe it is worth to extend the unit tests that were created for this issue.
Comment 3 Mark Thomas 2018-01-16 22:33:49 UTC
Regression fixed in:
- trunk for 9.0.4 onwards
- 8.5.x for 8.5.27 onwards
- 8.0.x for 8.0.49 onwards
- 7.0.x for 7.0.84 onwards