root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 8 Packages Projects Releases Wiki Activity

Allow to specify AbstractHttpMessageConverter default charset 

Before this commit, specifying the charset to use with produces or
consumes @RequestMapping attributes resulted in default charset
loss. That was really annoying for JSON for example, where using
UTF-8 charset is mandatory in a lot of use cases.

This commit adds a defaultCharset property to
AbstractHttpMessageConverter in order to avoid losing the
default charset when specifying the charset with these
@RequestMapping attributes.

It changes slightly the default behavior (that's why we have waited
4.3), but it is much more error prone, and will match with most
user's expectations since the charset loss was accidental in most
use cases (users usually just want to limit the media type supported
by a specific handler method).

Issue: SPR-13631
This commit is contained in:
Sebastien Deleuze 2016-02-23 10:27:39 +01:00
parent 61824b1ade
commit c385427397
15 changed files with 114 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
spring-core/src/main/java/org/springframework/util/MimeType.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -22,6 +22,7 @@ import java.util.BitSet;
import java.util.Collections;
import java.util.Comparator;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@ -126,11 +127,28 @@ public class MimeType implements Comparable<MimeType>, Serializable {
* Create a new {@code MimeType} for the given type, subtype, and character set.
* @param type the primary type
* @param subtype the subtype
* @param charSet the character set
* @param charset the character set
* @throws IllegalArgumentException if any of the parameters contains illegal characters
*/
public MimeType(String type, String subtype, Charset charSet) {
this(type, subtype, Collections.singletonMap(PARAM_CHARSET, charSet.name()));
public MimeType(String type, String subtype, Charset charset) {
this(type, subtype, Collections.singletonMap(PARAM_CHARSET, charset.name()));
}
/**
* Copy-constructor that copies the type, subtype, parameters of the given {@code MimeType},
* and allows to set the specified character set.
* @param other the other media type
* @param charset the character set
* @throws IllegalArgumentException if any of the parameters contains illegal characters
*/
public MimeType(MimeType other, Charset charset) {
this(other.getType(), other.getSubtype(), addCharsetParameter(charset, other.getParameters()));
}
private static Map<String, String> addCharsetParameter(Charset charset, Map<String, String> parameters) {
Map<String, String> map = new LinkedHashMap<String, String>(parameters);
map.put(PARAM_CHARSET, charset.name());
return map;
}
/**

14
spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/AsyncTests.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -67,7 +67,7 @@ public class AsyncTests {
this.mockMvc.perform(asyncDispatch(mvcResult))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8_VALUE))
.andExpect(content().string("{\"name\":\"Joe\",\"someDouble\":0.0,\"someBoolean\":false}"));
}
@ -81,7 +81,7 @@ public class AsyncTests {
this.mockMvc.perform(asyncDispatch(mvcResult))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8_VALUE))
.andExpect(content().string("{\"name\":\"Joe\",\"someDouble\":0.0,\"someBoolean\":false}"));
}
@ -94,7 +94,7 @@ public class AsyncTests {
this.mockMvc.perform(asyncDispatch(mvcResult))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8_VALUE))
.andExpect(content().string("{\"name\":\"Joe\",\"someDouble\":0.0,\"someBoolean\":false}"));
}
@ -122,7 +122,7 @@ public class AsyncTests {
this.mockMvc.perform(asyncDispatch(mvcResult))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8_VALUE))
.andExpect(content().string("{\"name\":\"Joe\",\"someDouble\":0.0,\"someBoolean\":false}"));
}
@ -137,7 +137,7 @@ public class AsyncTests {
this.mockMvc.perform(asyncDispatch(mvcResult))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8_VALUE))
.andExpect(content().string("{\"name\":\"Joe\",\"someDouble\":0.0,\"someBoolean\":false}"));
}
@ -161,7 +161,7 @@ public class AsyncTests {
this.mockMvc.perform(asyncDispatch(mvcResult))
.andDo(print(writer))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8_VALUE))
.andExpect(content().string("{\"name\":\"Joe\",\"someDouble\":0.0,\"someBoolean\":false}"));
assertTrue(writer.toString().contains("Async started = false"));

10
spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/resultmatchers/ContentAssertionTests.java
View File

@ -16,8 +16,6 @@
package org.springframework.test.web.servlet.samples.standalone.resultmatchers;
import java.nio.charset.Charset;
import org.junit.Before;
import org.junit.Test;
@ -44,8 +42,6 @@ import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*;
*/
public class ContentAssertionTests {
public static final MediaType TEXT_PLAIN_UTF8 = new MediaType("text", "plain", Charset.forName("UTF-8"));
private MockMvc mockMvc;
@Before
@ -56,8 +52,10 @@ public class ContentAssertionTests {
@Test
public void testContentType() throws Exception {
this.mockMvc.perform(get("/handle").accept(MediaType.TEXT_PLAIN))
.andExpect(content().contentType(MediaType.TEXT_PLAIN))
.andExpect(content().contentType("text/plain"));
.andExpect(content().contentType(MediaType.valueOf("text/plain;charset=ISO-8859-1")))
.andExpect(content().contentType("text/plain;charset=ISO-8859-1"))
.andExpect(content().contentTypeCompatibleWith("text/plain"))
.andExpect(content().contentTypeCompatibleWith(MediaType.TEXT_PLAIN));
this.mockMvc.perform(get("/handleUtf8"))
.andExpect(content().contentType(MediaType.valueOf("text/plain;charset=UTF-8")))

2
spring-test/src/test/java/org/springframework/test/web/servlet/samples/standalone/resultmatchers/XpathAssertionTests.java
View File

@ -158,7 +158,7 @@ public class XpathAssertionTests {
standaloneSetup(new BlogFeedController()).build()
.perform(get("/blog.atom").accept(MediaType.APPLICATION_ATOM_XML))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_ATOM_XML))
.andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_ATOM_XML))
.andExpect(xpath("//feed/title").string("Test Feed"))
.andExpect(xpath("//feed/icon").string("http://www.example.com/favicon.ico"));
}

11
spring-web/src/main/java/org/springframework/http/MediaType.java
View File

@ -278,6 +278,17 @@ public class MediaType extends MimeType implements Serializable {
this(type, subtype, Collections.singletonMap(PARAM_QUALITY_FACTOR, Double.toString(qualityValue)));
}
/**
* Copy-constructor that copies the type, subtype and parameters of the given
* {@code MediaType}, and allows to set the specified character set.
* @param other the other media type
* @param charset the character set
* @throws IllegalArgumentException if any of the parameters contain illegal characters
*/
public MediaType(MediaType other, Charset charset) {
super(other, charset);
}
/**
* Copy-constructor that copies the type and subtype of the given {@code MediaType},
* and allows for different parameter.

24
spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java
View File

@ -18,6 +18,7 @@ package org.springframework.http.converter;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@ -51,6 +52,8 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
private List<MediaType> supportedMediaTypes = Collections.emptyList();
private Charset defaultCharset;
/**
* Construct an {@code AbstractHttpMessageConverter} with no supported media types.
@ -75,6 +78,11 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
setSupportedMediaTypes(Arrays.asList(supportedMediaTypes));
}
protected AbstractHttpMessageConverter(Charset defaultCharset, MediaType... supportedMediaTypes) {
this.defaultCharset = defaultCharset;
setSupportedMediaTypes(Arrays.asList(supportedMediaTypes));
}
/**
* Set the list of {@link MediaType} objects supported by this converter.
@ -89,6 +97,16 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
return Collections.unmodifiableList(this.supportedMediaTypes);
}
/**
* Set the default character set if any.
*/
public void setDefaultCharset(Charset defaultCharset) {
this.defaultCharset = defaultCharset;
}
public Charset getDefaultCharset() {
return defaultCharset;
}
/**
* This implementation checks if the given class is {@linkplain #supports(Class) supported},
@ -200,7 +218,8 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
/**
* Add default headers to the output message.
* <p>This implementation delegates to {@link #getDefaultContentType(Object)} if a content
* type was not provided, calls {@link #getContentLength}, and sets the corresponding headers
* type was not provided, set if necessary the default character set, calls
* {@link #getContentLength}, and sets the corresponding headers.
* @since 4.2
*/
protected void addDefaultHeaders(HttpHeaders headers, T t, MediaType contentType) throws IOException{
@ -214,6 +233,9 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
contentTypeToUse = (mediaType != null ? mediaType : contentTypeToUse);
}
if (contentTypeToUse != null) {
if (contentTypeToUse.getCharSet() == null && this.defaultCharset != null) {
contentTypeToUse = new MediaType(contentTypeToUse, this.defaultCharset);
}
headers.setContentType(contentTypeToUse);
}
}

