From 2fa1caca0cf5dad868870f856c0cadb983709f37 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 2 Sep 2015 14:17:42 +0200 Subject: [PATCH] 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 --- .../server/ServletServerHttpResponse.java | 5 +- .../context/request/ServletWebRequest.java | 142 ++++++++++-------- .../web/context/request/WebRequest.java | 5 +- .../web/filter/ShallowEtagHeaderFilter.java | 12 +- 4 files changed, 91 insertions(+), 73 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java index a54112db9b9..478022f69c3 100644 --- a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java +++ b/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()); } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java index 3807fdf6ffe..c734e08f6c8 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java +++ b/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) { diff --git a/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java index bcaaffe168b..cea9fa70af3 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/WebRequest.java +++ b/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. - *

This will also transparently set the "Etag" and "Last-Modified" response headers, - * for both the modified case and the not-modified case. + *

This will also transparently set the "ETag" and "Last-Modified" + * response headers, for both the modified case and the not-modified case. *

Typical usage: * 

 	 * public String myHandleMethod(WebRequest webRequest, Model model) {
diff --git a/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java b/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java
index 9c19c60becf..805697a3b07 100644
--- a/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java
+++ b/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;
 			}