Skip to content

Commit 6501bc5

Browse files
bclozelbclozel
committed
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. Code duplication has been removed from `HttpEntityMethodProcessor` but the Servlet 2.5 baseline is still respected. Issue: SPR-14659 Cherry-picked from: cc5300c
1 parent f3dae0c commit 6501bc5

File tree

4 files changed

+170
-206
lines changed

4 files changed

+170
-206
lines changed

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

Lines changed: 143 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,18 @@
1717
package org.springframework.web.context.request;
1818

1919
import java.security.Principal;
20-
import java.util.Date;
20+
import java.text.ParseException;
21+
import java.text.SimpleDateFormat;
22+
import java.util.Arrays;
23+
import java.util.Enumeration;
2124
import java.util.Iterator;
25+
import java.util.List;
2226
import java.util.Locale;
2327
import java.util.Map;
28+
import java.util.TimeZone;
2429
import java.util.regex.Matcher;
2530
import java.util.regex.Pattern;
31+
2632
import javax.servlet.http.HttpServletRequest;
2733
import javax.servlet.http.HttpServletResponse;
2834
import javax.servlet.http.HttpSession;
@@ -45,32 +51,35 @@
4551
*/
4652
public class ServletWebRequest extends ServletRequestAttributes implements NativeWebRequest {
4753

48-
private static final String HEADER_ETAG = "ETag";
49-
50-
private static final String HEADER_IF_MODIFIED_SINCE = "If-Modified-Since";
51-
52-
private static final String HEADER_IF_UNMODIFIED_SINCE = "If-Unmodified-Since";
53-
54-
private static final String HEADER_IF_NONE_MATCH = "If-None-Match";
55-
56-
private static final String HEADER_LAST_MODIFIED = "Last-Modified";
54+
private static final String ETAG = "ETag";
5755

58-
private static final String METHOD_GET = "GET";
56+
private static final String IF_MODIFIED_SINCE = "If-Modified-Since";
5957

60-
private static final String METHOD_HEAD = "HEAD";
58+
private static final String IF_UNMODIFIED_SINCE = "If-Unmodified-Since";
6159

62-
private static final String METHOD_POST = "POST";
60+
private static final String IF_NONE_MATCH = "If-None-Match";
6361

64-
private static final String METHOD_PUT = "PUT";
62+
private static final String LAST_MODIFIED = "Last-Modified";
6563

66-
private static final String METHOD_DELETE = "DELETE";
64+
private static final List<String> SAFE_METHODS = Arrays.asList("GET", "HEAD");
6765

6866
/**
6967
* Pattern matching ETag multiple field values in headers such as "If-Match", "If-None-Match"
7068
* @see <a href="https://tools.ietf.org/html/rfc7232#section-2.3">Section 2.3 of RFC 7232</a>
7169
*/
7270
private static final Pattern ETAG_HEADER_VALUE_PATTERN = Pattern.compile("\\*|\\s*((W\\/)?(\"[^\"]*\"))\\s*,?");
7371

72+
/**
73+
* Date formats as specified in the HTTP RFC
74+
* @see <a href="https://tools.ietf.org/html/rfc7231#section-7.1.1.1">Section 7.1.1.1 of RFC 7231</a>
75+
*/
76+
private static final String[] DATE_FORMATS = new String[] {
77+
"EEE, dd MMM yyyy HH:mm:ss zzz",
78+
"EEE, dd-MMM-yy HH:mm:ss zzz",
79+
"EEE MMM dd HH:mm:ss yyyy"
80+
};
81+
82+
private static TimeZone GMT = TimeZone.getTimeZone("GMT");
7483

7584
/** Checking for Servlet 3.0+ HttpServletResponse.getHeader(String) */
7685
private static final boolean servlet3Present =
@@ -194,103 +203,62 @@ public boolean isSecure() {
194203

195204
@Override
196205
public boolean checkNotModified(long lastModifiedTimestamp) {
197-
HttpServletResponse response = getResponse();
198-
if (lastModifiedTimestamp >= 0 && !this.notModified) {
199-
if (isCompatibleWithConditionalRequests(response)) {
200-
this.notModified = isTimestampNotModified(lastModifiedTimestamp);
201-
if (response != null) {
202-
if (supportsNotModifiedStatus()) {
203-
if (this.notModified) {
204-
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
205-
}
206-
if (isHeaderAbsent(response, HEADER_LAST_MODIFIED)) {
207-
response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
208-
}
209-
}
210-
else if (supportsConditionalUpdate()) {
211-
if (this.notModified) {
212-
response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED);
213-
}
214-
}
215-
}
216-
}
217-
}
218-
return this.notModified;
206+
return checkNotModified(null, lastModifiedTimestamp);
219207
}
220208

221209
@Override
222210
public boolean checkNotModified(String etag) {
223-
HttpServletResponse response = getResponse();
224-
if (StringUtils.hasLength(etag) && !this.notModified) {
225-
if (isCompatibleWithConditionalRequests(response)) {
226-
etag = addEtagPadding(etag);
227-
if (hasRequestHeader(HEADER_IF_NONE_MATCH)) {
228-
this.notModified = isEtagNotModified(etag);
229-
}
230-
if (response != null) {
231-
if (this.notModified && supportsNotModifiedStatus()) {
232-
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
233-
}
234-
if (isHeaderAbsent(response, HEADER_ETAG)) {
235-
response.setHeader(HEADER_ETAG, etag);
236-
}
237-
}
238-
}
239-
}
240-
return this.notModified;
211+
return checkNotModified(etag, -1);
241212
}
242213

243214
@Override
244215
public boolean checkNotModified(String etag, long lastModifiedTimestamp) {
245216
HttpServletResponse response = getResponse();
246-
if (StringUtils.hasLength(etag) && !this.notModified) {
247-
if (isCompatibleWithConditionalRequests(response)) {
248-
etag = addEtagPadding(etag);
249-
if (hasRequestHeader(HEADER_IF_NONE_MATCH)) {
250-
this.notModified = isEtagNotModified(etag);
251-
}
252-
else if (hasRequestHeader(HEADER_IF_MODIFIED_SINCE)) {
253-
this.notModified = isTimestampNotModified(lastModifiedTimestamp);
254-
}
255-
if (response != null) {
256-
if (supportsNotModifiedStatus()) {
257-
if (this.notModified) {
258-
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
259-
}
260-
if (isHeaderAbsent(response, HEADER_ETAG)) {
261-
response.setHeader(HEADER_ETAG, etag);
262-
}
263-
if (isHeaderAbsent(response, HEADER_LAST_MODIFIED)) {
264-
response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
265-
}
266-
}
267-
else if (supportsConditionalUpdate()) {
268-
if (this.notModified) {
269-
response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED);
270-
}
271-
}
272-
}
217+
if (this.notModified || !isStatusOK(response)) {
218+
return this.notModified;
219+
}
220+
221+
// Evaluate conditions in order of precedence.
222+
// See https://tools.ietf.org/html/rfc7232#section-6
223+
224+
if (validateIfUnmodifiedSince(lastModifiedTimestamp)) {
225+
if (this.notModified) {
226+
response.setStatus(HttpStatus.PRECONDITION_FAILED.value());
273227
}
228+
return this.notModified;
274229
}
275-
return this.notModified;
276-
}
277230

278-
public boolean isNotModified() {
279-
return this.notModified;
280-
}
231+
boolean validated = validateIfNoneMatch(etag);
281232

233+
if (!validated) {
234+
validateIfModifiedSince(lastModifiedTimestamp);
235+
}
282236

283-
private boolean isCompatibleWithConditionalRequests(HttpServletResponse response) {
284-
try {
285-
if (response == null || !servlet3Present) {
286-
// Can't check response.getStatus() - let's assume we're good
287-
return true;
237+
// Update response
238+
239+
boolean isHttpGetOrHead = SAFE_METHODS.contains(getRequest().getMethod());
240+
if (this.notModified) {
241+
response.setStatus(isHttpGetOrHead ?
242+
HttpStatus.NOT_MODIFIED.value() : HttpStatus.PRECONDITION_FAILED.value());
243+
}
244+
if (isHttpGetOrHead) {
245+
if(lastModifiedTimestamp > 0 && isHeaderAbsent(response, LAST_MODIFIED)) {
246+
response.setDateHeader(LAST_MODIFIED, lastModifiedTimestamp);
247+
}
248+
if (StringUtils.hasLength(etag) && isHeaderAbsent(response, ETAG)) {
249+
response.setHeader(ETAG, padEtagIfNecessary(etag));
288250
}
289-
return HttpStatus.valueOf(response.getStatus()).is2xxSuccessful();
290251
}
291-
catch (IllegalArgumentException ex) {
252+
253+
return this.notModified;
254+
}
255+
256+
private boolean isStatusOK(HttpServletResponse response) {
257+
if (response == null || !servlet3Present) {
258+
// Can't check response.getStatus() - let's assume we're good
292259
return true;
293260
}
261+
return HttpStatus.OK.value() == 200;
294262
}
295263

296264
private boolean isHeaderAbsent(HttpServletResponse response, String header) {
@@ -301,34 +269,78 @@ private boolean isHeaderAbsent(HttpServletResponse response, String header) {
301269
return (response.getHeader(header) == null);
302270
}
303271

304-
private boolean hasRequestHeader(String headerName) {
305-
return StringUtils.hasLength(getHeader(headerName));
272+
private boolean validateIfUnmodifiedSince(long lastModifiedTimestamp) {
273+
if (lastModifiedTimestamp < 0) {
274+
return false;
275+
}
276+
long ifUnmodifiedSince = parseDateHeader(IF_UNMODIFIED_SINCE);
277+
if (ifUnmodifiedSince == -1) {
278+
return false;
279+
}
280+
// We will perform this validation...
281+
this.notModified = (ifUnmodifiedSince < (lastModifiedTimestamp / 1000 * 1000));
282+
return true;
306283
}
307284

308-
private boolean supportsNotModifiedStatus() {
309-
String method = getRequest().getMethod();
310-
return (METHOD_GET.equals(method) || METHOD_HEAD.equals(method));
285+
private boolean validateIfNoneMatch(String etag) {
286+
if (!StringUtils.hasLength(etag)) {
287+
return false;
288+
}
289+
Enumeration<String> ifNoneMatch;
290+
try {
291+
ifNoneMatch = getRequest().getHeaders(IF_NONE_MATCH);
292+
}
293+
catch (IllegalArgumentException ex) {
294+
return false;
295+
}
296+
if (!ifNoneMatch.hasMoreElements()) {
297+
return false;
298+
}
299+
// We will perform this validation...
300+
etag = padEtagIfNecessary(etag);
301+
while (ifNoneMatch.hasMoreElements()) {
302+
String clientETags = ifNoneMatch.nextElement();
303+
304+
Matcher eTagMatcher = ETAG_HEADER_VALUE_PATTERN.matcher(clientETags);
305+
// Compare weak/strong ETags as per https://tools.ietf.org/html/rfc7232#section-2.3
306+
while (eTagMatcher.find()) {
307+
if (StringUtils.hasLength(eTagMatcher.group())
308+
&& etag.replaceFirst("^W/", "").equals(eTagMatcher.group(3))) {
309+
this.notModified = true;
310+
break;
311+
}
312+
}
313+
}
314+
return true;
311315
}
312316

313-
private boolean supportsConditionalUpdate() {
314-
String method = getRequest().getMethod();
315-
return (METHOD_POST.equals(method) || METHOD_PUT.equals(method) || METHOD_DELETE.equals(method))
316-
&& hasRequestHeader(HEADER_IF_UNMODIFIED_SINCE);
317+
private String padEtagIfNecessary(String etag) {
318+
if (!StringUtils.hasLength(etag)) {
319+
return etag;
320+
}
321+
if ((etag.startsWith("\"") || etag.startsWith("W/\"")) && etag.endsWith("\"")) {
322+
return etag;
323+
}
324+
return "\"" + etag + "\"";
317325
}
318326

319-
private boolean isTimestampNotModified(long lastModifiedTimestamp) {
320-
long ifModifiedSince = parseDateHeader(HEADER_IF_MODIFIED_SINCE);
321-
if (ifModifiedSince != -1) {
322-
return (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000));
327+
private boolean validateIfModifiedSince(long lastModifiedTimestamp) {
328+
if (lastModifiedTimestamp < 0) {
329+
return false;
323330
}
324-
long ifUnmodifiedSince = parseDateHeader(HEADER_IF_UNMODIFIED_SINCE);
325-
if (ifUnmodifiedSince != -1) {
326-
return (ifUnmodifiedSince < (lastModifiedTimestamp / 1000 * 1000));
331+
long ifModifiedSince = parseDateHeader(IF_MODIFIED_SINCE);
332+
if (ifModifiedSince == -1) {
333+
return false;
327334
}
328-
return false;
335+
// We will perform this validation...
336+
this.notModified = ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000);
337+
return true;
338+
}
339+
340+
public boolean isNotModified() {
341+
return this.notModified;
329342
}
330343

331-
@SuppressWarnings("deprecation")
332344
private long parseDateHeader(String headerName) {
333345
long dateValue = -1;
334346
try {
@@ -340,36 +352,32 @@ private long parseDateHeader(String headerName) {
340352
int separatorIndex = headerValue.indexOf(';');
341353
if (separatorIndex != -1) {
342354
String datePart = headerValue.substring(0, separatorIndex);
343-
try {
344-
dateValue = Date.parse(datePart);
345-
}
346-
catch (IllegalArgumentException ex2) {
347-
// Giving up
348-
}
355+
dateValue = parseDateValue(datePart);
349356
}
350357
}
351358
return dateValue;
352359
}
353360

354-
private boolean isEtagNotModified(String etag) {
355-
String ifNoneMatch = getHeader(HEADER_IF_NONE_MATCH);
356-
// compare weak/strong ETag as per https://tools.ietf.org/html/rfc7232#section-2.3
357-
String serverETag = etag.replaceFirst("^W/", "");
358-
Matcher eTagMatcher = ETAG_HEADER_VALUE_PATTERN.matcher(ifNoneMatch);
359-
while (eTagMatcher.find()) {
360-
if ("*".equals(eTagMatcher.group())
361-
|| serverETag.equals(eTagMatcher.group(3))) {
362-
return true;
363-
}
361+
private long parseDateValue(String headerValue) {
362+
if (headerValue == null) {
363+
// No header value sent at all
364+
return -1;
364365
}
365-
return false;
366-
}
367-
368-
private String addEtagPadding(String etag) {
369-
if (!(etag.startsWith("\"") || etag.startsWith("W/\"")) || !etag.endsWith("\"")) {
370-
etag = "\"" + etag + "\"";
366+
if (headerValue.length() >= 3) {
367+
// Short "0" or "-1" like values are never valid HTTP date headers...
368+
// Let's only bother with SimpleDateFormat parsing for long enough values.
369+
for (String dateFormat : DATE_FORMATS) {
370+
SimpleDateFormat simpleDateFormat = new SimpleDateFormat(dateFormat, Locale.US);
371+
simpleDateFormat.setTimeZone(GMT);
372+
try {
373+
return simpleDateFormat.parse(headerValue).getTime();
374+
}
375+
catch (ParseException ex) {
376+
// ignore
377+
}
378+
}
371379
}
372-
return etag;
380+
return -1;
373381
}
374382

375383
@Override

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,7 @@ public void checkNotModifiedInvalidStatus() {
9494
servletRequest.addHeader("If-Modified-Since", epochTime);
9595
servletResponse.setStatus(0);
9696

97-
assertTrue(request.checkNotModified(epochTime));
98-
assertEquals(304, servletResponse.getStatus());
99-
assertEquals(dateFormat.format(epochTime), servletResponse.getHeader("Last-Modified"));
97+
assertFalse(request.checkNotModified(epochTime));
10098
}
10199

102100
@Test // SPR-14559
@@ -202,13 +200,13 @@ public void checkModifiedUnpaddedETag() {
202200
}
203201

204202
@Test
205-
public void checkNotModifiedWildcardETag() {
203+
public void checkNotModifiedWildcardIsIgnored() {
206204
String eTag = "\"Foo\"";
207205
servletRequest.addHeader("If-None-Match", "*");
208206

209-
assertTrue(request.checkNotModified(eTag));
207+
assertFalse(request.checkNotModified(eTag));
210208

211-
assertEquals(304, servletResponse.getStatus());
209+
assertEquals(200, servletResponse.getStatus());
212210
assertEquals(eTag, servletResponse.getHeader("ETag"));
213211
}
214212

0 commit comments

Comments
 (0)