From 0d6f80052d7a08bec69c2ef70818ff6551b54c5c Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 1 Mar 2016 14:37:29 +0100 Subject: [PATCH] Support conditional updates in ServletWebRequest Prior to this commit, `ServletWebRequest.checkNotModified` would only support conditional GET/HEAD requests with "If-Modified-Since" and/or "If-None-Match" request headers. In those cases, the server would return "HTTP 304 Not Modified" responses if the resource didn't change. This commit adds support for conditional update requests, such as POST/PUT/DELETE requests with "If-Unmodified-Since" request headers. If the underlying resource has been modified since the specified date, the server will return a "409 Precondition failed" response status to prevent concurrent updates. Even if the modification status of the resource is reversed here (modified vs. not modified), we're keeping here the same intent for the return value, which signals if the response requires more processing or if the handler method can return immediately: ``` if (request.checkNotModified(lastModified)) { // shortcut exit - no further processing necessary return null; } ``` Issue: SPR-13863 --- .../context/request/ServletWebRequest.java | 81 ++++++++++++++----- .../web/context/request/WebRequest.java | 30 +++---- .../ServletWebRequestHttpMethodsTests.java | 30 ++++++- src/asciidoc/web-mvc.adoc | 15 +++- 4 files changed, 115 insertions(+), 41 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java index 6cc8e052721..b9ff874f9bb 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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,6 +21,7 @@ import java.util.Date; import java.util.Iterator; import java.util.Locale; import java.util.Map; + import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; @@ -47,6 +48,8 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ 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"; @@ -55,6 +58,12 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ private static final String METHOD_HEAD = "HEAD"; + private static final String METHOD_POST = "POST"; + + private static final String METHOD_PUT = "PUT"; + + private static final String METHOD_DELETE = "DELETE"; + /** Checking for Servlet 3.0+ HttpServletResponse.getHeader(String) */ private static final boolean servlet3Present = @@ -183,11 +192,18 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ if (isCompatibleWithConditionalRequests(response)) { this.notModified = isTimestampNotModified(lastModifiedTimestamp); if (response != null) { - if (this.notModified && supportsNotModifiedStatus()) { - response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); + if (supportsNotModifiedStatus()) { + if (this.notModified) { + response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); + } + if (isHeaderAbsent(response, HEADER_LAST_MODIFIED)) { + response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp); + } } - if (isHeaderAbsent(response, HEADER_LAST_MODIFIED)) { - response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp); + else if (supportsConditionalUpdate()) { + if (this.notModified) { + response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED); + } } } } @@ -223,14 +239,21 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ etag = addEtagPadding(etag); this.notModified = isEtagNotModified(etag) && isTimestampNotModified(lastModifiedTimestamp); if (response != null) { - if (this.notModified && supportsNotModifiedStatus()) { - response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); - } - if (isHeaderAbsent(response, HEADER_ETAG)) { - response.setHeader(HEADER_ETAG, etag); + 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); + } } - if (isHeaderAbsent(response, HEADER_LAST_MODIFIED)) { - response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp); + else if (supportsConditionalUpdate()) { + if (this.notModified) { + response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED); + } } } } @@ -250,7 +273,8 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ return true; } return HttpStatus.valueOf(response.getStatus()).is2xxSuccessful(); - } catch (IllegalArgumentException e) { + } + catch (IllegalArgumentException e) { return true; } } @@ -268,27 +292,46 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ return (METHOD_GET.equals(method) || METHOD_HEAD.equals(method)); } - @SuppressWarnings("deprecation") + private boolean supportsConditionalUpdate() { + String method = getRequest().getMethod(); + String ifUnmodifiedHeader = getRequest().getHeader(HEADER_IF_UNMODIFIED_SINCE); + return (METHOD_POST.equals(method) || METHOD_PUT.equals(method) || METHOD_DELETE.equals(method)) + && StringUtils.hasLength(ifUnmodifiedHeader); + } + private boolean isTimestampNotModified(long lastModifiedTimestamp) { - long ifModifiedSince = -1; + long ifModifiedSince = parseDateHeader(HEADER_IF_MODIFIED_SINCE); + if (ifModifiedSince != -1) { + return (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000)); + } + long ifUnmodifiedSince = parseDateHeader(HEADER_IF_UNMODIFIED_SINCE); + if (ifUnmodifiedSince != -1) { + return (ifUnmodifiedSince < (lastModifiedTimestamp / 1000 * 1000)); + } + return false; + } + + @SuppressWarnings("deprecation") + private long parseDateHeader(String headerName) { + long dateValue = -1; try { - ifModifiedSince = getRequest().getDateHeader(HEADER_IF_MODIFIED_SINCE); + dateValue = getRequest().getDateHeader(headerName); } catch (IllegalArgumentException ex) { - String headerValue = getRequest().getHeader(HEADER_IF_MODIFIED_SINCE); + String headerValue = getRequest().getHeader(headerName); // Possibly an IE 10 style value: "Wed, 09 Apr 2014 09:57:42 GMT; length=13774" int separatorIndex = headerValue.indexOf(';'); if (separatorIndex != -1) { String datePart = headerValue.substring(0, separatorIndex); try { - ifModifiedSince = Date.parse(datePart); + dateValue = Date.parse(datePart); } catch (IllegalArgumentException ex2) { // Giving up } } } - return (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000)); + return dateValue; } private boolean isEtagNotModified(String etag) { diff --git a/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java index cea9fa70af3..8c190f74c24 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -126,10 +126,10 @@ public interface WebRequest extends RequestAttributes { boolean isSecure(); /** - * Check whether the request qualifies as not modified given the + * Check whether the requested resource has been modified given the * supplied last-modified timestamp (as determined by the application). - *

