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
commit
cc5300c4d5
  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

@ -17,12 +17,18 @@
package org.springframework.web.context.request; package org.springframework.web.context.request;
import java.security.Principal; 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.Iterator;
import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Map; import java.util.Map;
import java.util.TimeZone;
import java.util.regex.Matcher; import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession; import javax.servlet.http.HttpSession;
@ -44,25 +50,17 @@ import org.springframework.web.util.WebUtils;
*/ */
public class ServletWebRequest extends ServletRequestAttributes implements NativeWebRequest { public class ServletWebRequest extends ServletRequestAttributes implements NativeWebRequest {
private static final String HEADER_ETAG = "ETag"; private static final String 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 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" * 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
*/ */
private static final Pattern ETAG_HEADER_VALUE_PATTERN = Pattern.compile("\\*|\\s*((W\\/)?(\"[^\"]*\"))\\s*,?"); 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; private boolean notModified = false;
@ -189,141 +198,128 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
@Override @Override
public boolean checkNotModified(long lastModifiedTimestamp) { public boolean checkNotModified(long lastModifiedTimestamp) {
HttpServletResponse response = getResponse(); return checkNotModified(null, lastModifiedTimestamp);
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;
} }
@Override @Override
public boolean checkNotModified(String etag) { public boolean checkNotModified(String etag) {
HttpServletResponse response = getResponse(); return checkNotModified(etag, -1);
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;
} }
@Override @Override
public boolean checkNotModified(String etag, long lastModifiedTimestamp) { public boolean checkNotModified(String etag, long lastModifiedTimestamp) {
HttpServletResponse response = getResponse(); HttpServletResponse response = getResponse();
if (StringUtils.hasLength(etag) && !this.notModified) { if (this.notModified || HttpStatus.OK.value() != response.getStatus()) {
if (isCompatibleWithConditionalRequests(response)) { return this.notModified;
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);
}
}
}
}
} }
return this.notModified;
}
public boolean isNotModified() { // Evaluate conditions in order of precedence.
return this.notModified; // See https://tools.ietf.org/html/rfc7232#section-6
}
if (validateIfUnmodifiedSince(lastModifiedTimestamp)) {
private boolean isCompatibleWithConditionalRequests(HttpServletResponse response) { if (this.notModified) {
try { response.setStatus(HttpStatus.PRECONDITION_FAILED.value());
if (response == null) {
// Can't check response.getStatus() - let's assume we're good
return true;
} }
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) { // Update response
if (response == null) {
// Can't check response.getHeader(header) - let's assume it's not set boolean isHttpGetOrHead = SAFE_METHODS.contains(getRequest().getMethod());
return true; 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) { private boolean validateIfUnmodifiedSince(long lastModifiedTimestamp) {
return StringUtils.hasLength(getHeader(headerName)); 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() { private boolean validateIfNoneMatch(String etag) {
String method = getRequest().getMethod(); if (!StringUtils.hasLength(etag)) {
return (METHOD_GET.equals(method) || METHOD_HEAD.equals(method)); 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() { private String padEtagIfNecessary(String etag) {
String method = getRequest().getMethod(); if (!StringUtils.hasLength(etag)) {
return (METHOD_POST.equals(method) || METHOD_PUT.equals(method) || METHOD_DELETE.equals(method)) return etag;
&& hasRequestHeader(HEADER_IF_UNMODIFIED_SINCE); }
if ((etag.startsWith("\"") || etag.startsWith("W/\"")) && etag.endsWith("\"")) {
return etag;
}
return "\"" + etag + "\"";
} }
private boolean isTimestampNotModified(long lastModifiedTimestamp) { private boolean validateIfModifiedSince(long lastModifiedTimestamp) {
long ifModifiedSince = parseDateHeader(HEADER_IF_MODIFIED_SINCE); if (lastModifiedTimestamp < 0) {
if (ifModifiedSince != -1) { return false;
return (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000));
} }
long ifUnmodifiedSince = parseDateHeader(HEADER_IF_UNMODIFIED_SINCE); long ifModifiedSince = parseDateHeader(IF_MODIFIED_SINCE);
if (ifUnmodifiedSince != -1) { if (ifModifiedSince == -1) {
return (ifUnmodifiedSince < (lastModifiedTimestamp / 1000 * 1000)); 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) { private long parseDateHeader(String headerName) {
long dateValue = -1; long dateValue = -1;
try { try {
@ -335,36 +331,32 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
int separatorIndex = headerValue.indexOf(';'); int separatorIndex = headerValue.indexOf(';');
if (separatorIndex != -1) { if (separatorIndex != -1) {
String datePart = headerValue.substring(0, separatorIndex); String datePart = headerValue.substring(0, separatorIndex);
try { dateValue = parseDateValue(datePart);
dateValue = Date.parse(datePart);
}
catch (IllegalArgumentException ex2) {
// Giving up
}
} }
} }
return dateValue; return dateValue;
} }
private boolean isEtagNotModified(String etag) { private long parseDateValue(String headerValue) {
String ifNoneMatch = getHeader(HEADER_IF_NONE_MATCH); if (headerValue == null) {
// compare weak/strong ETag as per https://tools.ietf.org/html/rfc7232#section-2.3 // No header value sent at all
String serverETag = etag.replaceFirst("^W/", ""); return -1;
Matcher eTagMatcher = ETAG_HEADER_VALUE_PATTERN.matcher(ifNoneMatch);
while (eTagMatcher.find()) {
if ("*".equals(eTagMatcher.group())
|| serverETag.equals(eTagMatcher.group(3))) {
return true;
}
} }
return false; 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.
private String addEtagPadding(String etag) { for (String dateFormat : DATE_FORMATS) {
if (!(etag.startsWith("\"") || etag.startsWith("W/\"")) || !etag.endsWith("\"")) { SimpleDateFormat simpleDateFormat = new SimpleDateFormat(dateFormat, Locale.US);
etag = "\"" + etag + "\""; simpleDateFormat.setTimeZone(GMT);
try {
return simpleDateFormat.parse(headerValue).getTime();
}
catch (ParseException ex) {
// ignore
}
}
} }
return etag; return -1;
} }
@Override @Override

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

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

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

@ -28,8 +28,6 @@ import org.springframework.core.MethodParameter;
import org.springframework.core.ResolvableType; import org.springframework.core.ResolvableType;
import org.springframework.http.HttpEntity; import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders; import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.RequestEntity; import org.springframework.http.RequestEntity;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter;
@ -41,6 +39,7 @@ import org.springframework.web.HttpMediaTypeNotSupportedException;
import org.springframework.web.accept.ContentNegotiationManager; import org.springframework.web.accept.ContentNegotiationManager;
import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.context.request.ServletWebRequest;
import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.method.support.ModelAndViewContainer;
/** /**
@ -182,15 +181,15 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
} }
if (responseEntity instanceof ResponseEntity) { if (responseEntity instanceof ResponseEntity) {
outputMessage.getServletResponse().setStatus(((ResponseEntity<?>) responseEntity).getStatusCodeValue()); int returnStatus = ((ResponseEntity<?>) responseEntity).getStatusCodeValue();
HttpMethod method = inputMessage.getMethod(); outputMessage.getServletResponse().setStatus(returnStatus);
boolean isGetOrHead = (HttpMethod.GET == method || HttpMethod.HEAD == method); if(returnStatus == 200) {
if (isGetOrHead && isResourceNotModified(inputMessage, outputMessage)) { if (isResourceNotModified(inputMessage, outputMessage)) {
outputMessage.setStatusCode(HttpStatus.NOT_MODIFIED); // Ensure headers are flushed, no body should be written.
// Ensure headers are flushed, no body should be written. outputMessage.flush();
outputMessage.flush(); // Skip call to converters, as they may update the body.
// Skip call to converters, as they may update the body. return;
return; }
} }
} }
@ -223,55 +222,15 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
} }
private boolean isResourceNotModified(ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) { private boolean isResourceNotModified(ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) {
boolean notModified = false; ServletWebRequest servletWebRequest =
try { new ServletWebRequest(inputMessage.getServletRequest(), outputMessage.getServletResponse());
long ifModifiedSince = inputMessage.getHeaders().getIfModifiedSince(); HttpHeaders responseHeaders = outputMessage.getHeaders();
String eTag = addEtagPadding(outputMessage.getHeaders().getETag()); String etag = responseHeaders.getETag();
long lastModified = outputMessage.getHeaders().getLastModified(); long lastModifiedTimestamp = responseHeaders.getLastModified();
List<String> ifNoneMatch = inputMessage.getHeaders().getIfNoneMatch(); responseHeaders.remove(HttpHeaders.ETAG);
if (!ifNoneMatch.isEmpty() && (inputMessage.getHeaders().containsKey(HttpHeaders.IF_UNMODIFIED_SINCE) responseHeaders.remove(HttpHeaders.LAST_MODIFIED);
|| inputMessage.getHeaders().containsKey(HttpHeaders.IF_MATCH))) {
// invalid conditional request, do not process return servletWebRequest.checkNotModified(etag, lastModifiedTimestamp);
}
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;
} }
@Override @Override

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

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

Loading…
Cancel
Save