Improve Last-Modified & ETag support

Prior to this change, the `"Last-Modified"` and "`Etag`" support had
been improved with SPR-11324: HTTP response headers are now
automatically added for conditional requests and more.

This commit fixes the format of the "`Last-Modified`" and "`ETag`"
values, which were using an epoch timestamp rather than an HTTP-date
format defined in RFC 7231 section 7.1.1.1.

Also, Conditional responses are only applied when the given response
applies, i.e. when it has an compatible HTTP status (2xx).

Issue: SPR-13090
This commit is contained in:
Brian Clozel
2015-06-05 14:30:09 +02:00
parent 75edb3979e
commit 0175068cab
9 changed files with 129 additions and 49 deletions

View File

@@ -17,16 +17,19 @@
package org.springframework.web.context.request;
import java.security.Principal;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Iterator;
import java.util.Locale;
import java.util.Map;
import java.util.TimeZone;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.util.CollectionUtils;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
@@ -55,6 +58,10 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
private static final String METHOD_HEAD = "HEAD";
private static final String DATE_FORMAT = "EEE, dd MMM yyyy HH:mm:ss zzz";
private static TimeZone GMT = TimeZone.getTimeZone("GMT");
private boolean notModified = false;
@@ -174,19 +181,33 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
public boolean checkNotModified(long lastModifiedTimestamp) {
HttpServletResponse response = getResponse();
if (lastModifiedTimestamp >= 0 && !this.notModified) {
if (response == null || !response.containsHeader(HEADER_LAST_MODIFIED)) {
if (response == null || isResponseCompatibleWithConditional(response, HEADER_LAST_MODIFIED)) {
this.notModified = isTimeStampNotModified(lastModifiedTimestamp);
if (response != null) {
if (this.notModified && supportsNotModifiedStatus()) {
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
}
response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
response.setHeader(HEADER_LAST_MODIFIED, formatDate(lastModifiedTimestamp));
}
}
}
return this.notModified;
}
private boolean isResponseCompatibleWithConditional(HttpServletResponse response, String... headers) {
if (response != null) {
if (HttpStatus.valueOf(response.getStatus()).is2xxSuccessful()) {
for (String header : headers) {
if (response.containsHeader(header)) {
return false;
}
}
return true;
}
}
return false;
}
@SuppressWarnings("deprecation")
private boolean isTimeStampNotModified(long lastModifiedTimestamp) {
long ifModifiedSince = -1;
@@ -214,7 +235,7 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
public boolean checkNotModified(String etag) {
HttpServletResponse response = getResponse();
if (StringUtils.hasLength(etag) && !this.notModified) {
if (response == null || !response.containsHeader(HEADER_ETAG)) {
if (response == null || isResponseCompatibleWithConditional(response, HEADER_ETAG)) {
etag = addEtagPadding(etag);
this.notModified = isETagNotModified(etag);
if (response != null) {
@@ -244,7 +265,7 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
// 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/", ""))
|| clientETag.equals("*"))) {
|| clientETag.equals("*"))) {
return true;
}
}
@@ -262,8 +283,7 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
public boolean checkNotModified(String etag, long lastModifiedTimestamp) {
HttpServletResponse response = getResponse();
if (StringUtils.hasLength(etag) && !this.notModified) {
if (response == null ||
(!response.containsHeader(HEADER_ETAG) && !response.containsHeader(HEADER_LAST_MODIFIED))) {
if (response == null || isResponseCompatibleWithConditional(response, HEADER_ETAG, HEADER_LAST_MODIFIED)) {
etag = addEtagPadding(etag);
this.notModified = isETagNotModified(etag) && isTimeStampNotModified(lastModifiedTimestamp);
if (response != null) {
@@ -271,13 +291,19 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
}
response.setHeader(HEADER_ETAG, etag);
response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
response.setHeader(HEADER_LAST_MODIFIED, formatDate(lastModifiedTimestamp));
}
}
}
return this.notModified;
}
private String formatDate(long date) {
SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT, Locale.US);
dateFormat.setTimeZone(GMT);
return dateFormat.format(new Date(date));
}
public boolean isNotModified() {
return this.notModified;
}

View File

@@ -194,7 +194,7 @@ public interface WebRequest extends RequestAttributes {
* Check whether the request qualifies as not modified given the
* supplied {@code ETag} (entity tag) and last-modified timestamp,
* as determined by the application.
* <p>This will also transparently set the appropriate response headers,
* <p>This will also transparently set the "Etag" and "Last-Modified" response headers,
* for both the modified case and the not-modified case.
* <p>Typical usage:
* <pre class="code">

View File

@@ -20,6 +20,7 @@ import java.text.SimpleDateFormat;
import java.util.Arrays;
import java.util.Date;
import java.util.Locale;
import java.util.TimeZone;
import org.junit.Before;
import org.junit.Test;
@@ -42,6 +43,8 @@ import static org.junit.Assert.*;
@RunWith(Parameterized.class)
public class ServletWebRequestHttpMethodsTests {
private static final String CURRENT_TIME = "Wed, 09 Apr 2014 09:57:42 GMT";
private SimpleDateFormat dateFormat;
private MockHttpServletRequest servletRequest;
@@ -50,6 +53,8 @@ public class ServletWebRequestHttpMethodsTests {
private ServletWebRequest request;
private Date currentDate;
@Parameter
public String method;
@@ -63,33 +68,57 @@ public class ServletWebRequestHttpMethodsTests {
@Before
public void setUp() {
currentDate = new Date();
dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z", Locale.US);
dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
servletRequest = new MockHttpServletRequest(method, "http://example.org");
servletResponse = new MockHttpServletResponse();
request = new ServletWebRequest(servletRequest, servletResponse);
}
@Test
public void checkNotModifiedTimestamp() {
long currentTime = new Date().getTime();
servletRequest.addHeader("If-Modified-Since", currentTime);
public void checkNotModifiedNon2xxStatus() {
long epochTime = currentDate.getTime();
servletRequest.addHeader("If-Modified-Since", epochTime);
servletResponse.setStatus(304);
assertTrue(request.checkNotModified(currentTime));
assertFalse(request.checkNotModified(epochTime));
assertEquals(304, servletResponse.getStatus());
assertNull(servletResponse.getHeader("Last-Modified"));
}
@Test
public void checkNotModifiedHeaderAlreadySet() {
long epochTime = currentDate.getTime();
servletRequest.addHeader("If-Modified-Since", epochTime);
servletResponse.addHeader("Last-Modified", CURRENT_TIME);
assertFalse(request.checkNotModified(epochTime));
assertEquals(200, servletResponse.getStatus());
assertEquals(1, servletResponse.getHeaders("Last-Modified").size());
assertEquals(CURRENT_TIME, servletResponse.getHeader("Last-Modified"));
}
@Test
public void checkNotModifiedTimestamp() throws Exception {
long epochTime = currentDate.getTime();
servletRequest.addHeader("If-Modified-Since", epochTime);
assertTrue(request.checkNotModified(epochTime));
assertEquals(304, servletResponse.getStatus());
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
}
@Test
public void checkModifiedTimestamp() {
long currentTime = new Date().getTime();
long oneMinuteAgo = currentTime - (1000 * 60);
long oneMinuteAgo = currentDate.getTime() - (1000 * 60);
servletRequest.addHeader("If-Modified-Since", oneMinuteAgo);
assertFalse(request.checkNotModified(currentTime));
assertFalse(request.checkNotModified(currentDate.getTime()));
assertEquals(200, servletResponse.getStatus());
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
}
@Test
@@ -154,44 +183,43 @@ public class ServletWebRequestHttpMethodsTests {
public void checkNotModifiedETagAndTimestamp() {
String eTag = "\"Foo\"";
servletRequest.addHeader("If-None-Match", eTag);
long currentTime = new Date().getTime();
servletRequest.addHeader("If-Modified-Since", currentTime);
servletRequest.addHeader("If-Modified-Since", currentDate.getTime());
assertTrue(request.checkNotModified(eTag, currentTime));
assertTrue(request.checkNotModified(eTag, currentDate.getTime()));
assertEquals(304, servletResponse.getStatus());
assertEquals(eTag, servletResponse.getHeader("ETag"));
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
}
@Test
public void checkNotModifiedETagAndModifiedTimestamp() {
String eTag = "\"Foo\"";
servletRequest.addHeader("If-None-Match", eTag);
long currentTime = new Date().getTime();
long oneMinuteAgo = currentTime - (1000 * 60);
long currentEpoch = currentDate.getTime();
long oneMinuteAgo = currentEpoch - (1000 * 60);
servletRequest.addHeader("If-Modified-Since", oneMinuteAgo);
assertFalse(request.checkNotModified(eTag, currentTime));
assertFalse(request.checkNotModified(eTag, currentEpoch));
assertEquals(200, servletResponse.getStatus());
assertEquals(eTag, servletResponse.getHeader("ETag"));
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
}
@Test
public void checkModifiedETagAndNotModifiedTimestamp() {
public void checkModifiedETagAndNotModifiedTimestamp() throws Exception {
String currentETag = "\"Foo\"";
String oldEtag = "\"Bar\"";
servletRequest.addHeader("If-None-Match", oldEtag);
long currentTime = new Date().getTime();
servletRequest.addHeader("If-Modified-Since", currentTime);
long epochTime = currentDate.getTime();
servletRequest.addHeader("If-Modified-Since", epochTime);
assertFalse(request.checkNotModified(currentETag, currentTime));
assertFalse(request.checkNotModified(currentETag, epochTime));
assertEquals(200, servletResponse.getStatus());
assertEquals(currentETag, servletResponse.getHeader("ETag"));
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
}
@Test
@@ -231,26 +259,26 @@ public class ServletWebRequestHttpMethodsTests {
@Test
public void checkNotModifiedTimestampWithLengthPart() throws Exception {
long currentTime = dateFormat.parse("Wed, 09 Apr 2014 09:57:42 GMT").getTime();
long epochTime = dateFormat.parse(CURRENT_TIME).getTime();
servletRequest.setMethod("GET");
servletRequest.addHeader("If-Modified-Since", "Wed, 09 Apr 2014 09:57:42 GMT; length=13774");
assertTrue(request.checkNotModified(currentTime));
assertTrue(request.checkNotModified(epochTime));
assertEquals(304, servletResponse.getStatus());
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
assertEquals(CURRENT_TIME, servletResponse.getHeader("Last-Modified"));
}
@Test
public void checkModifiedTimestampWithLengthPart() throws Exception {
long currentTime = dateFormat.parse("Wed, 09 Apr 2014 09:57:42 GMT").getTime();
long epochTime = dateFormat.parse(CURRENT_TIME).getTime();
servletRequest.setMethod("GET");
servletRequest.addHeader("If-Modified-Since", "Wed, 08 Apr 2014 09:57:42 GMT; length=13774");
assertFalse(request.checkNotModified(currentTime));
assertFalse(request.checkNotModified(epochTime));
assertEquals(200, servletResponse.getStatus());
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
assertEquals(CURRENT_TIME, servletResponse.getHeader("Last-Modified"));
}
}