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 8624c2e7ba..6b9f0af707 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 @@ -47,7 +47,6 @@ import org.springframework.web.accept.ContentNegotiationManager; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.ServletWebRequest; -import org.springframework.web.filter.ShallowEtagHeaderFilter; import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.servlet.mvc.support.RedirectAttributes; import org.springframework.web.servlet.support.RequestContextUtils; @@ -205,7 +204,6 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro if ((HttpMethod.GET.equals(method) || HttpMethod.HEAD.equals(method)) && isResourceNotModified(inputMessage, outputMessage)) { outputMessage.flush(); - ShallowEtagHeaderFilter.disableContentCaching(inputMessage.getServletRequest()); return; } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java index 9d574bef38..12ab262310 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java @@ -36,7 +36,6 @@ import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.context.request.ServletWebRequest; -import org.springframework.web.filter.ShallowEtagHeaderFilter; import org.springframework.web.method.HandlerMethod; import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite; @@ -172,7 +171,6 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod { if (StringUtils.hasText(response.getHeader(HttpHeaders.ETAG))) { HttpServletRequest request = webRequest.getNativeRequest(HttpServletRequest.class); Assert.notNull(request, "Expected HttpServletRequest"); - ShallowEtagHeaderFilter.disableContentCaching(request); } } } 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 dde74a7ceb..574536c781 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 @@ -30,6 +30,8 @@ import java.util.Collections; import java.util.Date; import java.util.Set; +import javax.servlet.FilterChain; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -53,6 +55,7 @@ import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.HttpMediaTypeNotSupportedException; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.filter.ShallowEtagHeaderFilter; import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.testfixture.servlet.MockHttpServletRequest; import org.springframework.web.testfixture.servlet.MockHttpServletResponse; @@ -429,6 +432,28 @@ public class HttpEntityMethodProcessorMockTests { assertConditionalResponse(HttpStatus.NOT_MODIFIED, null, etagValue, -1); } + @Test + public void handleEtagWithHttp304AndEtagFilterHasNoImpact() throws Exception { + + String eTagValue = "\"deadb33f8badf00d\""; + + FilterChain chain = (req, res) -> { + servletRequest.addHeader(HttpHeaders.IF_NONE_MATCH, eTagValue); + ResponseEntity returnValue = ResponseEntity.ok().eTag(eTagValue).body("body"); + initStringMessageConversion(TEXT_PLAIN); + try { + processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); + } + catch (Exception ex) { + throw new IllegalStateException(ex); + } + }; + + new ShallowEtagHeaderFilter().doFilter(this.servletRequest, this.servletResponse, chain); + + assertConditionalResponse(HttpStatus.NOT_MODIFIED, null, eTagValue, -1); + } + @Test // SPR-14559 public void shouldHandleInvalidIfNoneMatchWithHttp200() throws Exception { String etagValue = "\"deadb33f8badf00d\""; diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java index 88e3deb28b..c20cedc9a2 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java @@ -23,6 +23,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import javax.servlet.FilterChain; import javax.servlet.http.HttpServletResponse; import org.junit.jupiter.api.Test; @@ -143,21 +144,29 @@ public class ServletInvocableHandlerMethodTests { .isTrue(); } - @Test // gh-23775 + @Test public void invokeAndHandle_VoidNotModifiedWithEtag() throws Exception { - String etag = "\"deadb33f8badf00d\""; - this.request.addHeader(HttpHeaders.IF_NONE_MATCH, etag); - this.webRequest.checkNotModified(etag); - ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new Handler(), "notModified"); - handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); + String eTagValue = "\"deadb33f8badf00d\""; - assertThat(this.mavContainer.isRequestHandled()) - .as("Null return value + 'not modified' request should result in 'request handled'") - .isTrue(); + FilterChain chain = (req, res) -> { + request.addHeader(HttpHeaders.IF_NONE_MATCH, eTagValue); + webRequest.checkNotModified(eTagValue); - assertThat(this.request.getAttribute(ShallowEtagHeaderFilter.class.getName() + ".STREAMING")) - .isEqualTo(true); + try { + ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new Handler(), "notModified"); + handlerMethod.invokeAndHandle(webRequest, mavContainer); + } + catch (Exception ex) { + throw new IllegalStateException(ex); + } + }; + + new ShallowEtagHeaderFilter().doFilter(this.request, this.response, chain); + + assertThat(response.getStatus()).isEqualTo(304); + assertThat(response.getHeader(HttpHeaders.ETAG)).isEqualTo(eTagValue); + assertThat(response.getContentAsString()).isEmpty(); } @Test // SPR-9159 @@ -171,6 +180,31 @@ public class ServletInvocableHandlerMethodTests { .as("When a status reason w/ used, the request is handled").isTrue(); } + @Test // gh-23775, gh-24635 + public void invokeAndHandle_ETagFilterHasNoImpactWhenETagPresent() throws Exception { + + String eTagValue = "\"deadb33f8badf00d\""; + + FilterChain chain = (req, res) -> { + request.addHeader(HttpHeaders.IF_NONE_MATCH, eTagValue); + webRequest.checkNotModified(eTagValue); + + try { + ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new Handler(), "notModified"); + handlerMethod.invokeAndHandle(webRequest, mavContainer); + } + catch (Exception ex) { + throw new IllegalStateException(ex); + } + }; + + new ShallowEtagHeaderFilter().doFilter(this.request, this.response, chain); + + assertThat(this.response.getStatus()).isEqualTo(304); + assertThat(this.response.getHeader(HttpHeaders.ETAG)).isEqualTo(eTagValue); + assertThat(this.response.getContentAsString()).isEmpty(); + } + @Test public void invokeAndHandle_Exception() throws Exception { this.returnValueHandlers.addHandler(new ExceptionRaisingReturnValueHandler());