HTTPCLIENT-1263: BrowserCompatSpec: attribute values containing spaces or special characters should be enclosed with quotes marks for version 1 cookies

Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net>

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1418654 13f79535-47bb-0310-9956-ffa450edef68

RELEASE_NOTES.txt

@@ -1,6 +1,10 @@
 Changes in trunk
 -------------------
 
+* [HTTPCLIENT-1263] BrowserCompatSpec: attribute values containing spaces or special characters 
+  should be enclosed with quotes marks for version 1 cookies.
+  Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net> 
+
 * [HTTPCLIENT-1266] NTLM engine refactoring and compatibility improvements.
   Contributed by Karl Wright <DaddyWri at gmail.com>  
 

httpclient/src/main/java/org/apache/http/impl/cookie/BrowserCompatSpec.java

@@ -40,6 +40,8 @@ import org.apache.http.cookie.Cookie;
 import org.apache.http.cookie.CookieOrigin;
 import org.apache.http.cookie.MalformedCookieException;
 import org.apache.http.cookie.SM;
+import org.apache.http.message.BasicHeaderElement;
+import org.apache.http.message.BasicHeaderValueFormatter;
 import org.apache.http.message.BufferedHeader;
 import org.apache.http.message.ParserCursor;
 import org.apache.http.util.CharArrayBuffer;
@@ -89,6 +91,7 @@ public class BrowserCompatSpec extends CookieSpecBase {
         registerAttribHandler(ClientCookie.COMMENT_ATTR, new BasicCommentHandler());
         registerAttribHandler(ClientCookie.EXPIRES_ATTR, new BasicExpiresHandler(
                 this.datepatterns));
+        registerAttribHandler(ClientCookie.VERSION_ATTR, new BrowserCompatVersionAttributeHandler());
     }
 
     /** Default constructor */
@@ -160,6 +163,13 @@ public class BrowserCompatSpec extends CookieSpecBase {
             if (i > 0) {
                 buffer.append("; ");
             }
+            if (cookie.getVersion() > 0) {
+                BasicHeaderValueFormatter.INSTANCE.formatHeaderElement(
+                        buffer,
+                        new BasicHeaderElement(cookie.getName(), cookie.getValue()),
+                        false);
+            } else {
+                // Netscape style cookies do not support quoted values
                 buffer.append(cookie.getName());
                 buffer.append("=");
                 String s = cookie.getValue();
@@ -167,6 +177,7 @@ public class BrowserCompatSpec extends CookieSpecBase {
                     buffer.append(s);
                 }
             }
+        }
         List<Header> headers = new ArrayList<Header>(1);
         headers.add(new BufferedHeader(buffer));
         return headers;

httpclient/src/main/java/org/apache/http/impl/cookie/BrowserCompatVersionAttributeHandler.java

@@ -0,0 +1,67 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.http.impl.cookie;
+
+import org.apache.http.annotation.Immutable;
+import org.apache.http.cookie.MalformedCookieException;
+import org.apache.http.cookie.SetCookie;
+
+/**
+ * <tt>"Version"</tt> cookie attribute handler for BrowserCompat cookie spec.
+ *
+ * @since 4.3
+ */
+@Immutable
+public class BrowserCompatVersionAttributeHandler extends
+		AbstractCookieAttributeHandler {
+
+	public BrowserCompatVersionAttributeHandler() {
+		super();
+	}
+
+	/**
+	 * Parse cookie version attribute.
+	 */
+	public void parse(final SetCookie cookie, final String value)
+			throws MalformedCookieException {
+		if (cookie == null) {
+			throw new IllegalArgumentException("Cookie may not be null");
+		}
+		if (value == null) {
+			throw new MalformedCookieException("Missing value for version attribute");
+		}
+		int version = 0;
+		try {
+			version = Integer.parseInt(value);
+		} catch (NumberFormatException e) {
+			// Just ignore invalid versions
+		}
+		cookie.setVersion(version);
+	}
+
+}

httpclient/src/test/java/org/apache/http/impl/cookie/TestBrowserCompatSpec.java