2
spring-web/src/main/java/org/springframework/http/converter/ObjectToStringHttpMessageConverter.java
View File

@ -74,7 +74,7 @@ public class ObjectToStringHttpMessageConverter extends AbstractHttpMessageConve
* @param defaultCharset the default charset
*/
public ObjectToStringHttpMessageConverter(ConversionService conversionService, Charset defaultCharset) {
super(new MediaType("text", "plain", defaultCharset));
super(defaultCharset, MediaType.TEXT_PLAIN);
Assert.notNull(conversionService, "conversionService is required");
this.conversionService = conversionService;

7
spring-web/src/main/java/org/springframework/http/converter/StringHttpMessageConverter.java
View File

@ -42,8 +42,6 @@ public class StringHttpMessageConverter extends AbstractHttpMessageConverter<Str
public static final Charset DEFAULT_CHARSET = Charset.forName("ISO-8859-1");
private final Charset defaultCharset;
private final List<Charset> availableCharsets;
private boolean writeAcceptCharset = true;
@ -62,8 +60,7 @@ public class StringHttpMessageConverter extends AbstractHttpMessageConverter<Str
* type does not specify one.
*/
public StringHttpMessageConverter(Charset defaultCharset) {
super(new MediaType("text", "plain", defaultCharset), MediaType.ALL);
this.defaultCharset = defaultCharset;
super(defaultCharset, MediaType.TEXT_PLAIN, MediaType.ALL);
this.availableCharsets = new ArrayList<Charset>(Charset.availableCharsets().values());
}
@ -125,7 +122,7 @@ public class StringHttpMessageConverter extends AbstractHttpMessageConverter<Str
return contentType.getCharSet();
}
else {
return this.defaultCharset;
return this.getDefaultCharset();
}
}

