Skip to content

Commit 9e37020

Browse files
rstoyanchevrstoyanchev
committed
Fix content negotiation issue with sort by q-value
Before this fix the q-value of media types in the Accept header were ignored when using the new RequestMappingHandlerAdapter in combination with @responsebody and HttpMessageConverters. Issue: SPR-9160 Backport-Issue: SPR-9168 Backport-Commit: 982cb2f
1 parent e66bc07 commit 9e37020

File tree

7 files changed

+167
-61
lines changed

7 files changed

+167
-61
lines changed

‎build-spring-framework/resources/changelog.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ Changes in version 3.1.2 (2012-06-??)
1818
* fix issue with parsing invalid Content-Type or Accept headers
1919
* add Jackson 2 HttpMessageConverter and View types
2020
* translate IOException from Jackson to HttpMessageNotReadableException
21+
* fix content negotiation issue when sorting selected media types by quality value
22+
2123

2224
Changes in version 3.1.1 (2012-02-16)
2325
-------------------------------------

‎org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.Collections;
2222
import java.util.LinkedHashSet;
2323
import java.util.List;
24-
import java.util.Map;
2524
import java.util.Set;
2625

2726
import javax.servlet.http.HttpServletRequest;
@@ -117,7 +116,7 @@ protected <T> void writeWithMessageConverters(T returnValue,
117116
}
118117

119118
List<MediaType> mediaTypes = new ArrayList<MediaType>(compatibleMediaTypes);
120-
MediaType.sortBySpecificity(mediaTypes);
119+
MediaType.sortBySpecificityAndQuality(mediaTypes);
121120

