ServletWebRequest.checkNotModified avoids HttpServletResponse.getHeader calls on Servlet 2.5

Includes a revision for consistent and defensive Servlet 3.0 method calls across Spring's web abstraction (in particular, also working in debug mode where method references may get resolved early, so ternary expressions are to be avoided).

Issue: SPR-13420

spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -39,6 +39,7 @@ import org.springframework.util.CollectionUtils;
  */
 public class ServletServerHttpResponse implements ServerHttpResponse {
 
+	/** Checking for Servlet 3.0+ HttpServletResponse.getHeader(String) */
 	private static final boolean servlet3Present =
 			ClassUtils.hasMethod(HttpServletResponse.class, "getHeader", String.class);
 
@@ -57,7 +58,7 @@ public class ServletServerHttpResponse implements ServerHttpResponse {
 	 * @param servletResponse the servlet response
 	 */
 	public ServletServerHttpResponse(HttpServletResponse servletResponse) {
-		Assert.notNull(servletResponse, "'servletResponse' must not be null");
+		Assert.notNull(servletResponse, "HttpServletResponse must not be null");
 		this.servletResponse = servletResponse;
 		this.headers = (servlet3Present ? new ServletResponseHttpHeaders() : new HttpHeaders());
 	}

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

@@ -56,9 +56,9 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 	private static final String METHOD_HEAD = "HEAD";
 
 
-	/** Checking for Servlet 3.0+ HttpServletResponse.getStatus() */
-	private static final boolean responseGetStatusAvailable =
-			ClassUtils.hasMethod(HttpServletResponse.class, "getStatus");
+	/** Checking for Servlet 3.0+ HttpServletResponse.getHeader(String) */
+	private static final boolean servlet3Present =
+			ClassUtils.hasMethod(HttpServletResponse.class, "getHeader", String.class);
 
 	private boolean notModified = false;
 
@@ -80,6 +80,7 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 		super(request, response);
 	}
 
+
 	@Override
 	public Object getNativeRequest() {
 		return getRequest();
@@ -174,6 +175,7 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 		return getRequest().isSecure();
 	}
 
+
 	@Override
 	public boolean checkNotModified(long lastModifiedTimestamp) {
 		HttpServletResponse response = getResponse();
@@ -184,7 +186,7 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 					if (this.notModified && supportsNotModifiedStatus()) {
 						response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
 					}
-					if (response.getHeader(HEADER_LAST_MODIFIED) == null) {
+					if (isHeaderAbsent(response, HEADER_LAST_MODIFIED)) {
 						response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
 					}
 				}
@@ -193,74 +195,108 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 		return this.notModified;
 	}
 
-	@SuppressWarnings("deprecation")
-	private boolean isTimestampNotModified(long lastModifiedTimestamp) {
-		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
+	@Override
+	public boolean checkNotModified(String etag) {
+		HttpServletResponse response = getResponse();
+		if (StringUtils.hasLength(etag) && !this.notModified) {
+			if (isCompatibleWithConditionalRequests(response)) {
+				etag = addEtagPadding(etag);
+				this.notModified = isEtagNotModified(etag);
+				if (response != null) {
+					if (this.notModified && supportsNotModifiedStatus()) {
+						response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
+					}
+					if (isHeaderAbsent(response, HEADER_ETAG)) {
+						response.setHeader(HEADER_ETAG, etag);
+					}
 				}
 			}
 		}
-		return (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000));
+		return this.notModified;
 	}
 
 	@Override
-	public boolean checkNotModified(String etag) {
+	public boolean checkNotModified(String etag, long lastModifiedTimestamp) {
 		HttpServletResponse response = getResponse();
 		if (StringUtils.hasLength(etag) && !this.notModified) {
 			if (isCompatibleWithConditionalRequests(response)) {
 				etag = addEtagPadding(etag);
-				this.notModified = isETagNotModified(etag);
+				this.notModified = isEtagNotModified(etag) && isTimestampNotModified(lastModifiedTimestamp);
 				if (response != null) {
 					if (this.notModified && supportsNotModifiedStatus()) {
 						response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
 					}
-					if (response.getHeader(HEADER_ETAG) == null) {
+					if (isHeaderAbsent(response, HEADER_ETAG)) {
 						response.setHeader(HEADER_ETAG, etag);
 					}
+					if (isHeaderAbsent(response, HEADER_LAST_MODIFIED)) {
+						response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
+					}
 				}
 			}
 		}
 		return this.notModified;
 	}
 
+	public boolean isNotModified() {
+		return this.notModified;
+	}
+
+
 	private boolean isCompatibleWithConditionalRequests(HttpServletResponse response) {
-		if (response == null || !responseGetStatusAvailable) {
+		if (response == null || !servlet3Present) {
 			// Can't check response.getStatus() - let's assume we're good
 			return true;
 		}
 		return HttpStatus.valueOf(response.getStatus()).is2xxSuccessful();
 	}
 
-	private String addEtagPadding(String etag) {
-		if (!(etag.startsWith("\"") || etag.startsWith("W/\"")) || !etag.endsWith("\"")) {
-			etag = "\"" + etag + "\"";
+	private boolean isHeaderAbsent(HttpServletResponse response, String header) {
+		if (response == null || !servlet3Present) {
+			// Can't check response.getHeader(header) - let's assume it's not set
+			return true;
 		}
-		return etag;
+		return (response.getHeader(header) == null);
+	}
+
+	private boolean supportsNotModifiedStatus() {
+		String method = getRequest().getMethod();
+		return (METHOD_GET.equals(method) || METHOD_HEAD.equals(method));
 	}
 
-	private boolean isETagNotModified(String etag) {
+	@SuppressWarnings("deprecation")
+	private boolean isTimestampNotModified(long lastModifiedTimestamp) {
+		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
+				}
+			}
+		}
+		return (ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000));
+	}
+
+	private boolean isEtagNotModified(String etag) {
 		if (StringUtils.hasLength(etag)) {
 			String ifNoneMatch = getRequest().getHeader(HEADER_IF_NONE_MATCH);
 			if (StringUtils.hasLength(ifNoneMatch)) {
-				String[] clientETags = StringUtils.delimitedListToStringArray(ifNoneMatch, ",", " ");
-				for (String clientETag : clientETags) {
-					// 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("*"))) {
+				String[] clientEtags = StringUtils.delimitedListToStringArray(ifNoneMatch, ",", " ");
+				for (String clientEtag : clientEtags) {
+					// compare weak/strong ETag as per https://tools.ietf.org/html/rfc7232#section-2.3
+					if (StringUtils.hasLength(clientEtag) &&
+							(clientEtag.replaceFirst("^W/", "").equals(etag.replaceFirst("^W/", "")) ||
+									clientEtag.equals("*"))) {
 						return true;
 					}
 				}
@@ -269,37 +305,13 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 		return false;
 	}
 
-	private boolean supportsNotModifiedStatus() {
-		String method = getRequest().getMethod();
-		return (METHOD_GET.equals(method) || METHOD_HEAD.equals(method));
-	}
-
-	@Override
-	public boolean checkNotModified(String etag, long lastModifiedTimestamp) {
-		HttpServletResponse response = getResponse();
-		if (StringUtils.hasLength(etag) && !this.notModified) {
-			if (isCompatibleWithConditionalRequests(response)) {
-				etag = addEtagPadding(etag);
-				this.notModified = isETagNotModified(etag) && isTimestampNotModified(lastModifiedTimestamp);
-				if (response != null) {
-					if (this.notModified && supportsNotModifiedStatus()) {
-						response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
-					}
-					if (response.getHeader(HEADER_ETAG) == null) {
-						response.setHeader(HEADER_ETAG, etag);
-					}
-					if (response.getHeader(HEADER_LAST_MODIFIED) == null) {
-						response.setDateHeader(HEADER_LAST_MODIFIED, lastModifiedTimestamp);
-					}
-				}
-			}
+	private String addEtagPadding(String etag) {
+		if (!(etag.startsWith("\"") || etag.startsWith("W/\"")) || !etag.endsWith("\"")) {
+			etag = "\"" + etag + "\"";
 		}
-		return this.notModified;
+		return etag;
 	}
 
-	public boolean isNotModified() {
-		return this.notModified;
-	}
 
 	@Override
 	public String getDescription(boolean includeClientInfo) {

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

@@ -27,6 +27,7 @@ import java.util.Map;
  * not for actual handling of the request.
  *
  * @author Juergen Hoeller
+ * @author Brian Clozel
  * @since 2.0
  * @see WebRequestInterceptor
  */
@@ -194,8 +195,8 @@ 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 "Etag" and "Last-Modified" response headers,
-	 * for both the modified case and the not-modified case.
+	 * <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">
 	 * public String myHandleMethod(WebRequest webRequest, Model model) {

spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java

@@ -58,12 +58,13 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter {
 
 	private static final String DIRECTIVE_NO_STORE = "no-store";
 
+	private static final String STREAMING_ATTRIBUTE = ShallowEtagHeaderFilter.class.getName() + ".STREAMING";
+
+
 	/** Checking for Servlet 3.0+ HttpServletResponse.getHeader(String) */
-	private static final boolean responseGetHeaderAvailable =
+	private static final boolean servlet3Present =
 			ClassUtils.hasMethod(HttpServletResponse.class, "getHeader", String.class);
 
-	private static final String STREAMING_ATTRIBUTE = ShallowEtagHeaderFilter.class.getName() + ".STREAMING";
-
 
 	/**
 	 * The default value is "false" so that the filter may delay the generation of
@@ -144,7 +145,10 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter {
 			int responseStatusCode, InputStream inputStream) {
 
 		if (responseStatusCode >= 200 && responseStatusCode < 300 && HttpMethod.GET.name().equals(request.getMethod())) {
-			String cacheControl = (responseGetHeaderAvailable ? response.getHeader(HEADER_CACHE_CONTROL) : null);
+			String cacheControl = null;
+			if (servlet3Present) {
+				cacheControl = response.getHeader(HEADER_CACHE_CONTROL);
+			}
 			if (cacheControl == null || !cacheControl.contains(DIRECTIVE_NO_STORE)) {
 				return true;
 			}