3
spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java
View File

@ -71,16 +71,19 @@ public abstract class AbstractJackson2HttpMessageConverter extends AbstractGener
protected AbstractJackson2HttpMessageConverter(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
this.setDefaultCharset(DEFAULT_CHARSET);
}
protected AbstractJackson2HttpMessageConverter(ObjectMapper objectMapper, MediaType supportedMediaType) {
super(supportedMediaType);
this.objectMapper = objectMapper;
this.setDefaultCharset(DEFAULT_CHARSET);
}
protected AbstractJackson2HttpMessageConverter(ObjectMapper objectMapper, MediaType... supportedMediaTypes) {
super(supportedMediaTypes);
this.objectMapper = objectMapper;
this.setDefaultCharset(DEFAULT_CHARSET);
}

5
spring-web/src/main/java/org/springframework/http/converter/json/GsonHttpMessageConverter.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -70,7 +70,8 @@ public class GsonHttpMessageConverter extends AbstractGenericHttpMessageConverte
* Construct a new {@code GsonHttpMessageConverter}.
*/
public GsonHttpMessageConverter() {
super(MediaType.APPLICATION_JSON_UTF8, new MediaType("application", "*+json", DEFAULT_CHARSET));
super(MediaType.APPLICATION_JSON, new MediaType("application", "*+json"));
this.setDefaultCharset(DEFAULT_CHARSET);
}

5
spring-web/src/main/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverter.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -63,8 +63,7 @@ public class MappingJackson2HttpMessageConverter extends AbstractJackson2HttpMes
* @see Jackson2ObjectMapperBuilder#json()
*/
public MappingJackson2HttpMessageConverter(ObjectMapper objectMapper) {
super(objectMapper, MediaType.APPLICATION_JSON_UTF8,
new MediaType("application", "*+json", DEFAULT_CHARSET));
super(objectMapper, MediaType.APPLICATION_JSON, new MediaType("application", "*+json"));
}
/**

8
spring-web/src/main/java/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverter.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -57,9 +57,9 @@ public class MappingJackson2XmlHttpMessageConverter extends AbstractJackson2Http
* @see Jackson2ObjectMapperBuilder#xml()
*/
public MappingJackson2XmlHttpMessageConverter(ObjectMapper objectMapper) {
super(objectMapper, new MediaType("application", "xml", DEFAULT_CHARSET),
new MediaType("text", "xml", DEFAULT_CHARSET),
new MediaType("application", "*+xml", DEFAULT_CHARSET));
super(objectMapper, new MediaType("application", "xml"),
new MediaType("text", "xml"),
new MediaType("application", "*+xml"));
Assert.isAssignable(XmlMapper.class, objectMapper.getClass());
}

