Skip to content

Commit 0175068

Browse files
bclozelbclozel
committed
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
1 parent 75edb39 commit 0175068

File tree

9 files changed

+129
-49
lines changed

9 files changed

+129
-49
lines changed

‎spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,19 @@
1717
package org.springframework.web.context.request;
1818

1919
import java.security.Principal;
20+
import java.text.SimpleDateFormat;
2021
import java.util.Date;
2122
import java.util.Iterator;
2223
import java.util.Locale;
2324
import java.util.Map;
25+
import java.util.TimeZone;
2426

2527
import javax.servlet.http.HttpServletRequest;
2628
import javax.servlet.http.HttpServletResponse;
2729
import javax.servlet.http.HttpSession;
2830

2931
import org.springframework.http.HttpMethod;
32+
import org.springframework.http.HttpStatus;
3033
import org.springframework.util.CollectionUtils;
3134
import org.springframework.util.ObjectUtils;
3235
import org.springframework.util.StringUtils;
@@ -55,6 +58,10 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
5558

5659
private static final String METHOD_HEAD = "HEAD";
5760

61+
private static final String DATE_FORMAT = "EEE, dd MMM yyyy HH:mm:ss zzz";
62+
63+
private static TimeZone GMT = TimeZone.getTimeZone("GMT");
64+
5865

5966
private boolean notModified = false;
6067

