diff --git a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java index a5a8b07ce5a..e5ff3c2c858 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java @@ -99,6 +99,15 @@ public class ServletWebRequestHttpMethodsTests { assertEquals(dateFormat.format(epochTime), servletResponse.getHeader("Last-Modified")); } + @Test // SPR-14559 + public void checkNotModifiedInvalidIfNoneMatchHeader() { + String eTag = "\"etagvalue\""; + servletRequest.addHeader("If-None-Match", "missingquotes"); + assertFalse(request.checkNotModified(eTag)); + assertEquals(200, servletResponse.getStatus()); + assertEquals(eTag, servletResponse.getHeader("ETag")); + } + @Test public void checkNotModifiedHeaderAlreadySet() { long epochTime = currentDate.getTime(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java index 164f10537a1..ea4f7de7910 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java @@ -223,24 +223,28 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro } private boolean isResourceNotModified(ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) { - List ifNoneMatch = inputMessage.getHeaders().getIfNoneMatch(); - long ifModifiedSince = inputMessage.getHeaders().getIfModifiedSince(); - String eTag = addEtagPadding(outputMessage.getHeaders().getETag()); - long lastModified = outputMessage.getHeaders().getLastModified(); boolean notModified = false; - - if (!ifNoneMatch.isEmpty() && (inputMessage.getHeaders().containsKey(HttpHeaders.IF_UNMODIFIED_SINCE) - || inputMessage.getHeaders().containsKey(HttpHeaders.IF_MATCH))) { - // invalid conditional request, do not process - } - else if (lastModified != -1 && StringUtils.hasLength(eTag)) { - notModified = isETagNotModified(ifNoneMatch, eTag) && isTimeStampNotModified(ifModifiedSince, lastModified); - } - else if (lastModified != -1) { - notModified = isTimeStampNotModified(ifModifiedSince, lastModified); + try { + long ifModifiedSince = inputMessage.getHeaders().getIfModifiedSince(); + String eTag = addEtagPadding(outputMessage.getHeaders().getETag()); + long lastModified = outputMessage.getHeaders().getLastModified(); + List ifNoneMatch = inputMessage.getHeaders().getIfNoneMatch(); + if (!ifNoneMatch.isEmpty() && (inputMessage.getHeaders().containsKey(HttpHeaders.IF_UNMODIFIED_SINCE) + || inputMessage.getHeaders().containsKey(HttpHeaders.IF_MATCH))) { + // invalid conditional request, do not process + } + else if (lastModified != -1 && StringUtils.hasLength(eTag)) { + notModified = isETagNotModified(ifNoneMatch, eTag) && isTimeStampNotModified(ifModifiedSince, lastModified); + } + else if (lastModified != -1) { + notModified = isTimeStampNotModified(ifModifiedSince, lastModified); + } + else if (StringUtils.hasLength(eTag)) { + notModified = isETagNotModified(ifNoneMatch, eTag); + } } - else if (StringUtils.hasLength(eTag)) { - notModified = isETagNotModified(ifNoneMatch, eTag); + catch (IllegalArgumentException exc) { + // invalid conditional request, do not process } return notModified; } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java index 3ccb9dc5082..9b3f1e4071c 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java @@ -379,6 +379,21 @@ public class HttpEntityMethodProcessorMockTests { assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG)); } + @Test // SPR-14559 + public void handleReturnValueEtagInvalidIfNoneMatch() throws Exception { + String etagValue = "\"deadb33f8badf00d\""; + servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, "unquoted"); + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.set(HttpHeaders.ETAG, etagValue); + ResponseEntity returnValue = new ResponseEntity<>("body", responseHeaders, HttpStatus.OK); + + initStringMessageConversion(MediaType.TEXT_PLAIN); + processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); + + assertTrue(mavContainer.isRequestHandled()); + assertEquals(HttpStatus.OK.value(), servletResponse.getStatus()); + } + @Test public void handleReturnValueETagAndLastModified() throws Exception { long currentTime = new Date().getTime();