From 29da44c8dcc0fd7244b47dfc57be700458d7238d Mon Sep 17 00:00:00 2001
From: Brian Clozel <bclozel@pivotal.io>
Date: Fri, 29 Apr 2016 11:17:47 +0200
Subject: [PATCH] Support ETags with special chars in ServletWebRequest

This commit makes sure that HTTP request headers containing ETag values
are properly parsed and not simply tokenized using a "," separator.
Indeed, ETags can legally contain separator characters such as " " and
",".

Issue: SPR-14216
---
 .../context/request/ServletWebRequest.java    | 33 +++++++++++--------
 .../ServletWebRequestHttpMethodsTests.java    | 12 +++++++
 2 files changed, 31 insertions(+), 14 deletions(-)

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 1d5e1963d7a..b29397f2140 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
@@ -21,6 +21,8 @@ import java.util.Date;
 import java.util.Iterator;
 import java.util.Locale;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -64,6 +66,12 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 
 	private static final String METHOD_DELETE = "DELETE";
 
+	/**
+	 * Pattern matching ETag multiple field values in headers such as "If-Match", "If-None-Match"
+	 * @see <a href="https://tools.ietf.org/html/rfc7232#section-2.3">Section 2.3 of RFC 7232</a>
+	 */
+	private static final Pattern ETAG_HEADER_VALUE_PATTERN = Pattern.compile("\\*|\\s*((W\\/)?(\"[^\"]*\"))\\s*,?");
+
 
 	/** Checking for Servlet 3.0+ HttpServletResponse.getHeader(String) */
 	private static final boolean servlet3Present =
@@ -217,7 +225,9 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 		if (StringUtils.hasLength(etag) && !this.notModified) {
 			if (isCompatibleWithConditionalRequests(response)) {
 				etag = addEtagPadding(etag);
-				this.notModified = isEtagNotModified(etag);
+				if (hasRequestHeader(HEADER_IF_NONE_MATCH)) {
+					this.notModified = isEtagNotModified(etag);
+				}
 				if (response != null) {
 					if (this.notModified && supportsNotModifiedStatus()) {
 						response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
@@ -343,18 +353,14 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 	}
 
 	private boolean isEtagNotModified(String etag) {
-		if (StringUtils.hasLength(etag)) {
-			String ifNoneMatch = getHeader(HEADER_IF_NONE_MATCH);
-			if (StringUtils.hasLength(ifNoneMatch)) {
-				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;
-					}
-				}
+		String ifNoneMatch = getHeader(HEADER_IF_NONE_MATCH);
+		// compare weak/strong ETag as per https://tools.ietf.org/html/rfc7232#section-2.3
+		String serverETag = etag.replaceFirst("^W/", "");
+		Matcher eTagMatcher = ETAG_HEADER_VALUE_PATTERN.matcher(ifNoneMatch);
+		while (eTagMatcher.find()) {
+			if ("*".equals(eTagMatcher.group())
+					|| serverETag.equals(eTagMatcher.group(3))) {
+				return true;
 			}
 		}
 		return false;
@@ -367,7 +373,6 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 		return etag;
 	}
 
-
 	@Override
 	public String getDescription(boolean includeClientInfo) {
 		HttpServletRequest request = getRequest();
diff --git a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java
index eccf61ba466..a5a8b07ce5a 100644
--- a/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java
+++ b/spring-web/src/test/java/org/springframework/web/context/request/ServletWebRequestHttpMethodsTests.java
@@ -144,6 +144,18 @@ public class ServletWebRequestHttpMethodsTests {
 		assertEquals(eTag, servletResponse.getHeader("ETag"));
 	}
 
+	@Test
+	public void checkNotModifiedETagWithSeparatorChars() {
+		String eTag = "\"Foo, Bar\"";
+		servletRequest.addHeader("If-None-Match", eTag);
+
+		assertTrue(request.checkNotModified(eTag));
+
+		assertEquals(304, servletResponse.getStatus());
+		assertEquals(eTag, servletResponse.getHeader("ETag"));
+	}
+
+
 	@Test
 	public void checkModifiedETag() {
 		String currentETag = "\"Foo\"";