SPR-7834 - HttpHeaders.getEtag() mangles the value

org.springframework.web/src/main/java/org/springframework/http/HttpHeaders.java

@@ -1,11 +1,11 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 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.
  * You may obtain a copy of the License at
  *
- *      http://www.apache.org/licenses/LICENSE-2.0
+ *     http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -310,7 +310,11 @@ public class HttpHeaders implements MultiValueMap<String, String> {
 	 * @param eTag the new entity tag
 	 */
 	public void setETag(String eTag) {
-		set(ETAG, quote(eTag));
+		if (eTag != null) {
+			Assert.isTrue(eTag.startsWith("\"") || eTag.startsWith("W/"), "Invalid eTag, does not start with W/ or \"");
+			Assert.isTrue(eTag.endsWith("\""), "Invalid eTag, does not end with \"");
+		}
+			set(ETAG, eTag);
 	}
 
 	/**
@@ -318,7 +322,7 @@ public class HttpHeaders implements MultiValueMap<String, String> {
 	 * @return the entity tag
 	 */
 	public String getETag() {
-		return unquote(getFirst(ETAG));
+		return getFirst(ETAG);
 	}
 
 	/**
@@ -362,7 +366,7 @@ public class HttpHeaders implements MultiValueMap<String, String> {
 	 * @param ifNoneMatch the new value of the header
 	 */
 	public void setIfNoneMatch(String ifNoneMatch) {
-		set(IF_NONE_MATCH, quote(ifNoneMatch));
+		set(IF_NONE_MATCH, ifNoneMatch);
 	}
 
 	/**
@@ -373,7 +377,7 @@ public class HttpHeaders implements MultiValueMap<String, String> {
 		StringBuilder builder = new StringBuilder();
 		for (Iterator<String> iterator = ifNoneMatchList.iterator(); iterator.hasNext();) {
 			String ifNoneMatch = iterator.next();
-			builder.append(quote(ifNoneMatch));
+			builder.append(ifNoneMatch);
 			if (iterator.hasNext()) {
 				builder.append(", ");
 			}
@@ -392,7 +396,7 @@ public class HttpHeaders implements MultiValueMap<String, String> {
 		if (value != null) {
 			String[] tokens = value.split(",\\s*");
 			for (String token : tokens) {
-				result.add(unquote(token));
+				result.add(token);
 			}
 		}
 		return result;
@@ -452,31 +456,6 @@ public class HttpHeaders implements MultiValueMap<String, String> {
 
 	// Utility methods
 
-	private String quote(String s) {
-		Assert.notNull(s);
-		if (!s.startsWith("\"")) {
-			s = "\"" + s;
-		}
-		if (!s.endsWith("\"")) {
-			s = s + "\"";
-		}
-		return s;
-	}
-
-	private String unquote(String s) {
-		if (s == null) {
-			return null;
-		}
-		if (s.startsWith("\"")) {
-			s = s.substring(1);
-		}
-		if (s.endsWith("\"")) {
-			s = s.substring(0, s.length() - 1);
-		}
-		return s;
-	}
-
-
 	private long getFirstDate(String headerName) {
 		String headerValue = getFirst(headerName);
 		if (headerValue == null) {

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

@@ -1,11 +1,11 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 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.
  * You may obtain a copy of the License at
  *
- *      http://www.apache.org/licenses/LICENSE-2.0
+ *     http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -206,12 +206,6 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ
 	public boolean checkNotModified(String eTag) {
 		if (StringUtils.hasLength(eTag) && !this.notModified &&
 				(this.response == null || !this.response.containsHeader(HEADER_ETAG))) {
-			if (!eTag.startsWith("\"")) {
-				eTag = "\"" + eTag;
-			}
-			if (!eTag.endsWith("\"")) {
-				eTag = eTag + "\"";
-			}
 			String ifNoneMatch = getRequest().getHeader(HEADER_IF_NONE_MATCH);
 			this.notModified = eTag.equals(ifNoneMatch);
 			if (this.response != null) {

org.springframework.web/src/test/java/org/springframework/http/HttpHeadersTests.java

@@ -1,11 +1,11 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 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.
  * You may obtain a copy of the License at
  *
- *      http://www.apache.org/licenses/LICENSE-2.0
+ *     http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -27,10 +27,11 @@ import java.util.List;
 import java.util.Locale;
 import java.util.TimeZone;
 
-import static org.junit.Assert.*;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.junit.Assert.*;
+
 /** @author Arjen Poutsma */
 public class HttpHeadersTests {
 
@@ -99,6 +100,14 @@ public class HttpHeadersTests {
 
 	@Test
 	public void eTag() {
+		String eTag = "\"v2.6\"";
+		headers.setETag(eTag);
+		assertEquals("Invalid ETag header", eTag, headers.getETag());
+		assertEquals("Invalid ETag header", "\"v2.6\"", headers.getFirst("ETag"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void illegalETag() {
 		String eTag = "v2.6";
 		headers.setETag(eTag);
 		assertEquals("Invalid ETag header", eTag, headers.getETag());
@@ -107,7 +116,7 @@ public class HttpHeadersTests {
 
 	@Test
 	public void ifNoneMatch() {
-		String ifNoneMatch = "v2.6";
+		String ifNoneMatch = "\"v2.6\"";
 		headers.setIfNoneMatch(ifNoneMatch);
 		assertEquals("Invalid If-None-Match header", ifNoneMatch, headers.getIfNoneMatch().get(0));
 		assertEquals("Invalid If-None-Match header", "\"v2.6\"", headers.getFirst("If-None-Match"));
@@ -115,8 +124,8 @@ public class HttpHeadersTests {
 
 	@Test
 	public void ifNoneMatchList() {
-		String ifNoneMatch1 = "v2.6";
+		String ifNoneMatch1 = "\"v2.6\"";
-		String ifNoneMatch2 = "v2.7";
+		String ifNoneMatch2 = "\"v2.7\"";
 		List<String> ifNoneMatchList = new ArrayList<String>(2);
 		ifNoneMatchList.add(ifNoneMatch1);
 		ifNoneMatchList.add(ifNoneMatch2);

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

@@ -1,11 +1,11 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 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.
  * You may obtain a copy of the License at
  *
- *      http://www.apache.org/licenses/LICENSE-2.0
+ *     http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -141,9 +141,9 @@ public class ServletWebRequestTests {
 
 	@Test
 	public void checkNotModifiedETag() {
-		String eTag = "Foo";
+		String eTag = "\"Foo\"";
 		servletRequest.setMethod("GET");
-		servletRequest.addHeader("If-None-Match", "\"" + eTag + "\"");
+		servletRequest.addHeader("If-None-Match", eTag );
 
 		request.checkNotModified(eTag);
 
@@ -151,20 +151,7 @@ public class ServletWebRequestTests {
 	}
 
 	@Test
-	public void checkModifiedETagNonQuoted() {
+	public void checkModifiedETag() {
-		String currentETag = "Foo";
-		String oldEtag = "Bar";
-		servletRequest.setMethod("GET");
-		servletRequest.addHeader("If-None-Match", "\"" + oldEtag + "\"");
-
-		request.checkNotModified(currentETag);
-
-		assertEquals(200, servletResponse.getStatus());
-		assertEquals("\"" + currentETag + "\"", servletResponse.getHeader("ETag"));
-	}
-
-	@Test
-	public void checkModifiedETagQuoted() {
 		String currentETag = "\"Foo\"";
 		String oldEtag = "Bar";
 		servletRequest.setMethod("GET");