@@ -174,19 +181,33 @@ public boolean isSecure() {
174181
public boolean checkNotModified(long lastModifiedTimestamp) {
175182
HttpServletResponse response = getResponse();
176183
if (lastModifiedTimestamp >= 0 && !this.notModified) {
177-
if (response == null || !response.containsHeader(HEADER_LAST_MODIFIED)) {
184+
if (response == null || isResponseCompatibleWithConditional(response, HEADER_LAST_MODIFIED)) {
178185
this.notModified = isTimeStampNotModified(lastModifiedTimestamp);
179186
if (response != null) {
180187
if (this.notModified && supportsNotModifiedStatus()) {
181188
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
182189
}
183-
response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
190+
response.setHeader(HEADER_LAST_MODIFIED, formatDate(lastModifiedTimestamp));
184191
}
185192
}
186193
}
187194
return this.notModified;
188195
}
189196

197+
private boolean isResponseCompatibleWithConditional(HttpServletResponse response, String... headers) {
198+
if (response != null) {
199+
if (HttpStatus.valueOf(response.getStatus()).is2xxSuccessful()) {
200+
for (String header : headers) {
201+
if (response.containsHeader(header)) {
202+
return false;
203+
}
204+
}
205+
return true;
206+
}
207+
}
208+
return false;
209+
}
210+
190211
@SuppressWarnings("deprecation")
191212
private boolean isTimeStampNotModified(long lastModifiedTimestamp) {
192213
long ifModifiedSince = -1;
@@ -214,7 +235,7 @@ private boolean isTimeStampNotModified(long lastModifiedTimestamp) {
214235
public boolean checkNotModified(String etag) {
215236
HttpServletResponse response = getResponse();
216237
if (StringUtils.hasLength(etag) && !this.notModified) {
217-
if (response == null || !response.containsHeader(HEADER_ETAG)) {
238+
if (response == null || isResponseCompatibleWithConditional(response, HEADER_ETAG)) {
218239
etag = addEtagPadding(etag);
219240
this.notModified = isETagNotModified(etag);
220241
if (response != null) {
@@ -244,7 +265,7 @@ private boolean isETagNotModified(String etag) {
244265
// compare weak/strong ETags as per https://tools.ietf.org/html/rfc7232#section-2.3
245266
if (StringUtils.hasLength(clientETag) &&
246267
(clientETag.replaceFirst("^W/", "").equals(etag.replaceFirst("^W/", ""))
247-
|| clientETag.equals("*"))) {
268+
|| clientETag.equals("*"))) {
248269
return true;
249270
}
250271
}
@@ -262,22 +283,27 @@ private boolean supportsNotModifiedStatus() {
262283
public boolean checkNotModified(String etag, long lastModifiedTimestamp) {
263284
HttpServletResponse response = getResponse();
264285
if (StringUtils.hasLength(etag) && !this.notModified) {
265-
if (response == null ||
266-
(!response.containsHeader(HEADER_ETAG) && !response.containsHeader(HEADER_LAST_MODIFIED))) {
286+
if (response == null || isResponseCompatibleWithConditional(response, HEADER_ETAG, HEADER_LAST_MODIFIED)) {
267287
etag = addEtagPadding(etag);
268288
this.notModified = isETagNotModified(etag) && isTimeStampNotModified(lastModifiedTimestamp);
269289
if (response != null) {
270290
if (this.notModified && supportsNotModifiedStatus()) {
271291
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
272292
}
273293
response.setHeader(HEADER_ETAG, etag);
274-
response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
294+
response.setHeader(HEADER_LAST_MODIFIED, formatDate(lastModifiedTimestamp));
275295
}
276296
}
277297
}
278298
return this.notModified;
279299
}
280300

301+
private String formatDate(long date) {
302+
SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT, Locale.US);
303+
dateFormat.setTimeZone(GMT);
304+
return dateFormat.format(new Date(date));
305+
}
306+
281307
public boolean isNotModified() {
282308
return this.notModified;
283309
}

‎spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ public interface WebRequest extends RequestAttributes {
194194
* Check whether the request qualifies as not modified given the
195195
* supplied {@code ETag} (entity tag) and last-modified timestamp,
196196
* as determined by the application.
197-
* <p>This will also transparently set the appropriate response headers,
197+
* <p>This will also transparently set the "Etag" and "Last-Modified" response headers,
198198
* for both the modified case and the not-modified case.
199199
* <p>Typical usage:
200200
* <pre class="code">

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

Lines changed: 56 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Arrays;
2121
import java.util.Date;
2222
import java.util.Locale;
23+
import java.util.TimeZone;
2324

2425
import org.junit.Before;
2526
import org.junit.Test;
@@ -42,6 +43,8 @@
4243
@RunWith(Parameterized.class)
4344
public class ServletWebRequestHttpMethodsTests {
4445

46+
private static final String CURRENT_TIME = "Wed, 09 Apr 2014 09:57:42 GMT";
47+
4548
private SimpleDateFormat dateFormat;
4649

4750
private MockHttpServletRequest servletRequest;
@@ -50,6 +53,8 @@ public class ServletWebRequestHttpMethodsTests {
5053

5154
private ServletWebRequest request;
5255

56+
private Date currentDate;
57+
5358
@Parameter
5459
public String method;
5560

@@ -63,33 +68,57 @@ static public Iterable<Object[]> safeMethods() {
6368

6469
@Before
6570
public void setUp() {
71+
currentDate = new Date();
6672
dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z", Locale.US);
73+
dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
6774
servletRequest = new MockHttpServletRequest(method, "http://example.org");
6875
servletResponse = new MockHttpServletResponse();
6976
request = new ServletWebRequest(servletRequest, servletResponse);
7077
}
7178

7279
@Test
73-
public void checkNotModifiedTimestamp() {
74-
long currentTime = new Date().getTime();
75-
servletRequest.addHeader("If-Modified-Since", currentTime);
80+
public void checkNotModifiedNon2xxStatus() {
81+
long epochTime = currentDate.getTime();
82+
servletRequest.addHeader("If-Modified-Since", epochTime);
83+
servletResponse.setStatus(304);
84+
85+
assertFalse(request.checkNotModified(epochTime));
86+
assertEquals(304, servletResponse.getStatus());
87+
assertNull(servletResponse.getHeader("Last-Modified"));
88+
}
89+
90+
@Test
91+
public void checkNotModifiedHeaderAlreadySet() {
92+
long epochTime = currentDate.getTime();
93+
servletRequest.addHeader("If-Modified-Since", epochTime);
94+
servletResponse.addHeader("Last-Modified", CURRENT_TIME);
95+
96+
assertFalse(request.checkNotModified(epochTime));
97+
assertEquals(200, servletResponse.getStatus());
98+
assertEquals(1, servletResponse.getHeaders("Last-Modified").size());
99+
assertEquals(CURRENT_TIME, servletResponse.getHeader("Last-Modified"));
100+
}
101+
102+
@Test
103+
public void checkNotModifiedTimestamp() throws Exception {
104+
long epochTime = currentDate.getTime();
105+
servletRequest.addHeader("If-Modified-Since", epochTime);
76106

77-
assertTrue(request.checkNotModified(currentTime));
107+
assertTrue(request.checkNotModified(epochTime));
78108

79109
assertEquals(304, servletResponse.getStatus());
80-
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
110+
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
81111
}
82112

83113
@Test
84114
public void checkModifiedTimestamp() {
85-
long currentTime = new Date().getTime();
86-
long oneMinuteAgo = currentTime - (1000 * 60);
115+
long oneMinuteAgo = currentDate.getTime() - (1000 * 60);
87116
servletRequest.addHeader("If-Modified-Since", oneMinuteAgo);
88117

89-
assertFalse(request.checkNotModified(currentTime));
118+
assertFalse(request.checkNotModified(currentDate.getTime()));
90119

91120
assertEquals(200, servletResponse.getStatus());
92-
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
121+
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
93122
}
94123

95124
@Test
@@ -154,44 +183,43 @@ public void checkNotModifiedWildcardETag() {
154183
public void checkNotModifiedETagAndTimestamp() {
155184
String eTag = "\"Foo\"";
156185
servletRequest.addHeader("If-None-Match", eTag);
157-
long currentTime = new Date().getTime();
158-
servletRequest.addHeader("If-Modified-Since", currentTime);
186+
servletRequest.addHeader("If-Modified-Since", currentDate.getTime());
159187

160-
assertTrue(request.checkNotModified(eTag, currentTime));
188+
assertTrue(request.checkNotModified(eTag, currentDate.getTime()));
161189

162190
assertEquals(304, servletResponse.getStatus());
163191
assertEquals(eTag, servletResponse.getHeader("ETag"));
164-
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
192+
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
165193
}
166194

167195
@Test
168196
public void checkNotModifiedETagAndModifiedTimestamp() {
169197
String eTag = "\"Foo\"";
170198
servletRequest.addHeader("If-None-Match", eTag);
171-
long currentTime = new Date().getTime();
172-
long oneMinuteAgo = currentTime - (1000 * 60);
199+
long currentEpoch = currentDate.getTime();
200+
long oneMinuteAgo = currentEpoch - (1000 * 60);
173201
servletRequest.addHeader("If-Modified-Since", oneMinuteAgo);
174202

175-
assertFalse(request.checkNotModified(eTag, currentTime));
203+
assertFalse(request.checkNotModified(eTag, currentEpoch));
176204

177205
assertEquals(200, servletResponse.getStatus());
178206
assertEquals(eTag, servletResponse.getHeader("ETag"));
179-
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
207+
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
180208
}
181209

182210
@Test
183-
public void checkModifiedETagAndNotModifiedTimestamp() {
211+
public void checkModifiedETagAndNotModifiedTimestamp() throws Exception {
184212
String currentETag = "\"Foo\"";
185213
String oldEtag = "\"Bar\"";
186214
servletRequest.addHeader("If-None-Match", oldEtag);
187-
long currentTime = new Date().getTime();
188-
servletRequest.addHeader("If-Modified-Since", currentTime);
215+
long epochTime = currentDate.getTime();
216+
servletRequest.addHeader("If-Modified-Since", epochTime);
189217

190-
assertFalse(request.checkNotModified(currentETag, currentTime));
218+
assertFalse(request.checkNotModified(currentETag, epochTime));
191219

192220
assertEquals(200, servletResponse.getStatus());
193221
assertEquals(currentETag, servletResponse.getHeader("ETag"));
194-
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
222+
assertEquals(dateFormat.format(currentDate), servletResponse.getHeader("Last-Modified"));
195223
}
196224

197225
@Test
@@ -231,26 +259,26 @@ public void checkNotModifiedMultipleETags() {
231259

232260
@Test
233261
public void checkNotModifiedTimestampWithLengthPart() throws Exception {
234-
long currentTime = dateFormat.parse("Wed, 09 Apr 2014 09:57:42 GMT").getTime();
262+
long epochTime = dateFormat.parse(CURRENT_TIME).getTime();
235263
servletRequest.setMethod("GET");
236264
servletRequest.addHeader("If-Modified-Since", "Wed, 09 Apr 2014 09:57:42 GMT; length=13774");
237265

238-
assertTrue(request.checkNotModified(currentTime));
266+
assertTrue(request.checkNotModified(epochTime));
239267

240268
assertEquals(304, servletResponse.getStatus());
241-
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
269+
assertEquals(CURRENT_TIME, servletResponse.getHeader("Last-Modified"));
242270
}
243271

244272
@Test
245273
public void checkModifiedTimestampWithLengthPart() throws Exception {
246-
long currentTime = dateFormat.parse("Wed, 09 Apr 2014 09:57:42 GMT").getTime();
274+
long epochTime = dateFormat.parse(CURRENT_TIME).getTime();
247275
servletRequest.setMethod("GET");
248276
servletRequest.addHeader("If-Modified-Since", "Wed, 08 Apr 2014 09:57:42 GMT; length=13774");
249277

250-
assertFalse(request.checkNotModified(currentTime));
278+
assertFalse(request.checkNotModified(epochTime));
251279

252280
assertEquals(200, servletResponse.getStatus());
253-
assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
281+
assertEquals(CURRENT_TIME, servletResponse.getHeader("Last-Modified"));
254282
}
255283

256284
}

‎spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
package org.springframework.web.servlet.support;
1818

19+
import java.text.SimpleDateFormat;
1920
import java.util.Arrays;
2021
import java.util.HashSet;
22+
import java.util.Locale;
2123
import java.util.Set;
24+
import java.util.TimeZone;
2225
import java.util.concurrent.TimeUnit;
2326

2427
import javax.servlet.ServletException;
@@ -91,6 +94,8 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport {
9194

9295
private boolean usePreviousHttpCachingBehavior = false;
9396

97+
private final SimpleDateFormat dateFormat;
98+
9499
private CacheControl cacheControl;
95100

96101

@@ -115,6 +120,8 @@ public WebContentGenerator(boolean restrictDefaultSupportedMethods) {
115120
this.supportedMethods.add(METHOD_HEAD);
116121
this.supportedMethods.add(METHOD_POST);
117122
}
123+
dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z", Locale.US);
124+
dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
118125
}
119126

120127
/**
@@ -123,6 +130,8 @@ public WebContentGenerator(boolean restrictDefaultSupportedMethods) {
123130
*/
124131
public WebContentGenerator(String... supportedMethods) {
125132
this.supportedMethods = new HashSet<String>(Arrays.asList(supportedMethods));
133+
dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z", Locale.US);
134+
dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
126135
}
127136

128137

@@ -404,7 +413,7 @@ else if (this.cacheSeconds == 0) {
404413
protected final void cacheForSeconds(HttpServletResponse response, int seconds, boolean mustRevalidate) {
405414
if (this.useExpiresHeader) {
406415
// HTTP 1.0 header
407-
response.setDateHeader(HEADER_EXPIRES, System.currentTimeMillis() + seconds * 1000L);
416+
response.setHeader(HEADER_EXPIRES, dateFormat.format(System.currentTimeMillis() + seconds * 1000L));
408417
}
409418
if (this.useCacheControlHeader) {
410419
// HTTP 1.1 header
@@ -424,7 +433,7 @@ protected final void preventCaching(HttpServletResponse response) {
424433
response.setHeader(HEADER_PRAGMA, "no-cache");
425434
if (this.useExpiresHeader) {
426435
// HTTP 1.0 header
427-
response.setDateHeader(HEADER_EXPIRES, 1L);
436+
response.setHeader(HEADER_EXPIRES, dateFormat.format(System.currentTimeMillis()));
428437
}
429438
if (this.useCacheControlHeader) {
430439
// HTTP 1.1 header: "no-cache" is the standard value,

‎spring-webmvc/src/test/java/org/springframework/web/servlet/ComplexWebApplicationContext.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ public void doSomething(HttpServletRequest request) throws ServletException, Ill
460460

461461
@Override
462462
public long lastModified() {
463-
return 99;
463+
return 1427846401000L;
464464
}
465465
}
466466

‎spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -177,7 +177,7 @@ public void testLocaleRequest() throws Exception {
177177
MockHttpServletResponse response = new MockHttpServletResponse();
178178
simpleDispatcherServlet.service(request, response);
179179
assertTrue("Not forwarded", response.getForwardedUrl() == null);
180-
assertEquals("98", response.getHeader("Last-Modified"));
180+
assertEquals("Wed, 01 Apr 2015 00:00:00 GMT", response.getHeader("Last-Modified"));
181181
}
182182

183183
public void testUnknownRequest() throws Exception {
@@ -205,7 +205,7 @@ public void testAnotherLocaleRequest() throws Exception {
205205
assertTrue(request.getAttribute("test3") != null);
206206
assertTrue(request.getAttribute("test3x") != null);
207207
assertTrue(request.getAttribute("test3y") != null);
208-
assertEquals("99", response.getHeader("Last-Modified"));
208+
assertEquals("Wed, 01 Apr 2015 00:00:01 GMT", response.getHeader("Last-Modified"));
209209
}
210210

211211
public void testExistingMultipartRequest() throws Exception {

0 commit comments

Comments
 (0)