diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractMediaTypeExpression.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractMediaTypeExpression.java index 933a98c5b3..1e36253fea 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractMediaTypeExpression.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractMediaTypeExpression.java @@ -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"); * you may not use this file except in compliance with the License. @@ -18,19 +18,23 @@ package org.springframework.web.servlet.mvc.condition; 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.web.bind.annotation.RequestMapping; /** * Supports media type expressions as described in: * {@link RequestMapping#consumes()} and {@link RequestMapping#produces()}. - * + * * @author Arjen Poutsma * @author Rossen Stoyanchev * @since 3.1 */ abstract class AbstractMediaTypeExpression implements Comparable, MediaTypeExpression { + protected final Log logger = LogFactory.getLog(getClass()); + private final MediaType mediaType; private final boolean isNegated; @@ -60,8 +64,16 @@ abstract class AbstractMediaTypeExpression implements Comparable acceptableMediaTypes = getAcceptableMediaTypes(inputMessage); List producibleMediaTypes = getProducibleMediaTypes(inputMessage.getServletRequest(), returnValueClass); - + Set compatibleMediaTypes = new LinkedHashSet(); for (MediaType a : acceptableMediaTypes) { for (MediaType p : producibleMediaTypes) { @@ -115,10 +115,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe if (compatibleMediaTypes.isEmpty()) { throw new HttpMediaTypeNotAcceptableException(allSupportedMediaTypes); } - + List mediaTypes = new ArrayList(compatibleMediaTypes); MediaType.sortBySpecificity(mediaTypes); - + MediaType selectedMediaType = null; for (MediaType mediaType : mediaTypes) { if (mediaType.isConcrete()) { @@ -130,7 +130,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe break; } } - + if (selectedMediaType != null) { for (HttpMessageConverter messageConverter : messageConverters) { if (messageConverter.canWrite(returnValueClass, selectedMediaType)) { @@ -166,7 +166,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe if (converter.canWrite(returnValueClass, null)) { result.addAll(converter.getSupportedMediaTypes()); } - } + } return result; } else { @@ -175,8 +175,16 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } private List getAcceptableMediaTypes(HttpInputMessage inputMessage) { - List result = inputMessage.getHeaders().getAccept(); - return result.isEmpty() ? Collections.singletonList(MediaType.ALL) : result; + try { + List result = inputMessage.getHeaders().getAccept(); + 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(); + } } /** diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestConditionTests.java index 2f10963f07..f98d2230b6 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestConditionTests.java @@ -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"); * you may not use this file except in compliance with the License. @@ -44,7 +44,7 @@ public class ConsumesRequestConditionTests { assertNotNull(condition.getMatchingCondition(request)); } - + @Test public void negatedConsumesMatch() { ConsumesRequestCondition condition = new ConsumesRequestCondition("!text/plain"); @@ -60,7 +60,7 @@ public class ConsumesRequestConditionTests { ConsumesRequestCondition condition = new ConsumesRequestCondition("!application/xml"); assertEquals(Collections.emptySet(), condition.getConsumableMediaTypes()); } - + @Test public void consumesWildcardMatch() { ConsumesRequestCondition condition = new ConsumesRequestCondition("text/*"); @@ -91,6 +91,26 @@ public class ConsumesRequestConditionTests { 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 public void compareToSingle() { MockHttpServletRequest request = new MockHttpServletRequest(); @@ -128,7 +148,7 @@ public class ConsumesRequestConditionTests { ConsumesRequestCondition result = condition1.combine(condition2); assertEquals(condition2, result); } - + @Test public void combineWithDefault() { ConsumesRequestCondition condition1 = new ConsumesRequestCondition("text/plain"); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java index 4bbff56fd6..437abeb0fd 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java @@ -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"); * you may not use this file except in compliance with the License. @@ -34,7 +34,7 @@ import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition.Pr * @author Rossen Stoyanchev */ public class ProducesRequestConditionTests { - + @Test public void producesMatch() { ProducesRequestCondition condition = new ProducesRequestCondition("text/plain"); @@ -44,7 +44,7 @@ public class ProducesRequestConditionTests { assertNotNull(condition.getMatchingCondition(request)); } - + @Test public void negatedProducesMatch() { ProducesRequestCondition condition = new ProducesRequestCondition("!text/plain"); @@ -60,7 +60,7 @@ public class ProducesRequestConditionTests { ProducesRequestCondition condition = new ProducesRequestCondition("!application/xml"); assertEquals(Collections.emptySet(), condition.getProducibleMediaTypes()); } - + @Test public void producesWildcardMatch() { ProducesRequestCondition condition = new ProducesRequestCondition("text/*"); @@ -91,6 +91,26 @@ public class ProducesRequestConditionTests { 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 public void compareTo() { ProducesRequestCondition html = new ProducesRequestCondition("text/html"); @@ -126,12 +146,12 @@ public class ProducesRequestConditionTests { assertTrue(html.compareTo(xml, request) > 0); assertTrue(xml.compareTo(html, request) < 0); } - + @Test public void compareToWithSingleExpression() { MockHttpServletRequest request = new MockHttpServletRequest(); request.addHeader("Accept", "text/plain"); - + ProducesRequestCondition condition1 = new ProducesRequestCondition("text/plain"); ProducesRequestCondition condition2 = new ProducesRequestCondition("text/*"); @@ -188,7 +208,7 @@ public class ProducesRequestConditionTests { @Test public void compareToMediaTypeAll() { MockHttpServletRequest request = new MockHttpServletRequest(); - + ProducesRequestCondition condition1 = new ProducesRequestCondition(); ProducesRequestCondition condition2 = new ProducesRequestCondition("application/json"); @@ -202,7 +222,7 @@ public class ProducesRequestConditionTests { assertTrue(condition1.compareTo(condition2, request) < 0); assertTrue(condition2.compareTo(condition1, request) > 0); - + request.addHeader("Accept", "*/*"); condition1 = new ProducesRequestCondition(); @@ -255,7 +275,7 @@ public class ProducesRequestConditionTests { ProducesRequestCondition result = condition1.combine(condition2); assertEquals(condition2, result); } - + @Test public void combineWithDefault() { ProducesRequestCondition condition1 = new ProducesRequestCondition("text/plain"); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorTests.java index 1b42aff0ed..6c8e7a7a65 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorTests.java @@ -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"); * you may not use this file except in compliance with the License. @@ -56,7 +56,7 @@ import org.springframework.web.servlet.mvc.method.annotation.HttpEntityMethodPro /** * Test fixture with {@link HttpEntityMethodProcessor} and mock {@link HttpMessageConverter}. - * + * * @author Arjen Poutsma * @author Rossen Stoyanchev */ @@ -106,7 +106,7 @@ public class HttpEntityMethodProcessorTests { returnTypeResponseEntityProduces = new MethodParameter(getClass().getMethod("handle4"), -1); mavContainer = new ModelAndViewContainer(); - + servletRequest = new MockHttpServletRequest(); servletResponse = new MockHttpServletResponse(); webRequest = new ServletWebRequest(servletRequest, servletResponse); @@ -219,7 +219,7 @@ public class HttpEntityMethodProcessorTests { fail("Expected exception"); } - + @Test(expected = HttpMediaTypeNotAcceptableException.class) public void handleReturnValueNotAcceptableProduces() throws Exception { String body = "Foo"; @@ -238,6 +238,17 @@ public class HttpEntityMethodProcessorTests { fail("Expected exception"); } + // SPR-9142 + + @Test(expected=HttpMediaTypeNotAcceptableException.class) + public void handleReturnValueNotAcceptableParseError() throws Exception { + ResponseEntity returnValue = new ResponseEntity("Body", HttpStatus.ACCEPTED); + servletRequest.addHeader("Accept", "01"); + + processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); + fail("Expected exception"); + } + @Test public void responseHeaderNoBody() throws Exception { HttpHeaders headers = new HttpHeaders(); @@ -269,7 +280,7 @@ public class HttpEntityMethodProcessorTests { assertEquals("headerValue", outputMessage.getValue().getHeaders().get("header").get(0)); verify(messageConverter); } - + public ResponseEntity handle1(HttpEntity httpEntity, ResponseEntity responseEntity, int i) { return responseEntity; } diff --git a/src/dist/changelog.txt b/src/dist/changelog.txt index f26431afc7..da7d11a1c3 100644 --- a/src/dist/changelog.txt +++ b/src/dist/changelog.txt @@ -2,6 +2,11 @@ SPRING FRAMEWORK CHANGELOG ========================== 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) -------------------------------------