root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Remove unnecessary calls to disableContentCaching 

These calls were added in error when trying to fix #22797 and #23775.
They are not needed in 304 scenarios. Those have no response content and
are skipped by ShallowETagHeaderFilter based on the status.

This leaves disableContentCaching invoked only in streaming scenarios,
which was the original intent and should be the only reason for that
method.

See gh-24635
This commit is contained in:
Rossen Stoyanchev 2020-03-04 17:20:13 +00:00
parent d3da7a50ec
commit c7e037da39
4 changed files with 70 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java
View File

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

2
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java
View File

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

25
spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java
View File

@ -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<String> 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\"";

56
spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java
View File

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