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 ee7c53ac64..630a8c123e 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 @@ -172,7 +172,7 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro Object body = responseEntity.getBody(); if (responseEntity instanceof ResponseEntity) { outputMessage.setStatusCode(((ResponseEntity) responseEntity).getStatusCode()); - if (inputMessage.getMethod() == HttpMethod.GET && + if (inputMessage.getMethod().equals(HttpMethod.GET) && isResourceNotModified(inputMessage, outputMessage)) { outputMessage.setStatusCode(HttpStatus.NOT_MODIFIED); // Ensure headers are flushed, no body should be written. @@ -196,7 +196,11 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro long lastModified = outputMessage.getHeaders().getLastModified(); boolean notModified = false; - if (lastModified != -1 && StringUtils.hasLength(eTag)) { + 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) { @@ -213,8 +217,7 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro for (String clientETag : ifNoneMatch) { // Compare weak/strong ETags as per https://tools.ietf.org/html/rfc7232#section-2.3 if (StringUtils.hasLength(clientETag) && - (clientETag.replaceFirst("^W/", "").equals(etag.replaceFirst("^W/", "")) || - clientETag.equals("*"))) { + (clientETag.replaceFirst("^W/", "").equals(etag.replaceFirst("^W/", "")))) { return true; } } 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 3669429edc..6387365ca4 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 @@ -349,11 +349,9 @@ public class HttpEntityMethodProcessorMockTests { processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); - assertTrue(mavContainer.isRequestHandled()); - assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus()); + assertResponseNotModified(); assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size()); assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED)); - assertEquals(0, servletResponse.getContentAsByteArray().length); } @Test @@ -370,11 +368,9 @@ public class HttpEntityMethodProcessorMockTests { processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); - assertTrue(mavContainer.isRequestHandled()); - assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus()); + assertResponseNotModified(); assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size()); assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG)); - assertEquals(0, servletResponse.getContentAsByteArray().length); } @Test @@ -395,13 +391,11 @@ public class HttpEntityMethodProcessorMockTests { processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); - assertTrue(mavContainer.isRequestHandled()); - assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus()); + assertResponseNotModified(); assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size()); assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED)); assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size()); assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG)); - assertEquals(0, servletResponse.getContentAsByteArray().length); } @Test @@ -420,12 +414,16 @@ public class HttpEntityMethodProcessorMockTests { processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); - assertTrue(mavContainer.isRequestHandled()); - assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus()); + assertResponseNotModified(); assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.LAST_MODIFIED).size()); assertEquals(dateFormat.format(oneMinuteAgo), servletResponse.getHeader(HttpHeaders.LAST_MODIFIED)); assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size()); assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG)); + } + + private void assertResponseNotModified() { + assertTrue(mavContainer.isRequestHandled()); + assertEquals(HttpStatus.NOT_MODIFIED.value(), servletResponse.getStatus()); assertEquals(0, servletResponse.getContentAsByteArray().length); } @@ -472,15 +470,81 @@ public class HttpEntityMethodProcessorMockTests { given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN)); given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true); + processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); + + assertResponseOkWithBody("body"); + assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size()); + assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG)); + } + + // SPR-13626 + @Test + public void handleReturnTypeGetIfNoneMatchWildcard() throws Exception { + String wildcardValue = "*"; + String etagValue = "\"some-etag\""; + servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, wildcardValue); + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.set(HttpHeaders.ETAG, etagValue); + ResponseEntity returnValue = new ResponseEntity("body", responseHeaders, HttpStatus.OK); + + given(messageConverter.canWrite(String.class, null)).willReturn(true); + given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN)); + given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true); processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); - assertTrue(mavContainer.isRequestHandled()); - assertEquals(HttpStatus.OK.value(), servletResponse.getStatus()); + assertResponseOkWithBody("body"); assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size()); assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG)); + } + + // SPR-13626 + @Test + public void handleReturnTypeIfNoneMatchIfMatch() throws Exception { + String etagValue = "\"some-etag\""; + servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue); + servletRequest.addHeader(HttpHeaders.IF_MATCH, "ifmatch"); + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.set(HttpHeaders.ETAG, etagValue); + ResponseEntity returnValue = new ResponseEntity("body", responseHeaders, HttpStatus.OK); + + given(messageConverter.canWrite(String.class, null)).willReturn(true); + given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN)); + given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true); + + processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); + + assertResponseOkWithBody("body"); + assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size()); + assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG)); + } + + // SPR-13626 + @Test + public void handleReturnTypeIfNoneMatchIfUnmodifiedSince() throws Exception { + String etagValue = "\"some-etag\""; + servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, etagValue); + servletRequest.addHeader(HttpHeaders.IF_UNMODIFIED_SINCE, dateFormat.format(new Date().getTime())); + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.set(HttpHeaders.ETAG, etagValue); + ResponseEntity returnValue = new ResponseEntity("body", responseHeaders, HttpStatus.OK); + + given(messageConverter.canWrite(String.class, null)).willReturn(true); + given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN)); + given(messageConverter.canWrite(String.class, MediaType.TEXT_PLAIN)).willReturn(true); + + processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); + + assertResponseOkWithBody("body"); + assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size()); + assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG)); + } + + private void assertResponseOkWithBody(String body) throws Exception { + assertTrue(mavContainer.isRequestHandled()); + assertEquals(HttpStatus.OK.value(), servletResponse.getStatus()); ArgumentCaptor outputMessage = ArgumentCaptor.forClass(HttpOutputMessage.class); - verify(messageConverter).write(eq("body"), eq(MediaType.TEXT_PLAIN), outputMessage.capture()); + verify(messageConverter).write(eq("body"), eq(MediaType.TEXT_PLAIN), outputMessage.capture()); } @SuppressWarnings("unused")