GitHub-Spring
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Align MVC checkNotModified with reactive support 

Since SPR-14522, the web reactive framework supports checkNotModified
features. This commit aligns the existing MVC infrastructure with
web reactive's behavior.

Because of the new Servlet 3.0 baseline, some constraints
aren't relevant anymore and duplicate code has been removed in
`HttpEntityMethodProcessor`.

Issue: SPR-14659
pull/1161/head
Brian Clozel 9 years ago
parent
72e4fac28d
commit
cc5300c4d5
4 changed files with 162 additions and 214 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 280
      spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java
  2. 10
      spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java
  3. 79
      spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java
  4. 7
      spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java

280
spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java
Unescape View File

@ -17,12 +17,18 @@ @@ -17,12 +17,18 @@
package org.springframework.web.context.request;
import java.security.Principal;
import java.util.Date;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Arrays;
import java.util.Enumeration;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.TimeZone;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
@ -44,25 +50,17 @@ import org.springframework.web.util.WebUtils; @@ -44,25 +50,17 @@ import org.springframework.web.util.WebUtils;
*/
public class ServletWebRequest extends ServletRequestAttributes implements NativeWebRequest {
private static final String HEADER_ETAG = "ETag";
private static final String HEADER_IF_MODIFIED_SINCE = "If-Modified-Since";
private static final String HEADER_IF_UNMODIFIED_SINCE = "If-Unmodified-Since";
private static final String HEADER_IF_NONE_MATCH = "If-None-Match";
private static final String HEADER_LAST_MODIFIED = "Last-Modified";
private static final String ETAG = "ETag";
private static final String METHOD_GET = "GET";
private static final String IF_MODIFIED_SINCE = "If-Modified-Since";
private static final String METHOD_HEAD = "HEAD";
private static final String IF_UNMODIFIED_SINCE = "If-Unmodified-Since";
private static final String METHOD_POST = "POST";
private static final String IF_NONE_MATCH = "If-None-Match";
private static final String METHOD_PUT = "PUT";
private static final String LAST_MODIFIED = "Last-Modified";
private static final String METHOD_DELETE = "DELETE";
private static final List<String> SAFE_METHODS = Arrays.asList("GET", "HEAD");
/**
* Pattern matching ETag multiple field values in headers such as "If-Match", "If-None-Match"
@ -70,6 +68,17 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ @@ -70,6 +68,17 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
*/
private static final Pattern ETAG_HEADER_VALUE_PATTERN = Pattern.compile("\\*|\\s*((W\\/)?(\"[^\"]*\"))\\s*,?");
/**
* Date formats as specified in the HTTP RFC
* @see <a href="https://tools.ietf.org/html/rfc7231#section-7.1.1.1">Section 7.1.1.1 of RFC 7231</a>
*/
private static final String[] DATE_FORMATS = new String[] {
"EEE, dd MMM yyyy HH:mm:ss zzz",
"EEE, dd-MMM-yy HH:mm:ss zzz",
"EEE MMM dd HH:mm:ss yyyy"
};
private static TimeZone GMT = TimeZone.getTimeZone("GMT");
private boolean notModified = false;
@ -189,141 +198,128 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ @@ -189,141 +198,128 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
@Override
public boolean checkNotModified(long lastModifiedTimestamp) {
HttpServletResponse response = getResponse();
if (lastModifiedTimestamp >= 0 && !this.notModified) {
if (isCompatibleWithConditionalRequests(response)) {
this.notModified = isTimestampNotModified(lastModifiedTimestamp);
if (response != null) {
if (supportsNotModifiedStatus()) {
if (this.notModified) {
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
}
if (isHeaderAbsent(response, HEADER_LAST_MODIFIED)) {
response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
}
}
else if (supportsConditionalUpdate()) {
if (this.notModified) {
response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED);
}
}
}
}
}
return this.notModified;
return checkNotModified(null, lastModifiedTimestamp);
}
@Override
public boolean checkNotModified(String etag) {
HttpServletResponse response = getResponse();
if (StringUtils.hasLength(etag) && !this.notModified) {
if (isCompatibleWithConditionalRequests(response)) {
etag = addEtagPadding(etag);
if (hasRequestHeader(HEADER_IF_NONE_MATCH)) {
this.notModified = isEtagNotModified(etag);
}
if (response != null) {
if (this.notModified && supportsNotModifiedStatus()) {
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
}
if (isHeaderAbsent(response, HEADER_ETAG)) {
response.setHeader(HEADER_ETAG, etag);
}
}
}
}
return this.notModified;
return checkNotModified(etag, -1);
}
@Override
public boolean checkNotModified(String etag, long lastModifiedTimestamp) {
HttpServletResponse response = getResponse();
if (StringUtils.hasLength(etag) && !this.notModified) {
if (isCompatibleWithConditionalRequests(response)) {
etag = addEtagPadding(etag);
if (hasRequestHeader(HEADER_IF_NONE_MATCH)) {
this.notModified = isEtagNotModified(etag);
}
else if (hasRequestHeader(HEADER_IF_MODIFIED_SINCE)) {
this.notModified = isTimestampNotModified(lastModifiedTimestamp);
}
if (response != null) {
if (supportsNotModifiedStatus()) {
if (this.notModified) {
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
}
if (isHeaderAbsent(response, HEADER_ETAG)) {
response.setHeader(HEADER_ETAG, etag);
}
if (isHeaderAbsent(response, HEADER_LAST_MODIFIED)) {
response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
}
}
else if (supportsConditionalUpdate()) {
if (this.notModified) {
response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED);
}
}
}
}
if (this.notModified || HttpStatus.OK.value() != response.getStatus()) {
return this.notModified;
}
return this.notModified;
}
public boolean isNotModified() {
return this.notModified;
}
// Evaluate conditions in order of precedence.
// See https://tools.ietf.org/html/rfc7232#section-6
private boolean isCompatibleWithConditionalRequests(HttpServletResponse response) {
try {
if (response == null) {
// Can't check response.getStatus() - let's assume we're good
return true;
if (validateIfUnmodifiedSince(lastModifiedTimestamp)) {
if (this.notModified) {
response.setStatus(HttpStatus.PRECONDITION_FAILED.value());
}
return HttpStatus.valueOf(response.getStatus()).is2xxSuccessful();
return this.notModified;
}
catch (IllegalArgumentException ex) {
return true;
boolean validated = validateIfNoneMatch(etag);
if (!validated) {
validateIfModifiedSince(lastModifiedTimestamp);
}
}
private boolean isHeaderAbsent(HttpServletResponse response, String header) {
if (response == null) {
// Can't check response.getHeader(header) - let's assume it's not set
return true;
// Update response
boolean isHttpGetOrHead = SAFE_METHODS.contains(getRequest().getMethod());
if (this.notModified) {
response.setStatus(isHttpGetOrHead ?
HttpStatus.NOT_MODIFIED.value() : HttpStatus.PRECONDITION_FAILED.value());
}
return (response.getHeader(header) == null);
if (isHttpGetOrHead) {
if(lastModifiedTimestamp > 0 && parseDateValue(response.getHeader(LAST_MODIFIED)) == -1) {
response.setDateHeader(LAST_MODIFIED, lastModifiedTimestamp);
}
if (StringUtils.hasLength(etag) && response.getHeader(ETAG) == null) {
response.setHeader(ETAG, padEtagIfNecessary(etag));
}
}
return this.notModified;
}
private boolean hasRequestHeader(String headerName) {
return StringUtils.hasLength(getHeader(headerName));
private boolean validateIfUnmodifiedSince(long lastModifiedTimestamp) {
if (lastModifiedTimestamp < 0) {
return false;
}
long ifUnmodifiedSince = parseDateHeader(IF_UNMODIFIED_SINCE);
if (ifUnmodifiedSince == -1) {
return false;
}
// We will perform this validation...
this.notModified = (ifUnmodifiedSince < (lastModifiedTimestamp / 1000 * 1000));
return true;
}
private boolean supportsNotModifiedStatus() {
String method = getRequest().getMethod();
return (METHOD_GET.equals(method) || METHOD_HEAD.equals(method));
private boolean validateIfNoneMatch(String etag) {
if (!StringUtils.hasLength(etag)) {
return false;
}
Enumeration<String> ifNoneMatch;
try {
ifNoneMatch = getRequest().getHeaders(IF_NONE_MATCH);
}
catch (IllegalArgumentException ex) {
return false;
}
if (!ifNoneMatch.hasMoreElements()) {
return false;
}
// We will perform this validation...
etag = padEtagIfNecessary(etag);
while (ifNoneMatch.hasMoreElements()) {
String clientETags = ifNoneMatch.nextElement();
Matcher eTagMatcher = ETAG_HEADER_VALUE_PATTERN.matcher(clientETags);
// Compare weak/strong ETags as per https://tools.ietf.org/html/rfc7232#section-2.3
while (eTagMatcher.find()) {
if (StringUtils.hasLength(eTagMatcher.group())
&& etag.replaceFirst("^W/", "").equals(eTagMatcher.group(3))) {
this.notModified = true;
break;
}
}
}
return true;
}
private boolean supportsConditionalUpdate() {
String method = getRequest().getMethod();
return (METHOD_POST.equals(method) || METHOD_PUT.equals(method) || METHOD_DELETE.equals(method))
&& hasRequestHeader(HEADER_IF_UNMODIFIED_SINCE);
private String padEtagIfNecessary(String etag) {
if (!StringUtils.hasLength(etag)) {
return etag;
}
if ((etag.startsWith("\"") || etag.startsWith("W/\"")) && etag.endsWith("\"")) {
return etag;
}
return "\"" + etag + "\"";
}
private boolean isTimestampNotModified(long lastModifiedTimestamp) {
long ifModifiedSince = parseDateHeader(HEADER_IF_MODIFIED_SINCE);
if (ifModifiedSince != -1) {
return (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000));
private boolean validateIfModifiedSince(long lastModifiedTimestamp) {
if (lastModifiedTimestamp < 0) {
return false;
}
long ifUnmodifiedSince = parseDateHeader(HEADER_IF_UNMODIFIED_SINCE);
if (ifUnmodifiedSince != -1) {
return (ifUnmodifiedSince < (lastModifiedTimestamp / 1000 * 1000));
long ifModifiedSince = parseDateHeader(IF_MODIFIED_SINCE);
if (ifModifiedSince == -1) {
return false;
}
return false;
// We will perform this validation...
this.notModified = ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000);
return true;
}
public boolean isNotModified() {
return this.notModified;
}
@SuppressWarnings("deprecation")
private long parseDateHeader(String headerName) {
long dateValue = -1;
try {
@ -335,36 +331,32 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ @@ -335,36 +331,32 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
int separatorIndex = headerValue.indexOf(';');
if (separatorIndex != -1) {
String datePart = headerValue.substring(0, separatorIndex);
try {
dateValue = Date.parse(datePart);
}
catch (IllegalArgumentException ex2) {
// Giving up
}
dateValue = parseDateValue(datePart);
}
}
return dateValue;
}
private boolean isEtagNotModified(String etag) {
String ifNoneMatch = getHeader(HEADER_IF_NONE_MATCH);
// compare weak/strong ETag as per https://tools.ietf.org/html/rfc7232#section-2.3
String serverETag = etag.replaceFirst("^W/", "");
Matcher eTagMatcher = ETAG_HEADER_VALUE_PATTERN.matcher(ifNoneMatch);
while (eTagMatcher.find()) {
if ("*".equals(eTagMatcher.group())
|| serverETag.equals(eTagMatcher.group(3))) {
return true;
}
private long parseDateValue(String headerValue) {
if (headerValue == null) {
// No header value sent at all
return -1;
}
return false;
}
private String addEtagPadding(String etag) {
if (!(etag.startsWith("\"") || etag.startsWith("W/\"")) || !etag.endsWith("\"")) {
etag = "\"" + etag + "\"";
if (headerValue.length() >= 3) {
// Short "0" or "-1" like values are never valid HTTP date headers...
// Let's only bother with SimpleDateFormat parsing for long enough values.
for (String dateFormat : DATE_FORMATS) {
SimpleDateFormat simpleDateFormat = new SimpleDateFormat(dateFormat, Locale.US);
simpleDateFormat.setTimeZone(GMT);
try {
return simpleDateFormat.parse(headerValue).getTime();
}
catch (ParseException ex) {
// ignore
}
}
}
return etag;
return -1;
}
@Override

10
spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java
Unescape View File

@ -94,9 +94,7 @@ public class ServletWebRequestHttpMethodsTests { @@ -94,9 +94,7 @@ public class ServletWebRequestHttpMethodsTests {
servletRequest.addHeader("If-Modified-Since", epochTime);
servletResponse.setStatus(0);
assertTrue(request.checkNotModified(epochTime));
assertEquals(304, servletResponse.getStatus());
assertEquals(dateFormat.format(epochTime), servletResponse.getHeader("Last-Modified"));
assertFalse(request.checkNotModified(epochTime));
}
@Test // SPR-14559
@ -202,13 +200,13 @@ public class ServletWebRequestHttpMethodsTests { @@ -202,13 +200,13 @@ public class ServletWebRequestHttpMethodsTests {
}
@Test
public void checkNotModifiedWildcardETag() {
public void checkNotModifiedWildcardIsIgnored() {
String eTag = "\"Foo\"";
servletRequest.addHeader("If-None-Match", "*");
assertTrue(request.checkNotModified(eTag));
assertFalse(request.checkNotModified(eTag));
assertEquals(304, servletResponse.getStatus());
assertEquals(200, servletResponse.getStatus());
assertEquals(eTag, servletResponse.getHeader("ETag"));
}

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

@ -28,8 +28,6 @@ import org.springframework.core.MethodParameter; @@ -28,8 +28,6 @@ import org.springframework.core.MethodParameter;
import org.springframework.core.ResolvableType;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.RequestEntity;
import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.HttpMessageConverter;
@ -41,6 +39,7 @@ import org.springframework.web.HttpMediaTypeNotSupportedException; @@ -41,6 +39,7 @@ import org.springframework.web.HttpMediaTypeNotSupportedException;
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.method.support.ModelAndViewContainer;
/**
@ -182,15 +181,15 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro @@ -182,15 +181,15 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
}
if (responseEntity instanceof ResponseEntity) {
outputMessage.getServletResponse().setStatus(((ResponseEntity<?>) responseEntity).getStatusCodeValue());
HttpMethod method = inputMessage.getMethod();
boolean isGetOrHead = (HttpMethod.GET == method || HttpMethod.HEAD == method);
if (isGetOrHead && isResourceNotModified(inputMessage, outputMessage)) {
outputMessage.setStatusCode(HttpStatus.NOT_MODIFIED);
// Ensure headers are flushed, no body should be written.
outputMessage.flush();
// Skip call to converters, as they may update the body.
return;
int returnStatus = ((ResponseEntity<?>) responseEntity).getStatusCodeValue();
outputMessage.getServletResponse().setStatus(returnStatus);
if(returnStatus == 200) {
if (isResourceNotModified(inputMessage, outputMessage)) {
// Ensure headers are flushed, no body should be written.
outputMessage.flush();
// Skip call to converters, as they may update the body.
return;
}
}
}
@ -223,55 +222,15 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro @@ -223,55 +222,15 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
}
private boolean isResourceNotModified(ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) {
boolean notModified = false;
try {
long ifModifiedSince = inputMessage.getHeaders().getIfModifiedSince();
String eTag = addEtagPadding(outputMessage.getHeaders().getETag());
long lastModified = outputMessage.getHeaders().getLastModified();
List<String> ifNoneMatch = inputMessage.getHeaders().getIfNoneMatch();
if (!ifNoneMatch.isEmpty() && (inputMessage.getHeaders().containsKey(HttpHeaders.IF_UNMODIFIED_SINCE)
|| inputMessage.getHeaders().containsKey(HttpHeaders.IF_MATCH))) {
// invalid conditional request, do not process
}
else if (lastModified != -1 && StringUtils.hasLength(eTag)) {
notModified = isETagNotModified(ifNoneMatch, eTag) && isTimeStampNotModified(ifModifiedSince, lastModified);
}
else if (lastModified != -1) {
notModified = isTimeStampNotModified(ifModifiedSince, lastModified);
}
else if (StringUtils.hasLength(eTag)) {
notModified = isETagNotModified(ifNoneMatch, eTag);
}
}
catch (IllegalArgumentException exc) {
// invalid conditional request, do not process
}
return notModified;
}
private boolean isETagNotModified(List<String> ifNoneMatch, String etag) {
if (StringUtils.hasLength(etag)) {
for (String clientETag : ifNoneMatch) {
// Compare weak/strong ETags as per https://tools.ietf.org/html/rfc7232#section-2.3
if (StringUtils.hasLength(clientETag) &&
(clientETag.replaceFirst("^W/", "").equals(etag.replaceFirst("^W/", "")))) {
return true;
}
}
}
return false;
}
private boolean isTimeStampNotModified(long ifModifiedSince, long lastModifiedTimestamp) {
return (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000));
}
private String addEtagPadding(String etag) {
if (StringUtils.hasLength(etag) &&
(!(etag.startsWith("\"") || etag.startsWith("W/\"")) || !etag.endsWith("\"")) ) {
etag = "\"" + etag + "\"";
}
return etag;
ServletWebRequest servletWebRequest =
new ServletWebRequest(inputMessage.getServletRequest(), outputMessage.getServletResponse());
HttpHeaders responseHeaders = outputMessage.getHeaders();
String etag = responseHeaders.getETag();
long lastModifiedTimestamp = responseHeaders.getLastModified();
responseHeaders.remove(HttpHeaders.ETAG);
responseHeaders.remove(HttpHeaders.LAST_MODIFIED);
return servletWebRequest.checkNotModified(etag, lastModifiedTimestamp);
}
@Override

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

@ -476,8 +476,7 @@ public class HttpEntityMethodProcessorMockTests { @@ -476,8 +476,7 @@ public class HttpEntityMethodProcessorMockTests {
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
assertResponseOkWithBody("body");
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
assertEquals(0, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
}
// SPR-13626
@ -511,7 +510,7 @@ public class HttpEntityMethodProcessorMockTests { @@ -511,7 +510,7 @@ public class HttpEntityMethodProcessorMockTests {
initStringMessageConversion(MediaType.TEXT_PLAIN);
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
assertResponseOkWithBody("body");
assertResponseNotModified();
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
}
@ -529,7 +528,7 @@ public class HttpEntityMethodProcessorMockTests { @@ -529,7 +528,7 @@ public class HttpEntityMethodProcessorMockTests {
initStringMessageConversion(MediaType.TEXT_PLAIN);
processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest);
assertResponseOkWithBody("body");
assertResponseNotModified();
assertEquals(1, servletResponse.getHeaderValues(HttpHeaders.ETAG).size());
assertEquals(etagValue, servletResponse.getHeader(HttpHeaders.ETAG));
}

Write Preview
Loading…
Cancel
Save