diff --git a/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java b/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java index 8de782ac98..fb7abfeb8e 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -31,6 +31,8 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.util.Assert; import org.springframework.util.DigestUtils; +import org.springframework.util.StringUtils; +import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.util.ContentCachingResponseWrapper; import org.springframework.web.util.WebUtils; @@ -98,7 +100,7 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter { HttpServletResponse responseToUse = response; if (!isAsyncDispatch(request) && !(response instanceof ContentCachingResponseWrapper)) { - responseToUse = new HttpStreamingAwareContentCachingResponseWrapper(response, request); + responseToUse = new ConditionalContentCachingResponseWrapper(response, request); } filterChain.doFilter(request, responseToUse); @@ -109,38 +111,35 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter { } private void updateResponse(HttpServletRequest request, HttpServletResponse response) throws IOException { - ContentCachingResponseWrapper responseWrapper = - WebUtils.getNativeResponse(response, ContentCachingResponseWrapper.class); - Assert.notNull(responseWrapper, "ContentCachingResponseWrapper not found"); - HttpServletResponse rawResponse = (HttpServletResponse) responseWrapper.getResponse(); - int statusCode = responseWrapper.getStatus(); - if (rawResponse.isCommitted()) { - responseWrapper.copyBodyToResponse(); - } - else if (isEligibleForEtag(request, responseWrapper, statusCode, responseWrapper.getContentInputStream())) { - String responseETag = generateETagHeaderValue(responseWrapper.getContentInputStream(), this.writeWeakETag); - rawResponse.setHeader(HttpHeaders.ETAG, responseETag); - String requestETag = request.getHeader(HttpHeaders.IF_NONE_MATCH); - if (requestETag != null && ("*".equals(requestETag) || compareETagHeaderValue(requestETag, responseETag))) { - rawResponse.setStatus(HttpServletResponse.SC_NOT_MODIFIED); + ContentCachingResponseWrapper wrapper = + WebUtils.getNativeResponse(response, ContentCachingResponseWrapper.class); + + Assert.notNull(wrapper, "ContentCachingResponseWrapper not found"); + HttpServletResponse rawResponse = (HttpServletResponse) wrapper.getResponse(); + + if (isEligibleForEtag(request, wrapper, wrapper.getStatus(), wrapper.getContentInputStream())) { + String eTag = wrapper.getHeader(HttpHeaders.ETAG); + if (!StringUtils.hasText(eTag)) { + eTag = generateETagHeaderValue(wrapper.getContentInputStream(), this.writeWeakETag); + rawResponse.setHeader(HttpHeaders.ETAG, eTag); } - else { - responseWrapper.copyBodyToResponse(); + if (new ServletWebRequest(request, rawResponse).checkNotModified(eTag)) { + return; } } - else { - responseWrapper.copyBodyToResponse(); - } + + wrapper.copyBodyToResponse(); } /** - * Indicates whether the given request and response are eligible for ETag generation. - *

The default implementation returns {@code true} if all conditions match: + * Whether an ETag should be calculated for the given request and response + * exchange. By default this is {@code true} if all of the following match: *

* @param request the HTTP request * @param response the HTTP response @@ -151,11 +150,14 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter { protected boolean isEligibleForEtag(HttpServletRequest request, HttpServletResponse response, int responseStatusCode, InputStream inputStream) { - String method = request.getMethod(); - if (responseStatusCode >= 200 && responseStatusCode < 300 && HttpMethod.GET.matches(method)) { + if (!response.isCommitted() && + responseStatusCode >= 200 && responseStatusCode < 300 && + HttpMethod.GET.matches(request.getMethod())) { + String cacheControl = response.getHeader(HttpHeaders.CACHE_CONTROL); return (cacheControl == null || !cacheControl.contains(DIRECTIVE_NO_STORE)); } + return false; } @@ -191,10 +193,12 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter { /** - * This method can be used to disable the content caching response wrapper - * of the ShallowEtagHeaderFilter. This can be done before the start of HTTP - * streaming for example where the response will be written to asynchronously - * and not in the context of a Servlet container thread. + * This method can be used to suppress the content caching response wrapper + * of the ShallowEtagHeaderFilter. The main reason for this is streaming + * scenarios which are not to be cached and do not need an eTag. + *

Note: This method must be called before the response + * is written to in order for the entire response content to be written + * without caching. * @since 4.2 */ public static void disableContentCaching(ServletRequest request) { @@ -207,27 +211,34 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter { } - private static class HttpStreamingAwareContentCachingResponseWrapper extends ContentCachingResponseWrapper { + /** + * Returns the raw OutputStream, instead of the one that does caching, + * if {@link #isContentCachingDisabled}. + */ + private static class ConditionalContentCachingResponseWrapper extends ContentCachingResponseWrapper { private final HttpServletRequest request; - public HttpStreamingAwareContentCachingResponseWrapper(HttpServletResponse response, HttpServletRequest request) { + + ConditionalContentCachingResponseWrapper(HttpServletResponse response, HttpServletRequest request) { super(response); this.request = request; } @Override public ServletOutputStream getOutputStream() throws IOException { - return (useRawResponse() ? getResponse().getOutputStream() : super.getOutputStream()); + return (isContentCachingDisabled(this.request) || hasETag() ? + getResponse().getOutputStream() : super.getOutputStream()); } @Override public PrintWriter getWriter() throws IOException { - return (useRawResponse() ? getResponse().getWriter() : super.getWriter()); + return (isContentCachingDisabled(this.request) || hasETag()? + getResponse().getWriter() : super.getWriter()); } - private boolean useRawResponse() { - return isContentCachingDisabled(this.request); + private boolean hasETag() { + return StringUtils.hasText(getHeader(HttpHeaders.ETAG)); } } diff --git a/spring-web/src/test/java/org/springframework/web/filter/ShallowEtagHeaderFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/ShallowEtagHeaderFilterTests.java index 804a3c5b0f..1dccc8e02a 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/ShallowEtagHeaderFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/ShallowEtagHeaderFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -16,6 +16,8 @@ package org.springframework.web.filter; +import java.nio.charset.StandardCharsets; + import javax.servlet.FilterChain; import javax.servlet.http.HttpServletResponse; @@ -62,7 +64,7 @@ public class ShallowEtagHeaderFilterTests { final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final byte[] responseBody = "Hello World".getBytes("UTF-8"); + final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8); FilterChain filterChain = (filterRequest, filterResponse) -> { assertThat(filterRequest).as("Invalid request passed").isEqualTo(request); ((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK); @@ -71,7 +73,7 @@ public class ShallowEtagHeaderFilterTests { filter.doFilter(request, response, filterChain); assertThat(response.getStatus()).as("Invalid status").isEqualTo(200); - assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); + assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); assertThat(response.getContentLength() > 0).as("Invalid Content-Length header").isTrue(); assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody); } @@ -82,7 +84,7 @@ public class ShallowEtagHeaderFilterTests { final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final byte[] responseBody = "Hello World".getBytes("UTF-8"); + final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8); FilterChain filterChain = (filterRequest, filterResponse) -> { assertThat(filterRequest).as("Invalid request passed").isEqualTo(request); ((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK); @@ -91,7 +93,7 @@ public class ShallowEtagHeaderFilterTests { filter.doFilter(request, response, filterChain); assertThat(response.getStatus()).as("Invalid status").isEqualTo(200); - assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("W/\"0b10a8db164e0754105b7a99be72e3fe5\""); + assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("W/\"0b10a8db164e0754105b7a99be72e3fe5\""); assertThat(response.getContentLength() > 0).as("Invalid Content-Length header").isTrue(); assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody); } @@ -105,14 +107,14 @@ public class ShallowEtagHeaderFilterTests { FilterChain filterChain = (filterRequest, filterResponse) -> { assertThat(filterRequest).as("Invalid request passed").isEqualTo(request); - byte[] responseBody = "Hello World".getBytes("UTF-8"); + byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8); FileCopyUtils.copy(responseBody, filterResponse.getOutputStream()); filterResponse.setContentLength(responseBody.length); }; filter.doFilter(request, response, filterChain); assertThat(response.getStatus()).as("Invalid status").isEqualTo(304); - assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); + assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); assertThat(response.containsHeader("Content-Length")).as("Response has Content-Length header").isFalse(); byte[] expecteds = new byte[0]; assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(expecteds); @@ -127,14 +129,14 @@ public class ShallowEtagHeaderFilterTests { FilterChain filterChain = (filterRequest, filterResponse) -> { assertThat(filterRequest).as("Invalid request passed").isEqualTo(request); - byte[] responseBody = "Hello World".getBytes("UTF-8"); + byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8); FileCopyUtils.copy(responseBody, filterResponse.getOutputStream()); filterResponse.setContentLength(responseBody.length); }; filter.doFilter(request, response, filterChain); assertThat(response.getStatus()).as("Invalid status").isEqualTo(304); - assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); + assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); assertThat(response.containsHeader("Content-Length")).as("Response has Content-Length header").isFalse(); byte[] expecteds = new byte[0]; assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(expecteds); @@ -156,7 +158,7 @@ public class ShallowEtagHeaderFilterTests { filter.doFilter(request, response, filterChain); assertThat(response.getStatus()).as("Invalid status").isEqualTo(304); - assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); + assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); assertThat(response.containsHeader("Content-Length")).as("Response has Content-Length header").isFalse(); byte[] expecteds = new byte[0]; assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(expecteds); @@ -167,7 +169,7 @@ public class ShallowEtagHeaderFilterTests { final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final byte[] responseBody = "Hello World".getBytes("UTF-8"); + final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8); FilterChain filterChain = (filterRequest, filterResponse) -> { assertThat(filterRequest).as("Invalid request passed").isEqualTo(request); ((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK); @@ -187,7 +189,7 @@ public class ShallowEtagHeaderFilterTests { final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final byte[] responseBody = "Hello World".getBytes("UTF-8"); + final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8); FilterChain filterChain = (filterRequest, filterResponse) -> { assertThat(filterRequest).as("Invalid request passed").isEqualTo(request); response.setContentLength(100); @@ -197,7 +199,7 @@ public class ShallowEtagHeaderFilterTests { filter.doFilter(request, response, filterChain); assertThat(response.getStatus()).as("Invalid status").isEqualTo(403); - assertThat(response.getHeader("ETag")).as("Invalid ETag header").isNull(); + assertThat(response.getHeader("ETag")).as("Invalid ETag").isNull(); assertThat(response.getContentLength()).as("Invalid Content-Length header").isEqualTo(100); assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody); } @@ -207,7 +209,7 @@ public class ShallowEtagHeaderFilterTests { final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final byte[] responseBody = "Hello World".getBytes("UTF-8"); + final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8); FilterChain filterChain = (filterRequest, filterResponse) -> { assertThat(filterRequest).as("Invalid request passed").isEqualTo(request); response.setContentLength(100); @@ -217,7 +219,7 @@ public class ShallowEtagHeaderFilterTests { filter.doFilter(request, response, filterChain); assertThat(response.getStatus()).as("Invalid status").isEqualTo(403); - assertThat(response.getHeader("ETag")).as("Invalid ETag header").isNull(); + assertThat(response.getHeader("ETag")).as("Invalid ETag").isNull(); assertThat(response.getContentLength()).as("Invalid Content-Length header").isEqualTo(100); assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody); assertThat(response.getErrorMessage()).as("Invalid error message").isEqualTo("ERROR"); @@ -228,7 +230,7 @@ public class ShallowEtagHeaderFilterTests { final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final byte[] responseBody = "Hello World".getBytes("UTF-8"); + final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8); FilterChain filterChain = (filterRequest, filterResponse) -> { assertThat(filterRequest).as("Invalid request passed").isEqualTo(request); response.setContentLength(100); @@ -238,19 +240,18 @@ public class ShallowEtagHeaderFilterTests { filter.doFilter(request, response, filterChain); assertThat(response.getStatus()).as("Invalid status").isEqualTo(302); - assertThat(response.getHeader("ETag")).as("Invalid ETag header").isNull(); + assertThat(response.getHeader("ETag")).as("Invalid ETag").isNull(); assertThat(response.getContentLength()).as("Invalid Content-Length header").isEqualTo(100); assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody); assertThat(response.getRedirectedUrl()).as("Invalid redirect URL").isEqualTo("https://www.google.com"); } - // SPR-13717 - @Test + @Test // SPR-13717 public void filterFlushResponse() throws Exception { final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final byte[] responseBody = "Hello World".getBytes("UTF-8"); + final byte[] responseBody = "Hello World".getBytes(StandardCharsets.UTF_8); FilterChain filterChain = (filterRequest, filterResponse) -> { assertThat(filterRequest).as("Invalid request passed").isEqualTo(request); ((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK); @@ -260,7 +261,7 @@ public class ShallowEtagHeaderFilterTests { filter.doFilter(request, response, filterChain); assertThat(response.getStatus()).as("Invalid status").isEqualTo(200); - assertThat(response.getHeader("ETag")).as("Invalid ETag header").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); + assertThat(response.getHeader("ETag")).as("Invalid ETag").isEqualTo("\"0b10a8db164e0754105b7a99be72e3fe5\""); assertThat(response.getContentLength() > 0).as("Invalid Content-Length header").isTrue(); assertThat(response.getContentAsByteArray()).as("Invalid content").isEqualTo(responseBody); } 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 38d437db22..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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -21,10 +21,8 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Collections; -import java.util.EnumSet; import java.util.List; import java.util.Map; -import java.util.Set; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -49,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; @@ -70,8 +67,6 @@ import org.springframework.web.servlet.support.RequestContextUtils; */ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodProcessor { - private static final Set SAFE_METHODS = EnumSet.of(HttpMethod.GET, HttpMethod.HEAD); - /** * Basic constructor with converters only. Suitable for resolving * {@code HttpEntity}. For handling {@code ResponseEntity} consider also @@ -205,12 +200,10 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro int returnStatus = ((ResponseEntity) responseEntity).getStatusCodeValue(); outputMessage.getServletResponse().setStatus(returnStatus); if (returnStatus == 200) { - if (SAFE_METHODS.contains(inputMessage.getMethod()) + HttpMethod method = inputMessage.getMethod(); + if ((HttpMethod.GET.equals(method) || HttpMethod.HEAD.equals(method)) && isResourceNotModified(inputMessage, outputMessage)) { - // Ensure headers are flushed, no body should be written. outputMessage.flush(); - ShallowEtagHeaderFilter.disableContentCaching(inputMessage.getServletRequest()); - // Skip call to converters, as they may update the body. 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/HttpEntityMethodProcessorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorTests.java index 10b3188654..1b809cf281 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-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -19,8 +19,13 @@ package org.springframework.web.servlet.mvc.method.annotation; import java.io.Serializable; import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import javax.servlet.FilterChain; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonTypeName; import org.junit.jupiter.api.BeforeEach; @@ -28,6 +33,7 @@ import org.junit.jupiter.api.Test; import org.springframework.core.MethodParameter; import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.ByteArrayHttpMessageConverter; @@ -42,6 +48,7 @@ import org.springframework.web.bind.annotation.ResponseBody; 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.HandlerMethod; import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.testfixture.servlet.MockHttpServletRequest; @@ -200,6 +207,41 @@ public class HttpEntityMethodProcessorTests { assertThat(servletResponse.getContentAsString()).isEqualTo("Foo"); } + @Test // SPR-13423 + public void handleReturnValueWithETagAndETagFilter() throws Exception { + + String eTagValue = "\"deadb33f8badf00d\""; + String content = "body"; + + Method method = getClass().getDeclaredMethod("handle"); + MethodParameter returnType = new MethodParameter(method, -1); + + FilterChain chain = (req, res) -> { + ResponseEntity returnValue = ResponseEntity.ok().eTag(eTagValue).body(content); + try { + ServletWebRequest requestToUse = + new ServletWebRequest((HttpServletRequest) req, (HttpServletResponse) res); + + new HttpEntityMethodProcessor(Collections.singletonList(new StringHttpMessageConverter())) + .handleReturnValue(returnValue, returnType, mavContainer, requestToUse); + + assertThat(this.servletResponse.getContentAsString()) + .as("Response body was cached? It should be written directly to the raw response") + .isEqualTo(content); + } + catch (Exception ex) { + throw new IllegalStateException(ex); + } + }; + + this.servletRequest.setMethod("GET"); + new ShallowEtagHeaderFilter().doFilter(this.servletRequest, this.servletResponse, chain); + + assertThat(this.servletResponse.getStatus()).isEqualTo(200); + assertThat(this.servletResponse.getHeader(HttpHeaders.ETAG)).isEqualTo(eTagValue); + assertThat(this.servletResponse.getContentAsString()).isEqualTo(content); + } + @SuppressWarnings("unused") private void handle(HttpEntity> arg1, HttpEntity arg2) { 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 584289bbad..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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -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; @@ -90,8 +91,10 @@ public class ServletInvocableHandlerMethodTests { ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new Handler(), "responseStatus"); handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); - assertThat(this.mavContainer.isRequestHandled()).as("Null return value + @ResponseStatus should result in 'request handled'").isTrue(); assertThat(this.response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + assertThat(this.mavContainer.isRequestHandled()) + .as("Null return value + @ResponseStatus should result in 'request handled'") + .isTrue(); } @Test @@ -99,8 +102,10 @@ public class ServletInvocableHandlerMethodTests { ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new Handler(), "composedResponseStatus"); handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); - assertThat(this.mavContainer.isRequestHandled()).as("Null return value + @ComposedResponseStatus should result in 'request handled'").isTrue(); assertThat(this.response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + assertThat(this.mavContainer.isRequestHandled()) + .as("Null return value + @ComposedResponseStatus should result in 'request handled'") + .isTrue(); } @Test @@ -120,7 +125,9 @@ public class ServletInvocableHandlerMethodTests { getHandlerMethod(new Handler(), "httpServletResponse", HttpServletResponse.class); handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); - assertThat(this.mavContainer.isRequestHandled()).as("Null return value + HttpServletResponse arg should result in 'request handled'").isTrue(); + assertThat(this.mavContainer.isRequestHandled()) + .as("Null return value + HttpServletResponse arg should result in 'request handled'") + .isTrue(); } @Test @@ -137,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 @@ -159,9 +174,35 @@ public class ServletInvocableHandlerMethodTests { ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new Handler(), "responseStatusWithReason"); handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); - assertThat(this.mavContainer.isRequestHandled()).as("When a status reason w/ used, the request is handled").isTrue(); assertThat(this.response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); assertThat(this.response.getErrorMessage()).isEqualTo("400 Bad Request"); + assertThat(this.mavContainer.isRequestHandled()) + .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 @@ -180,15 +221,15 @@ public class ServletInvocableHandlerMethodTests { this.returnValueHandlers.addHandler(new ViewNameMethodReturnValueHandler()); // Invoke without a request parameter (String return value) - ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new Handler(), "dynamicReturnValue", String.class); - handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); + ServletInvocableHandlerMethod hm = getHandlerMethod(new Handler(), "dynamicReturnValue", String.class); + hm.invokeAndHandle(this.webRequest, this.mavContainer); assertThat(this.mavContainer.getView()).isNotNull(); assertThat(this.mavContainer.getView().getClass()).isEqualTo(RedirectView.class); // Invoke with a request parameter (RedirectView return value) this.request.setParameter("param", "value"); - handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); + hm.invokeAndHandle(this.webRequest, this.mavContainer); assertThat(this.mavContainer.getViewName()).isEqualTo("view"); }