Fix issue with parsing media types
Invalid Content-Type or Accept header values previously resulted in the IllegalArgumentException getting propagated. After this change such errors are detected and generally treated as a "no match", which may for example result in a 406 in the case of the Accept header. Issue: SPR-9148
This commit is contained in:
parent
0b02933938
commit
ca8b98e947
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2011 the original author or authors.
|
* Copyright 2002-2012 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -18,6 +18,8 @@ package org.springframework.web.servlet.mvc.condition;
|
||||||
|
|
||||||
import javax.servlet.http.HttpServletRequest;
|
import javax.servlet.http.HttpServletRequest;
|
||||||
|
|
||||||
|
import org.apache.commons.logging.Log;
|
||||||
|
import org.apache.commons.logging.LogFactory;
|
||||||
import org.springframework.http.MediaType;
|
import org.springframework.http.MediaType;
|
||||||
import org.springframework.web.bind.annotation.RequestMapping;
|
import org.springframework.web.bind.annotation.RequestMapping;
|
||||||
|
|
||||||
|
@ -31,6 +33,8 @@ import org.springframework.web.bind.annotation.RequestMapping;
|
||||||
*/
|
*/
|
||||||
abstract class AbstractMediaTypeExpression implements Comparable<AbstractMediaTypeExpression>, MediaTypeExpression {
|
abstract class AbstractMediaTypeExpression implements Comparable<AbstractMediaTypeExpression>, MediaTypeExpression {
|
||||||
|
|
||||||
|
protected final Log logger = LogFactory.getLog(getClass());
|
||||||
|
|
||||||
private final MediaType mediaType;
|
private final MediaType mediaType;
|
||||||
|
|
||||||
private final boolean isNegated;
|
private final boolean isNegated;
|
||||||
|
@ -60,9 +64,17 @@ abstract class AbstractMediaTypeExpression implements Comparable<AbstractMediaTy
|
||||||
}
|
}
|
||||||
|
|
||||||
public final boolean match(HttpServletRequest request) {
|
public final boolean match(HttpServletRequest request) {
|
||||||
|
try {
|
||||||
boolean match = matchMediaType(request);
|
boolean match = matchMediaType(request);
|
||||||
return !isNegated ? match : !match;
|
return !isNegated ? match : !match;
|
||||||
}
|
}
|
||||||
|
catch (IllegalArgumentException ex) {
|
||||||
|
if (logger.isDebugEnabled()) {
|
||||||
|
logger.debug("Could not parse media type header: " + ex.getMessage());
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
protected abstract boolean matchMediaType(HttpServletRequest request);
|
protected abstract boolean matchMediaType(HttpServletRequest request);
|
||||||
|
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2011 the original author or authors.
|
* Copyright 2002-2012 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -175,9 +175,17 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
|
||||||
}
|
}
|
||||||
|
|
||||||
private List<MediaType> getAcceptableMediaTypes(HttpInputMessage inputMessage) {
|
private List<MediaType> getAcceptableMediaTypes(HttpInputMessage inputMessage) {
|
||||||
|
try {
|
||||||
List<MediaType> result = inputMessage.getHeaders().getAccept();
|
List<MediaType> result = inputMessage.getHeaders().getAccept();
|
||||||
return result.isEmpty() ? Collections.singletonList(MediaType.ALL) : result;
|
return result.isEmpty() ? Collections.singletonList(MediaType.ALL) : result;
|
||||||
}
|
}
|
||||||
|
catch (IllegalArgumentException ex) {
|
||||||
|
if (logger.isDebugEnabled()) {
|
||||||
|
logger.debug("Could not parse Accept header: " + ex.getMessage());
|
||||||
|
}
|
||||||
|
return Collections.emptyList();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the more specific media type using the q-value of the first media type for both.
|
* Returns the more specific media type using the q-value of the first media type for both.
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2011 the original author or authors.
|
* Copyright 2002-2012 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -91,6 +91,26 @@ public class ConsumesRequestConditionTests {
|
||||||
assertNull(condition.getMatchingCondition(request));
|
assertNull(condition.getMatchingCondition(request));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void consumesParseError() {
|
||||||
|
ConsumesRequestCondition condition = new ConsumesRequestCondition("text/plain");
|
||||||
|
|
||||||
|
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||||
|
request.setContentType("01");
|
||||||
|
|
||||||
|
assertNull(condition.getMatchingCondition(request));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void consumesParseErrorWithNegation() {
|
||||||
|
ConsumesRequestCondition condition = new ConsumesRequestCondition("!text/plain");
|
||||||
|
|
||||||
|
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||||
|
request.setContentType("01");
|
||||||
|
|
||||||
|
assertNull(condition.getMatchingCondition(request));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void compareToSingle() {
|
public void compareToSingle() {
|
||||||
MockHttpServletRequest request = new MockHttpServletRequest();
|
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2011 the original author or authors.
|
* Copyright 2002-2012 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -91,6 +91,26 @@ public class ProducesRequestConditionTests {
|
||||||
assertNull(condition.getMatchingCondition(request));
|
assertNull(condition.getMatchingCondition(request));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void producesParseError() {
|
||||||
|
ProducesRequestCondition condition = new ProducesRequestCondition("text/plain");
|
||||||
|
|
||||||
|
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||||
|
request.addHeader("Accept", "bogus");
|
||||||
|
|
||||||
|
assertNull(condition.getMatchingCondition(request));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void producesParseErrorWithNegation() {
|
||||||
|
ProducesRequestCondition condition = new ProducesRequestCondition("!text/plain");
|
||||||
|
|
||||||
|
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||||
|
request.addHeader("Accept", "bogus");
|
||||||
|
|
||||||
|
assertNull(condition.getMatchingCondition(request));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void compareTo() {
|
public void compareTo() {
|
||||||
ProducesRequestCondition html = new ProducesRequestCondition("text/html");
|
ProducesRequestCondition html = new ProducesRequestCondition("text/html");
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2011 the original author or authors.
|
* Copyright 2002-2012 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -238,6 +238,17 @@ public class HttpEntityMethodProcessorTests {
|
||||||
fail("Expected exception");
|
fail("Expected exception");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SPR-9142
|
||||||
|
|
||||||
|
@Test(expected=HttpMediaTypeNotAcceptableException.class)
|
||||||
|
public void handleReturnValueNotAcceptableParseError() throws Exception {
|
||||||
|
ResponseEntity<String> returnValue = new ResponseEntity<String>("Body", HttpStatus.ACCEPTED);
|
||||||
|
servletRequest.addHeader("Accept", "01");
|
||||||
|
|
||||||
|
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
|
||||||
|
fail("Expected exception");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void responseHeaderNoBody() throws Exception {
|
public void responseHeaderNoBody() throws Exception {
|
||||||
HttpHeaders headers = new HttpHeaders();
|
HttpHeaders headers = new HttpHeaders();
|
||||||
|
|
|
@ -2,6 +2,11 @@ SPRING FRAMEWORK CHANGELOG
|
||||||
==========================
|
==========================
|
||||||
http://www.springsource.org
|
http://www.springsource.org
|
||||||
|
|
||||||
|
Changes in version 3.2 M1
|
||||||
|
-------------------------------------
|
||||||
|
|
||||||
|
* fix issue with parsing invalid Content-Type or Accept headers
|
||||||
|
|
||||||
|
|
||||||
Changes in version 3.1.1 (2012-02-16)
|
Changes in version 3.1.1 (2012-02-16)
|
||||||
-------------------------------------
|
-------------------------------------
|
||||||
|
|
Loading…
Reference in New Issue