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..3007318e94 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. @@ -98,7 +98,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 +109,34 @@ 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); + 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 responseETag = generateETagHeaderValue(wrapper.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); - } - else { - responseWrapper.copyBodyToResponse(); + 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 +147,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 +190,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 +208,30 @@ 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) ? + getResponse().getOutputStream() : super.getOutputStream()); } @Override public PrintWriter getWriter() throws IOException { - return (useRawResponse() ? getResponse().getWriter() : super.getWriter()); - } - - private boolean useRawResponse() { - return isContentCachingDisabled(this.request); + return (isContentCachingDisabled(this.request) ? + getResponse().getWriter() : super.getWriter()); } } 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..8624c2e7ba 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; @@ -70,8 +68,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 +201,11 @@ 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/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..88e3deb28b 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. @@ -90,8 +90,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 +101,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 +124,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 @@ -159,9 +165,10 @@ 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 @@ -180,15 +187,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"); }