122121
MediaType selectedMediaType = null;
123122
for (MediaType mediaType : mediaTypes) {
@@ -131,6 +130,8 @@ else if (mediaType.equals(MediaType.ALL) || mediaType.equals(MEDIA_TYPE_APPLICAT
131130
}
132131
}
133132

133+
selectedMediaType = selectedMediaType.removeQualityValue();
134+
134135
if (selectedMediaType != null) {
135136
for (HttpMessageConverter<?> messageConverter : messageConverters) {
136137
if (messageConverter.canWrite(returnValueClass, selectedMediaType)) {
@@ -188,14 +189,12 @@ private List<MediaType> getAcceptableMediaTypes(HttpInputMessage inputMessage) {
188189
}
189190

190191
/**
191-
* Returns the more specific media type using the q-value of the first media type for both.
192+
* Return the more specific of the acceptable and the producible media types
193+
* with the q-value of the former.
192194
*/
193-
private MediaType getMostSpecificMediaType(MediaType type1, MediaType type2) {
194-
double quality = type1.getQualityValue();
195-
Map<String, String> params = Collections.singletonMap("q", String.valueOf(quality));
196-
MediaType t1 = new MediaType(type1, params);
197-
MediaType t2 = new MediaType(type2, params);
198-
return MediaType.SPECIFICITY_COMPARATOR.compare(t1, t2) <= 0 ? type1 : type2;
195+
private MediaType getMostSpecificMediaType(MediaType acceptType, MediaType produceType) {
196+
produceType = produceType.copyQualityValue(acceptType);
197+
return MediaType.SPECIFICITY_COMPARATOR.compare(acceptType, produceType) < 0 ? acceptType : produceType;
199198
}
200199

201200
}

‎org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -101,6 +101,7 @@
101101
*
102102
* @author Arjen Poutsma
103103
* @author Juergen Hoeller
104+
* @author Rossen Stoyanchev
104105
* @since 3.0
105106
* @see ViewResolver
106107
* @see InternalResourceViewResolver
@@ -117,6 +118,9 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
117118

118119
private static final UrlPathHelper urlPathHelper = new UrlPathHelper();
119120

121+
static {
122+
urlPathHelper.setUrlDecode(false);
123+
}
120124

121125
private int order = Ordered.HIGHEST_PRECEDENCE;
122126

@@ -270,7 +274,7 @@ protected void initServletContext(ServletContext servletContext) {
270274
String name = viewResolvers.get(i).getClass().getName() + i;
271275
getApplicationContext().getAutowireCapableBeanFactory().initializeBean(viewResolvers.get(i), name);
272276
}
273-
277+
274278
}
275279
if (this.viewResolvers.isEmpty()) {
276280
logger.warn("Did not find any ViewResolvers to delegate to; please configure them using the " +
@@ -351,13 +355,13 @@ protected List<MediaType> getMediaTypes(HttpServletRequest request) {
351355
}
352356
}
353357
}
354-
List<MediaType> mediaTypes = new ArrayList<MediaType>(compatibleMediaTypes);
355-
MediaType.sortByQualityValue(mediaTypes);
358+
List<MediaType> selectedMediaTypes = new ArrayList<MediaType>(compatibleMediaTypes);
359+
MediaType.sortBySpecificityAndQuality(selectedMediaTypes);
356360
if (logger.isDebugEnabled()) {
357-
logger.debug("Requested media types are " + mediaTypes + " based on Accept header types " +
361+
logger.debug("Requested media types are " + selectedMediaTypes + " based on Accept header types " +
358362
"and producible media types " + producibleMediaTypes + ")");
359363
}
360-
return mediaTypes;
364+
return selectedMediaTypes;
361365
}
362366
catch (IllegalArgumentException ex) {
363367
if (logger.isDebugEnabled()) {
@@ -392,14 +396,12 @@ private List<MediaType> getProducibleMediaTypes(HttpServletRequest request) {
392396
}
393397

394398
/**
395-
* Returns the more specific media type using the q-value of the first media type for both.
399+
* Return the more specific of the acceptable and the producible media types
400+
* with the q-value of the former.
396401
*/
397-
private MediaType getMostSpecificMediaType(MediaType type1, MediaType type2) {
398-
double quality = type1.getQualityValue();
399-
Map<String, String> params = Collections.singletonMap("q", String.valueOf(quality));
400-
MediaType t1 = new MediaType(type1, params);
401-
MediaType t2 = new MediaType(type2, params);
402-
return MediaType.SPECIFICITY_COMPARATOR.compare(t1, t2) <= 0 ? type1 : type2;
402+
private MediaType getMostSpecificMediaType(MediaType acceptType, MediaType produceType) {
403+
produceType = produceType.copyQualityValue(acceptType);
404+
return MediaType.SPECIFICITY_COMPARATOR.compare(acceptType, produceType) < 0 ? acceptType : produceType;
403405
}
404406

405407
/**

‎org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,25 +48,25 @@
4848
import org.springframework.http.converter.ByteArrayHttpMessageConverter;
4949
import org.springframework.http.converter.HttpMessageConverter;
5050
import org.springframework.http.converter.StringHttpMessageConverter;
51+
import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
5152
import org.springframework.mock.web.MockHttpServletRequest;
5253
import org.springframework.mock.web.MockHttpServletResponse;
5354
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
5455
import org.springframework.web.HttpMediaTypeNotAcceptableException;
5556
import org.springframework.web.HttpMediaTypeNotSupportedException;
57+
import org.springframework.web.bind.MethodArgumentNotValidException;
5658
import org.springframework.web.bind.WebDataBinder;
5759
import org.springframework.web.bind.annotation.RequestBody;
5860
import org.springframework.web.bind.annotation.ResponseBody;
5961
import org.springframework.web.bind.support.WebDataBinderFactory;
6062
import org.springframework.web.context.request.NativeWebRequest;
6163
import org.springframework.web.context.request.ServletWebRequest;
62-
import org.springframework.web.bind.MethodArgumentNotValidException;
6364
import org.springframework.web.method.support.ModelAndViewContainer;
6465
import org.springframework.web.servlet.HandlerMapping;
65-
import org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor;
6666

6767
/**
6868
* Test fixture with {@link RequestResponseBodyMethodProcessor} and mock {@link HttpMessageConverter}.
69-
*
69+
*
7070
* @author Arjen Poutsma
7171
* @author Rossen Stoyanchev
7272
*/
@@ -100,7 +100,7 @@ public void setUp() throws Exception {
100100

101101
processor = new RequestResponseBodyMethodProcessor(Collections.<HttpMessageConverter<?>>singletonList(messageConverter));
102102
reset(messageConverter);
103-
103+
104104
Method handle = getClass().getMethod("handle1", String.class, Integer.TYPE);
105105
paramRequestBodyString = new MethodParameter(handle, 0);
106106
paramInt = new MethodParameter(handle, 1);
@@ -110,7 +110,7 @@ public void setUp() throws Exception {
110110
paramValidBean = new MethodParameter(getClass().getMethod("handle4", SimpleBean.class), 0);
111111

112112
mavContainer = new ModelAndViewContainer();
113-
113+
114114
servletRequest = new MockHttpServletRequest();
115115
servletResponse = new MockHttpServletResponse();
116116
webRequest = new ServletWebRequest(servletRequest, servletResponse);
@@ -175,10 +175,10 @@ private void testResolveArgumentWithValidation(SimpleBean simpleBean) throws IOE
175175

176176
processor = new RequestResponseBodyMethodProcessor(Collections.<HttpMessageConverter<?>>singletonList(beanConverter));
177177
processor.resolveArgument(paramValidBean, mavContainer, webRequest, new ValidatingBinderFactory());
178-
178+
179179
verify(beanConverter);
180180
}
181-
181+
182182
@Test(expected = HttpMediaTypeNotSupportedException.class)
183183
public void resolveArgumentNotReadable() throws Exception {
184184
MediaType contentType = MediaType.TEXT_PLAIN;
@@ -248,7 +248,7 @@ public void handleReturnValueNotAcceptable() throws Exception {
248248

249249
fail("Expected exception");
250250
}
251-
251+
252252
@Test(expected = HttpMediaTypeNotAcceptableException.class)
253253
public void handleReturnValueNotAcceptableProduces() throws Exception {
254254
MediaType accepted = MediaType.TEXT_PLAIN;
@@ -264,19 +264,35 @@ public void handleReturnValueNotAcceptableProduces() throws Exception {
264264
fail("Expected exception");
265265
}
266266

267+
// SPR-9160
268+
267269
@Test
268-
public void handleStringReturnValue() throws Exception {
270+
public void handleReturnValueSortByQuality() throws Exception {
271+
this.servletRequest.addHeader("Accept", "text/plain; q=0.5, application/json");
272+
273+
List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
274+
converters.add(new MappingJackson2HttpMessageConverter());
275+
converters.add(new StringHttpMessageConverter());
276+
RequestResponseBodyMethodProcessor handler = new RequestResponseBodyMethodProcessor(converters);
277+
278+
handler.writeWithMessageConverters("Foo", returnTypeStringProduces, webRequest);
279+
280+
assertEquals("application/json;charset=UTF-8", servletResponse.getHeader("Content-Type"));
281+
}
282+
283+
@Test
284+
public void handleReturnValueString() throws Exception {
269285
List<HttpMessageConverter<?>>converters = new ArrayList<HttpMessageConverter<?>>();
270286
converters.add(new ByteArrayHttpMessageConverter());
271287
converters.add(new StringHttpMessageConverter());
272-
288+
273289
processor = new RequestResponseBodyMethodProcessor(converters);
274290
processor.handleReturnValue("Foo", returnTypeString, mavContainer, webRequest);
275-
291+
276292
assertEquals("text/plain;charset=ISO-8859-1", servletResponse.getHeader("Content-Type"));
277293
assertEquals("Foo", servletResponse.getContentAsString());
278294
}
279-
295+
280296
@ResponseBody
281297
public String handle1(@RequestBody String s, int i) {
282298
return s;
@@ -293,7 +309,7 @@ public String handle3() {
293309

294310
public void handle4(@Valid @RequestBody SimpleBean b) {
295311
}
296-
312+
297313
private final class ValidatingBinderFactory implements WebDataBinderFactory {
298314
public WebDataBinder createBinder(NativeWebRequest webRequest, Object target, String objectName) throws Exception {
299315
LocalValidatorFactoryBean validator = new LocalValidatorFactoryBean();
@@ -318,5 +334,5 @@ public String getName() {
318334
return name;
319335
}
320336
}
321-
337+
322338
}

0 commit comments

Comments
 (0)