This will also transparently set the appropriate response headers, - * for both the modified case and the not-modified case. + *

This will also transparently set the "Last-Modified" response header + * and HTTP status when applicable. *

Typical usage: * 

 	 * public String myHandleMethod(WebRequest webRequest, Model model) {
@@ -142,6 +142,8 @@ public interface WebRequest extends RequestAttributes {
 	 *   model.addAttribute(...);
 	 *   return "myViewName";
 	 * }
+ *

This method works with conditional GET/HEAD requests, but + * also with conditional POST/PUT/DELETE requests. *

Note: you can use either * this {@code #checkNotModified(long)} method; or * {@link #checkNotModified(String)}. If you want enforce both @@ -160,10 +162,10 @@ public interface WebRequest extends RequestAttributes { boolean checkNotModified(long lastModifiedTimestamp); /** - * Check whether the request qualifies as not modified given the + * Check whether the requested resource has been modified given the * supplied {@code ETag} (entity tag), as determined by the application. - *

This will also transparently set the appropriate response headers, - * for both the modified case and the not-modified case. + *

This will also transparently set the "ETag" response header + * and HTTP status when applicable. *

Typical usage: * 

 	 * public String myHandleMethod(WebRequest webRequest, Model model) {
@@ -185,18 +187,16 @@ public interface WebRequest extends RequestAttributes {
 	 * @param etag the entity tag that the application determined
 	 * for the underlying resource. This parameter will be padded
 	 * with quotes (") if necessary.
-	 * @return whether the request qualifies as not modified,
-	 * allowing to abort request processing and relying on the response
-	 * telling the client that the content has not been modified
+	 * @return true if the request does not require further processing.
 	 */
 	boolean checkNotModified(String etag);
 
 	/**
-	 * Check whether the request qualifies as not modified given the
+	 * Check whether the requested resource has been modified given the
 	 * supplied {@code ETag} (entity tag) and last-modified timestamp,
 	 * as determined by the application.
 	 * 
This will also transparently set the "ETag" and "Last-Modified"
-	 * response headers, for both the modified case and the not-modified case.
+	 * response headers, and HTTP status when applicable.
 	 * 
Typical usage:
 	 * 
 	 * public String myHandleMethod(WebRequest webRequest, Model model) {
@@ -210,6 +210,8 @@ public interface WebRequest extends RequestAttributes {
 	 *   model.addAttribute(...);
 	 *   return "myViewName";
 	 * }

+	 * 
This method works with conditional GET/HEAD requests, but
+	 * also with conditional POST/PUT/DELETE requests.
 	 * 
Note: The HTTP specification recommends
 	 * setting both ETag and Last-Modified values, but you can also
 	 * use {@code #checkNotModified(String)} or
@@ -219,9 +221,7 @@ public interface WebRequest extends RequestAttributes {
 	 * with quotes (") if necessary.
 	 * @param lastModifiedTimestamp the last-modified timestamp that
 	 * the application determined for the underlying resource
-	 * @return whether the request qualifies as not modified,
-	 * allowing to abort request processing and relying on the response
-	 * telling the client that the content has not been modified
+	 * @return true if the request does not require further processing.
 	 * @since 4.2
 	 */
 	boolean checkNotModified(String etag, long lastModifiedTimestamp);
diff --git a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java
index 3832ae3a0b4..6dd2adf9edf 100644
--- a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java
+++ b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.context.request;
 
+import static org.junit.Assert.*;
+
 import java.text.SimpleDateFormat;
 import java.util.Arrays;
 import java.util.Date;
@@ -32,8 +34,6 @@ import org.junit.runners.Parameterized.Parameters;
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.mock.web.test.MockHttpServletResponse;
 
-import static org.junit.Assert.*;
-
 /**
  * Parameterized tests for ServletWebRequest
  * @author Juergen Hoeller
@@ -293,4 +293,28 @@ public class ServletWebRequestHttpMethodsTests {
 		assertEquals(dateFormat.format(epochTime), servletResponse.getHeader("Last-Modified"));
 	}
 
+	@Test
+	public void checkNotModifiedTimestampConditionalPut() throws Exception {
+		long currentEpoch = currentDate.getTime();
+		long oneMinuteAgo = currentEpoch - (1000 * 60);
+		servletRequest.setMethod("PUT");
+		servletRequest.addHeader("If-UnModified-Since", currentEpoch);
+
+		assertFalse(request.checkNotModified(oneMinuteAgo));
+		assertEquals(200, servletResponse.getStatus());
+		assertEquals(null, servletResponse.getHeader("Last-Modified"));
+	}
+
+	@Test
+	public void checkNotModifiedTimestampConditionalPutConflict() throws Exception {
+		long currentEpoch = currentDate.getTime();
+		long oneMinuteAgo = currentEpoch - (1000 * 60);
+		servletRequest.setMethod("PUT");
+		servletRequest.addHeader("If-UnModified-Since", oneMinuteAgo);
+
+		assertTrue(request.checkNotModified(currentEpoch));
+		assertEquals(412, servletResponse.getStatus());
+		assertEquals(null, servletResponse.getHeader("Last-Modified"));
+	}
+
 }
diff --git a/src/asciidoc/web-mvc.adoc b/src/asciidoc/web-mvc.adoc
index 2f82b89af83..2db3fc036b4 100644
--- a/src/asciidoc/web-mvc.adoc
+++ b/src/asciidoc/web-mvc.adoc
@@ -4449,17 +4449,24 @@ This can be achieved as follows:
 ----
 
 There are two key elements here: calling `request.checkNotModified(lastModified)` and
-returning `null`. The former sets the response status to 304 before it returns `true`.
+returning `null`. The former sets the appropriate response status and headers
+before it returns `true`.
 The latter, in combination with the former, causes Spring MVC to do no further
 processing of the request.
 
 Note that there are 3 variants for this:
 
 * `request.checkNotModified(lastModified)` compares lastModified with the
-`'If-Modified-Since'` request header
-* `request.checkNotModified(eTag)` compares eTag with the `'ETag'` request header
+`'If-Modified-Since'` or `'If-Unmodified-Since'` request header
+* `request.checkNotModified(eTag)` compares eTag with the `'If-None-Match'` request header
 * `request.checkNotModified(eTag, lastModified)` does both, meaning that both
-conditions should be valid for the server to issue an `HTTP 304 Not Modified` response
+conditions should be valid
+
+When receiving conditional `'GET'`/`'HEAD'` requests, `checkNotModified` will check
+that the resource has not been modified and if so, it will result in a `HTTP 304 Not Modified`
+response. In case of conditional `'POST'`/`'PUT'`/`'DELETE'` requests, `checkNotModified`
+will check that the resource has not been modified and if it has been, it will result in a
+`HTTP 409 Precondition Failed` response to prevent concurrent modifications.
 
 
 [[mvc-httpcaching-shallowetag]]