Polishing and minor refactoring

See gh-24635
This commit is contained in:
Rossen Stoyanchev 2020-03-04 18:59:18 +00:00
parent e7df445e37
commit d3da7a50ec
4 changed files with 81 additions and 74 deletions

View File

@ -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.
* <p>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:
* <ul>
* <li>response status codes in the {@code 2xx} series</li>
* <li>request method is a GET</li>
* <li>response Cache-Control header is not set or does not contain a "no-store" directive</li>
* <li>Response is not committed.</li>
* <li>Response status codes is in the {@code 2xx} series.</li>
* <li>Request method is a GET.</li>
* <li>Response Cache-Control header does not contain "no-store" (or is not present at all).</li>
* </ul>
* @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.
* <p><strong>Note:</strong> 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());
}
}

View File

@ -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);
}

View File

@ -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<HttpMethod> 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;
}
}

View File

@ -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");
}