4
spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java
View File

@ -1356,7 +1356,7 @@ public class ServletAnnotationControllerTests {
request.addHeader("Accept", "application/json, text/javascript, */*");
MockHttpServletResponse response = new MockHttpServletResponse();
servlet.service(request, response);
assertEquals("Invalid response status code", "application/json", response.getHeader("Content-Type"));
assertEquals("Invalid response status code", "application/json;charset=ISO-8859-1", response.getHeader("Content-Type"));
}
@Test
@ -1770,7 +1770,7 @@ public class ServletAnnotationControllerTests {
servlet.service(request, response);
assertEquals(200, response.getStatus());
assertEquals("application/json", response.getHeader("Content-Type"));
assertEquals("application/json;charset=ISO-8859-1", response.getHeader("Content-Type"));
assertEquals("homeJson", response.getContentAsString());
}

23
spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java
View File

@ -653,6 +653,22 @@ public class RequestResponseBodyMethodProcessorTests {
assertTrue(content.contains("\"name\":\"bar\""));
}
@Test // SPR-13631
public void defaultCharset() throws Exception {
Method method = JacksonController.class.getMethod("defaultCharset");
HandlerMethod handlerMethod = new HandlerMethod(new JacksonController(), method);
MethodParameter methodReturnType = handlerMethod.getReturnType();
List<HttpMessageConverter<?>> converters = new ArrayList<>();
converters.add(new MappingJackson2HttpMessageConverter());
RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters);
Object returnValue = new JacksonController().defaultCharset();
processor.handleReturnValue(returnValue, methodReturnType, this.container, this.request);
assertEquals("UTF-8", this.servletResponse.getCharacterEncoding());
}
private void assertContentDisposition(RequestResponseBodyMethodProcessor processor,
boolean expectContentDisposition, String requestURI, String comment) throws Exception {
@ -921,6 +937,13 @@ public class RequestResponseBodyMethodProcessorTests {
bar.setName("bar");
return Arrays.asList(foo, bar);
}
@RequestMapping(produces = MediaType.APPLICATION_JSON_VALUE)
@ResponseBody
public String defaultCharset() {
return "foo";
}
}
private static class EmptyRequestBodyAdvice implements RequestBodyAdvice {

12
spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java
View File

@ -1011,7 +1011,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
request.addHeader("Accept", "application/json, text/javascript, */*");
MockHttpServletResponse response = new MockHttpServletResponse();
getServlet().service(request, response);
assertEquals("Invalid content-type", "application/json", response.getHeader("Content-Type"));
assertEquals("Invalid content-type", "application/json;charset=ISO-8859-1", response.getHeader("Content-Type"));
}
@Test
@ -1531,7 +1531,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
getServlet().service(request, response);
assertEquals(200, response.getStatus());
assertEquals("application/json", response.getHeader("Content-Type"));
assertEquals("application/json;charset=ISO-8859-1", response.getHeader("Content-Type"));
assertEquals("homeJson", response.getContentAsString());
}
@ -1652,7 +1652,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
getServlet().service(request, response);
assertEquals(200, response.getStatus());
assertEquals("text/html", response.getContentType());
assertEquals("text/html;charset=ISO-8859-1", response.getContentType());
assertEquals("inline;filename=f.txt", response.getHeader("Content-Disposition"));
assertArrayEquals(content, response.getContentAsByteArray());
}
@ -1678,7 +1678,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
getServlet().service(request, response);
assertEquals(200, response.getStatus());
assertEquals("text/html", response.getContentType());
assertEquals("text/html;charset=ISO-8859-1", response.getContentType());
assertNull(response.getHeader("Content-Disposition"));
assertArrayEquals(content, response.getContentAsByteArray());
}
@ -1704,7 +1704,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
getServlet().service(request, response);
assertEquals(200, response.getStatus());
assertEquals("text/html", response.getContentType());
assertEquals("text/html;charset=ISO-8859-1", response.getContentType());
assertNull(response.getHeader("Content-Disposition"));
assertArrayEquals(content, response.getContentAsByteArray());
}
@ -1730,7 +1730,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
getServlet().service(request, response);
assertEquals(200, response.getStatus());
assertEquals("text/css", response.getContentType());
assertEquals("text/css;charset=ISO-8859-1", response.getContentType());
assertNull(response.getHeader("Content-Disposition"));
assertArrayEquals(content, response.getContentAsByteArray());
}