@@ -94,7 +94,7 @@ public class TestBrowserCompatSpec {
         Assert.assertEquals("custno", cookies.get(0).getName());
         Assert.assertEquals("12345", cookies.get(0).getValue());
         Assert.assertEquals("test", cookies.get(0).getComment());
-        Assert.assertEquals(0, cookies.get(0).getVersion());
+        Assert.assertEquals(1, cookies.get(0).getVersion());
         Assert.assertEquals("www.apache.org", cookies.get(0).getDomain());
         Assert.assertEquals("/", cookies.get(0).getPath());
         Assert.assertFalse(cookies.get(0).isSecure());
@@ -102,7 +102,7 @@ public class TestBrowserCompatSpec {
         Assert.assertEquals("name", cookies.get(1).getName());
         Assert.assertEquals("John", cookies.get(1).getValue());
         Assert.assertEquals(null, cookies.get(1).getComment());
-        Assert.assertEquals(0, cookies.get(1).getVersion());
+        Assert.assertEquals(1, cookies.get(1).getVersion());
         Assert.assertEquals(".apache.org", cookies.get(1).getDomain());
         Assert.assertEquals("/", cookies.get(1).getPath());
         Assert.assertTrue(cookies.get(1).isSecure());
@@ -130,7 +130,7 @@ public class TestBrowserCompatSpec {
         Assert.assertEquals("custno", cookies.get(0).getName());
         Assert.assertEquals("12345", cookies.get(0).getValue());
         Assert.assertEquals("test", cookies.get(0).getComment());
-        Assert.assertEquals(0, cookies.get(0).getVersion());
+        Assert.assertEquals(1, cookies.get(0).getVersion());
         Assert.assertEquals("www.apache.org", cookies.get(0).getDomain());
         Assert.assertEquals("/", cookies.get(0).getPath());
         Assert.assertFalse(cookies.get(0).isSecure());
@@ -138,7 +138,7 @@ public class TestBrowserCompatSpec {
         Assert.assertEquals("name", cookies.get(1).getName());
         Assert.assertEquals("John", cookies.get(1).getValue());
         Assert.assertEquals(null, cookies.get(1).getComment());
-        Assert.assertEquals(0, cookies.get(1).getVersion());
+        Assert.assertEquals(1, cookies.get(1).getVersion());
         Assert.assertEquals(".apache.org", cookies.get(1).getDomain());
         Assert.assertEquals("/", cookies.get(1).getPath());
         Assert.assertTrue(cookies.get(1).isSecure());
@@ -166,7 +166,7 @@ public class TestBrowserCompatSpec {
         Assert.assertEquals("name", cookies.get(0).getName());
         Assert.assertEquals("Doe, John", cookies.get(0).getValue());
         Assert.assertEquals(null, cookies.get(0).getComment());
-        Assert.assertEquals(0, cookies.get(0).getVersion());
+        Assert.assertEquals(1, cookies.get(0).getVersion());
         Assert.assertEquals(".apache.org", cookies.get(0).getDomain());
         Assert.assertEquals("/", cookies.get(0).getPath());
         Assert.assertTrue(cookies.get(0).isSecure());
@@ -458,7 +458,7 @@ public class TestBrowserCompatSpec {
         Assert.assertTrue("Secure",cookies.get(0).isSecure());
         Assert.assertEquals(new Date(10000L),cookies.get(0).getExpiryDate());
         Assert.assertEquals("Comment","This is a comment.",cookies.get(0).getComment());
-        Assert.assertEquals("Version",0,cookies.get(0).getVersion());
+        Assert.assertEquals("Version",1,cookies.get(0).getVersion());
     }
 
     @Test
@@ -998,4 +998,38 @@ public class TestBrowserCompatSpec {
         }
     }
 
+    /**
+     * Tests cookie version 1 with space in cookie value.
+     */
+    @Test
+    public void testFormatCookieWithSpaceInValue() throws Exception {
+        CookieOrigin origin = new CookieOrigin("myhost.mydomain.com", 80, "/", false);
+        CookieSpec cookieSpec = new BrowserCompatSpec();
+        Header setCookieHeader = new BasicHeader("Set-Cookie", "test=\"value 1\"; Version=1");
+        Cookie cookie = cookieSpec.parse(setCookieHeader, origin).get(0);
+        List<Cookie> cookies = new ArrayList<Cookie>();
+        cookies.add(cookie);
+        List<Header> headers = cookieSpec.formatCookies(cookies);
+        Assert.assertNotNull(headers);
+        Assert.assertEquals(1, headers.size());
+        Assert.assertEquals("test=\"value 1\"", headers.get(0).getValue());
+    }
+
+    /**
+     * Tests Netscape cookie with space in cookie value.
+     */
+    @Test
+    public void testFormatCookieVersion0WithSpaceInValue() throws Exception {
+        CookieOrigin origin = new CookieOrigin("myhost.mydomain.com", 80, "/", false);
+        CookieSpec cookieSpec = new BrowserCompatSpec();
+        Header setCookieHeader = new BasicHeader("Set-Cookie", "test=value 1");
+        Cookie cookie = cookieSpec.parse(setCookieHeader, origin).get(0);
+        List<Cookie> cookies = new ArrayList<Cookie>();
+        cookies.add(cookie);
+        List<Header> headers = cookieSpec.formatCookies(cookies);
+        Assert.assertNotNull(headers);
+        Assert.assertEquals(1, headers.size());
+        Assert.assertEquals("test=value 1", headers.get(0).getValue());
+    }
+
 }