checkNotModified leniently handles IE-10-style If-Modified-Since values and...

checkNotModified leniently handles IE-10-style If-Modified-Since values and silently proceeds if header value cannot be parsed at all

Issue: SPR-11727

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

@@ -17,6 +17,7 @@
 package org.springframework.web.context.request;
 
 import java.security.Principal;
+import java.util.Date;
 import java.util.Iterator;
 import java.util.Locale;
 import java.util.Map;
@@ -103,6 +104,14 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 	}
 
 
+	/**
+	 * Return the HTTP method of the request.
+	 * @since 4.0.2
+	 */
+	public HttpMethod getHttpMethod() {
+		return HttpMethod.valueOf(getRequest().getMethod().trim().toUpperCase());
+	}
+
 	@Override
 	public String getHeader(String headerName) {
 		return getRequest().getHeader(headerName);
@@ -170,10 +179,28 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 	}
 
 	@Override
+	@SuppressWarnings("deprecation")
 	public boolean checkNotModified(long lastModifiedTimestamp) {
 		if (lastModifiedTimestamp >= 0 && !this.notModified &&
 				(this.response == null || !this.response.containsHeader(HEADER_LAST_MODIFIED))) {
-			long ifModifiedSince = getRequest().getDateHeader(HEADER_IF_MODIFIED_SINCE);
+			long ifModifiedSince = -1;
+			try {
+				ifModifiedSince = getRequest().getDateHeader(HEADER_IF_MODIFIED_SINCE);
+			}
+			catch (IllegalArgumentException ex) {
+				String headerValue = getRequest().getHeader(HEADER_IF_MODIFIED_SINCE);
+				// 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);
+					}
+					catch (IllegalArgumentException ex2) {
+						// Giving up
+					}
+				}
+			}
 			this.notModified = (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000));
 			if (this.response != null) {
 				if (this.notModified && supportsNotModifiedStatus()) {
@@ -188,17 +215,17 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 	}
 
 	@Override
-	public boolean checkNotModified(String eTag) {
-		if (StringUtils.hasLength(eTag) && !this.notModified &&
+	public boolean checkNotModified(String etag) {
+		if (StringUtils.hasLength(etag) && !this.notModified &&
 				(this.response == null || !this.response.containsHeader(HEADER_ETAG))) {
 			String ifNoneMatch = getRequest().getHeader(HEADER_IF_NONE_MATCH);
-			this.notModified = eTag.equals(ifNoneMatch);
+			this.notModified = etag.equals(ifNoneMatch);
 			if (this.response != null) {
 				if (this.notModified && supportsNotModifiedStatus()) {
 					this.response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
 				}
 				else {
-					this.response.setHeader(HEADER_ETAG, eTag);
+					this.response.setHeader(HEADER_ETAG, etag);
 				}
 			}
 		}
@@ -236,14 +263,6 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 		return sb.toString();
 	}
 
-	/**
-	 * Return the HTTP method of the request.
-	 * @since 4.0.2
-	 */
-	public HttpMethod getHttpMethod() {
-		return HttpMethod.valueOf(getRequest().getMethod().trim().toUpperCase());
-	}
-
 
 	@Override
 	public String toString() {

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

 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -142,8 +142,11 @@ public interface WebRequest extends RequestAttributes {
 	 *   return "myViewName";
 	 * }</pre>
 	 * <p><strong>Note:</strong> that you typically want to use either
-	 * this {@link #checkNotModified(long)} method; or
+	 * this {@code #checkNotModified(long)} method; or
 	 * {@link #checkNotModified(String)}, but not both.
+	 * <p>If the "If-Modified-Since" header is set but cannot be parsed
+	 * to a date value, this method will ignore the header and proceed
+	 * with setting the last-modified timestamp on the response.
 	 * @param lastModifiedTimestamp the last-modified timestamp that
 	 * the application determined for the underlying resource
 	 * @return whether the request qualifies as not modified,
@@ -170,16 +173,16 @@ public interface WebRequest extends RequestAttributes {
 	 *   return "myViewName";
 	 * }</pre>
 	 * <p><strong>Note:</strong> that you typically want to use either
-	 * this {@link #checkNotModified(String)} method; or
+	 * this {@code #checkNotModified(String)} method; or
 	 * {@link #checkNotModified(long)}, but not both.
-	 * @param eTag the entity tag that the application determined
+	 * @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
 	 */
-	boolean checkNotModified(String eTag);
+	boolean checkNotModified(String etag);
 
 	/**
 	 * Get a short description of this request,

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

@@ -19,7 +19,6 @@ package org.springframework.web.context.request;
 import java.util.Date;
 import java.util.Locale;
 import java.util.Map;
-
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
@@ -118,75 +117,90 @@ public class ServletWebRequestTests {
 	}
 
 	@Test
-	public void checkNotModifiedTimeStampForGET() {
+	public void checkNotModifiedTimestampForGET() {
 		long currentTime = new Date().getTime();
 		servletRequest.setMethod("GET");
 		servletRequest.addHeader("If-Modified-Since", currentTime);
 
-		request.checkNotModified(currentTime);
-
+		assertTrue(request.checkNotModified(currentTime));
 		assertEquals(304, servletResponse.getStatus());
 	}
 
 	@Test
-	public void checkModifiedTimeStampForGET() {
+	public void checkModifiedTimestampForGET() {
 		long currentTime = new Date().getTime();
 		long oneMinuteAgo  = currentTime - (1000 * 60);
 		servletRequest.setMethod("GET");
 		servletRequest.addHeader("If-Modified-Since", oneMinuteAgo);
 
-		request.checkNotModified(currentTime);
+		assertFalse(request.checkNotModified(currentTime));
+		assertEquals(200, servletResponse.getStatus());
+		assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
+	}
 
+	@Test
+	public void checkNotModifiedTimestampForHEAD() {
+		long currentTime = new Date().getTime();
+		servletRequest.setMethod("HEAD");
+		servletRequest.addHeader("If-Modified-Since", currentTime);
+
+		assertTrue(request.checkNotModified(currentTime));
+		assertEquals(304, servletResponse.getStatus());
+	}
+
+	@Test
+	public void checkModifiedTimestampForHEAD() {
+		long currentTime = new Date().getTime();
+		long oneMinuteAgo  = currentTime - (1000 * 60);
+		servletRequest.setMethod("HEAD");
+		servletRequest.addHeader("If-Modified-Since", oneMinuteAgo);
+
+		assertFalse(request.checkNotModified(currentTime));
 		assertEquals(200, servletResponse.getStatus());
 		assertEquals(""+currentTime, servletResponse.getHeader("Last-Modified"));
 	}
 
 	@Test
-	public void checkNotModifiedETagForGET() {
-		String eTag = "\"Foo\"";
+	public void checkNotModifiedTimestampWithLengthPart() {
+		long currentTime = Date.parse("Wed, 09 Apr 2014 09:57:42 GMT");
 		servletRequest.setMethod("GET");
-		servletRequest.addHeader("If-None-Match", eTag );
-
-		request.checkNotModified(eTag);
+		servletRequest.addHeader("If-Modified-Since", "Wed, 09 Apr 2014 09:57:42 GMT; length=13774");
 
+		assertTrue(request.checkNotModified(currentTime));
 		assertEquals(304, servletResponse.getStatus());
 	}
 
 	@Test
-	public void checkModifiedETagForGET() {
-		String currentETag = "\"Foo\"";
-		String oldEtag = "Bar";
+	public void checkModifiedTimestampWithLengthPart() {
+		long currentTime = Date.parse("Wed, 09 Apr 2014 09:57:42 GMT");
 		servletRequest.setMethod("GET");
-		servletRequest.addHeader("If-None-Match", oldEtag);
-
-		request.checkNotModified(currentETag);
+		servletRequest.addHeader("If-Modified-Since", "Wed, 08 Apr 2014 09:57:42 GMT; length=13774");
 
+		assertFalse(request.checkNotModified(currentTime));
 		assertEquals(200, servletResponse.getStatus());
-		assertEquals(currentETag, servletResponse.getHeader("ETag"));
+		assertEquals("" + currentTime, servletResponse.getHeader("Last-Modified"));
 	}
 
 	@Test
-	public void checkNotModifiedTimeStampForHEAD() {
-		long currentTime = new Date().getTime();
-		servletRequest.setMethod("HEAD");
-		servletRequest.addHeader("If-Modified-Since", currentTime);
-
-		request.checkNotModified(currentTime);
+	public void checkNotModifiedETagForGET() {
+		String eTag = "\"Foo\"";
+		servletRequest.setMethod("GET");
+		servletRequest.addHeader("If-None-Match", eTag );
 
+		assertTrue(request.checkNotModified(eTag));
 		assertEquals(304, servletResponse.getStatus());
 	}
 
 	@Test
-	public void checkModifiedTimeStampForHEAD() {
-		long currentTime = new Date().getTime();
-		long oneMinuteAgo  = currentTime - (1000 * 60);
-		servletRequest.setMethod("HEAD");
-		servletRequest.addHeader("If-Modified-Since", oneMinuteAgo);
-
-		request.checkNotModified(currentTime);
+	public void checkModifiedETagForGET() {
+		String currentETag = "\"Foo\"";
+		String oldEtag = "Bar";
+		servletRequest.setMethod("GET");
+		servletRequest.addHeader("If-None-Match", oldEtag);
 
+		assertFalse(request.checkNotModified(currentETag));
 		assertEquals(200, servletResponse.getStatus());
-		assertEquals(""+currentTime, servletResponse.getHeader("Last-Modified"));
+		assertEquals(currentETag, servletResponse.getHeader("ETag"));
 	}
 
 	@Test
@@ -195,8 +209,7 @@ public class ServletWebRequestTests {
 		servletRequest.setMethod("HEAD");
 		servletRequest.addHeader("If-None-Match", eTag );
 
-		request.checkNotModified(eTag);
-
+		assertTrue(request.checkNotModified(eTag));
 		assertEquals(304, servletResponse.getStatus());
 	}
 
@@ -207,8 +220,7 @@ public class ServletWebRequestTests {
 		servletRequest.setMethod("HEAD");
 		servletRequest.addHeader("If-None-Match", oldEtag);
 
-		request.checkNotModified(currentETag);
-
+		assertFalse(request.checkNotModified(currentETag));
 		assertEquals(200, servletResponse.getStatus());
 		assertEquals(currentETag, servletResponse.getHeader("ETag"));
 	}