From e5d8fa077178345f9974d5ef239dfd2ca61c21d1 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Mon, 3 Jan 2011 15:51:23 +0000 Subject: [PATCH] SPR-7834 - HttpHeaders.getEtag() mangles the value --- .../org/springframework/http/HttpHeaders.java | 43 +++++-------------- .../context/request/ServletWebRequest.java | 10 +---- .../http/HttpHeadersTests.java | 21 ++++++--- .../request/ServletWebRequestTests.java | 23 +++------- 4 files changed, 33 insertions(+), 64 deletions(-) diff --git a/org.springframework.web/src/main/java/org/springframework/http/HttpHeaders.java b/org.springframework.web/src/main/java/org/springframework/http/HttpHeaders.java index c4ba00c4cf..59cc9b986d 100644 --- a/org.springframework.web/src/main/java/org/springframework/http/HttpHeaders.java +++ b/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 { * @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 { * @return the entity tag */ public String getETag() { - return unquote(getFirst(ETAG)); + return getFirst(ETAG); } /** @@ -362,7 +366,7 @@ public class HttpHeaders implements MultiValueMap { * @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 { StringBuilder builder = new StringBuilder(); for (Iterator 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 { 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 { // 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) { diff --git a/org.springframework.web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java b/org.springframework.web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java index 17a23c08ce..1caf21e48d 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java +++ b/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) { diff --git a/org.springframework.web/src/test/java/org/springframework/http/HttpHeadersTests.java b/org.springframework.web/src/test/java/org/springframework/http/HttpHeadersTests.java index 6fe9ed0d9d..b8051bd859 100644 --- a/org.springframework.web/src/test/java/org/springframework/http/HttpHeadersTests.java +++ b/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 ifNoneMatch2 = "v2.7"; + String ifNoneMatch1 = "\"v2.6\""; + String ifNoneMatch2 = "\"v2.7\""; List ifNoneMatchList = new ArrayList(2); ifNoneMatchList.add(ifNoneMatch1); ifNoneMatchList.add(ifNoneMatch2); diff --git a/org.springframework.web/src/test/java/org/springframework/web/context/request/ServletWebRequestTests.java b/org.springframework.web/src/test/java/org/springframework/web/context/request/ServletWebRequestTests.java index 2e02082de5..63377a8bcf 100644 --- a/org.springframework.web/src/test/java/org/springframework/web/context/request/ServletWebRequestTests.java +++ b/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() { - 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() { + public void checkModifiedETag() { String currentETag = "\"Foo\""; String oldEtag = "Bar"; servletRequest.setMethod("GET"); -- GitLab