6641309: Wrong Cookie separator used in HttpURLConnection
authorjccollet
Wed, 05 Mar 2008 11:40:22 +0100
changeset 74 068494063b1b
parent 73 cf334423502b
child 75 dedcbd82b6e9
6641309: Wrong Cookie separator used in HttpURLConnection Summary: Added a space to cookie separator. Generified the code and added tags. Reviewed-by: chegar
jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java
jdk/test/java/net/CookieHandler/CookieManagerTest.java
jdk/test/sun/net/www/protocol/http/B6641309.java
--- a/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java	Wed Mar 05 09:52:50 2008 +0800
+++ b/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java	Wed Mar 05 11:40:22 2008 +0100
@@ -64,11 +64,6 @@
 import java.util.TimeZone;
 import java.net.MalformedURLException;
 import java.nio.ByteBuffer;
-import java.nio.channels.ReadableByteChannel;
-import java.nio.channels.WritableByteChannel;
-import java.nio.channels.Selector;
-import java.nio.channels.SelectionKey;
-import java.nio.channels.SelectableChannel;
 import java.lang.reflect.*;
 
 /**
@@ -824,6 +819,7 @@
      * - get input, [read input,] get output, [write output]
      */
 
+    @Override
     public synchronized OutputStream getOutputStream() throws IOException {
 
         try {
@@ -908,30 +904,25 @@
 
             URI uri = ParseUtil.toURI(url);
             if (uri != null) {
-                Map cookies = cookieHandler.get(uri, requests.getHeaders(EXCLUDE_HEADERS));
+                Map<String, List<String>> cookies = cookieHandler.get(uri, requests.getHeaders(EXCLUDE_HEADERS));
                 if (!cookies.isEmpty()) {
-                    Set s = cookies.entrySet();
-                    Iterator k_itr = s.iterator();
-                    while (k_itr.hasNext()) {
-                        Map.Entry entry = (Map.Entry)k_itr.next();
-                        String key = (String)entry.getKey();
+                    for (Map.Entry<String, List<String>> entry : cookies.entrySet()) {
+                        String key = entry.getKey();
                         // ignore all entries that don't have "Cookie"
                         // or "Cookie2" as keys
                         if (!"Cookie".equalsIgnoreCase(key) &&
                             !"Cookie2".equalsIgnoreCase(key)) {
                             continue;
                         }
-                        List l = (List)entry.getValue();
+                        List<String> l = entry.getValue();
                         if (l != null && !l.isEmpty()) {
-                            Iterator v_itr = l.iterator();
                             StringBuilder cookieValue = new StringBuilder();
-                            while (v_itr.hasNext()) {
-                                String value = (String)v_itr.next();
-                                cookieValue.append(value).append(';');
+                            for (String value : l) {
+                                cookieValue.append(value).append("; ");
                             }
-                            // strip off the ending ;-sign
+                            // strip off the trailing '; '
                             try {
-                                requests.add(key, cookieValue.substring(0, cookieValue.length() - 1));
+                                requests.add(key, cookieValue.substring(0, cookieValue.length() - 2));
                             } catch (StringIndexOutOfBoundsException ignored) {
                                 // no-op
                             }
@@ -950,6 +941,8 @@
         } // end of getting cookies
     }
 
+    @Override
+    @SuppressWarnings("empty-statement")
     public synchronized InputStream getInputStream() throws IOException {
 
         if (!doInput) {
@@ -1386,6 +1379,7 @@
         }
     }
 
+    @Override
     public InputStream getErrorStream() {
         if (connected && responseCode >= 400) {
             // Client Error 4xx and Server Error 5xx
@@ -2152,6 +2146,7 @@
      * Gets a header field by name. Returns null if not known.
      * @param name the name of the header field
      */
+    @Override
     public String getHeaderField(String name) {
         try {
             getInputStream();
@@ -2174,6 +2169,7 @@
      * @return a Map of header fields
      * @since 1.4
      */
+    @Override
     public Map getHeaderFields() {
         try {
             getInputStream();
@@ -2190,6 +2186,7 @@
      * Gets a header field by index. Returns null if not known.
      * @param n the index of the header field
      */
+    @Override
     public String getHeaderField(int n) {
         try {
             getInputStream();
@@ -2205,6 +2202,7 @@
      * Gets a header field by index. Returns null if not known.
      * @param n the index of the header field
      */
+    @Override
     public String getHeaderFieldKey(int n) {
         try {
             getInputStream();
@@ -2222,6 +2220,7 @@
      * exists, overwrite its value with the new value.
      * @param value the value to be set
      */
+    @Override
     public void setRequestProperty(String key, String value) {
         if (connected)
             throw new IllegalStateException("Already connected");
@@ -2243,6 +2242,7 @@
      * @see #getRequestProperties(java.lang.String)
      * @since 1.4
      */
+    @Override
     public void addRequestProperty(String key, String value) {
         if (connected)
             throw new IllegalStateException("Already connected");
@@ -2262,6 +2262,7 @@
         requests.set(key, value);
     }
 
+    @Override
     public String getRequestProperty (String key) {
         // don't return headers containing security sensitive information
         if (key != null) {
@@ -2286,6 +2287,7 @@
      * @throws IllegalStateException if already connected
      * @since 1.4
      */
+    @Override
     public Map getRequestProperties() {
         if (connected)
             throw new IllegalStateException("Already connected");
@@ -2294,6 +2296,7 @@
         return requests.getHeaders(EXCLUDE_HEADERS);
     }
 
+    @Override
     public void setConnectTimeout(int timeout) {
         if (timeout < 0)
             throw new IllegalArgumentException("timeouts can't be negative");
@@ -2313,6 +2316,7 @@
      * @see java.net.URLConnection#connect()
      * @since 1.5
      */
+    @Override
     public int getConnectTimeout() {
         return (connectTimeout < 0 ? 0 : connectTimeout);
     }
@@ -2337,6 +2341,7 @@
      * @see java.io.InputStream#read()
      * @since 1.5
      */
+    @Override
     public void setReadTimeout(int timeout) {
         if (timeout < 0)
             throw new IllegalArgumentException("timeouts can't be negative");
@@ -2354,10 +2359,12 @@
      * @see java.io.InputStream#read()
      * @since 1.5
      */
+    @Override
     public int getReadTimeout() {
         return readTimeout < 0 ? 0 : readTimeout;
     }
 
+    @Override
     protected void finalize() {
         // this should do nothing.  The stream finalizer will close
         // the fd
@@ -2437,6 +2444,7 @@
          * @see     java.io.FilterInputStream#in
          * @see     java.io.FilterInputStream#reset()
          */
+        @Override
         public synchronized void mark(int readlimit) {
             super.mark(readlimit);
             if (cacheRequest != null) {
@@ -2466,6 +2474,7 @@
          * @see        java.io.FilterInputStream#in
          * @see        java.io.FilterInputStream#mark(int)
          */
+        @Override
         public synchronized void reset() throws IOException {
             super.reset();
             if (cacheRequest != null) {
@@ -2474,6 +2483,7 @@
             }
         }
 
+        @Override
         public int read() throws IOException {
             try {
                 byte[] b = new byte[1];
@@ -2487,10 +2497,12 @@
             }
         }
 
+        @Override
         public int read(byte[] b) throws IOException {
             return read(b, 0, b.length);
         }
 
+        @Override
         public int read(byte[] b, int off, int len) throws IOException {
             try {
                 int newLen = super.read(b, off, len);
@@ -2521,6 +2533,7 @@
             }
         }
 
+        @Override
         public void close () throws IOException {
             try {
                 if (outputStream != null) {
@@ -2565,6 +2578,7 @@
             error = false;
         }
 
+        @Override
         public void write (int b) throws IOException {
             checkError();
             written ++;
@@ -2574,10 +2588,12 @@
             out.write (b);
         }
 
+        @Override
         public void write (byte[] b) throws IOException {
             write (b, 0, b.length);
         }
 
+        @Override
         public void write (byte[] b, int off, int len) throws IOException {
             checkError();
             written += len;
@@ -2608,6 +2624,7 @@
             return closed && ! error;
         }
 
+        @Override
         public void close () throws IOException {
             if (closed) {
                 return;
@@ -2726,6 +2743,7 @@
             }
         }
 
+        @Override
         public int available() throws IOException {
             if (is == null) {
                 return buffer.remaining();
@@ -2740,10 +2758,12 @@
             return (ret == -1? ret : (b[0] & 0x00FF));
         }
 
+        @Override
         public int read(byte[] b) throws IOException {
             return read(b, 0, b.length);
         }
 
+        @Override
         public int read(byte[] b, int off, int len) throws IOException {
             int rem = buffer.remaining();
             if (rem > 0) {
@@ -2759,6 +2779,7 @@
             }
         }
 
+        @Override
         public void close() throws IOException {
             buffer = null;
             if (is != null) {
@@ -2775,6 +2796,7 @@
 
 class EmptyInputStream extends InputStream {
 
+    @Override
     public int available() {
         return 0;
     }
--- a/jdk/test/java/net/CookieHandler/CookieManagerTest.java	Wed Mar 05 09:52:50 2008 +0800
+++ b/jdk/test/java/net/CookieHandler/CookieManagerTest.java	Wed Mar 05 11:40:22 2008 +0100
@@ -132,17 +132,17 @@
                 ),
                 new CookieTestCase("Set-Cookie",
                 "PART_NUMBER=ROCKET_LAUNCHER_0001; path=/;" + "domain=." + localHostAddr,
-                "CUSTOMER=WILE:BOB;PART_NUMBER=ROCKET_LAUNCHER_0001",
+                "CUSTOMER=WILE:BOB; PART_NUMBER=ROCKET_LAUNCHER_0001",
                 "/"
                 ),
                 new CookieTestCase("Set-Cookie",
                 "SHIPPING=FEDEX; path=/foo;" + "domain=." + localHostAddr,
-                "CUSTOMER=WILE:BOB;PART_NUMBER=ROCKET_LAUNCHER_0001",
+                "CUSTOMER=WILE:BOB; PART_NUMBER=ROCKET_LAUNCHER_0001",
                 "/"
                 ),
                 new CookieTestCase("Set-Cookie",
                 "SHIPPING=FEDEX; path=/foo;" + "domain=." + localHostAddr,
-                "CUSTOMER=WILE:BOB;PART_NUMBER=ROCKET_LAUNCHER_0001;SHIPPING=FEDEX",
+                "CUSTOMER=WILE:BOB; PART_NUMBER=ROCKET_LAUNCHER_0001; SHIPPING=FEDEX",
                 "/foo"
                 )
                 };
@@ -157,7 +157,7 @@
                 ),
                 new CookieTestCase("Set-Cookie",
                 "PART_NUMBER=RIDING_ROCKET_0023; path=/ammo;" + "domain=." + localHostAddr,
-                "PART_NUMBER=RIDING_ROCKET_0023;PART_NUMBER=ROCKET_LAUNCHER_0001",
+                "PART_NUMBER=RIDING_ROCKET_0023; PART_NUMBER=ROCKET_LAUNCHER_0001",
                 "/ammo"
                 )
                 };
@@ -167,17 +167,17 @@
         testCases[count++] = new CookieTestCase[]{
                 new CookieTestCase("Set-Cookie2",
                 "Customer=\"WILE_E_COYOTE\"; Version=\"1\"; Path=\"/acme\";" + "domain=." + localHostAddr,
-                "$Version=\"1\";Customer=\"WILE_E_COYOTE\";$Path=\"/acme\";$Domain=\"." + localHostAddr + "\"",
+                "$Version=\"1\"; Customer=\"WILE_E_COYOTE\";$Path=\"/acme\";$Domain=\"." + localHostAddr + "\"",
                 "/acme/login"
                 ),
                 new CookieTestCase("Set-Cookie2",
                 "Part_Number=\"Rocket_Launcher_0001\"; Version=\"1\";Path=\"/acme\";" + "domain=." + localHostAddr,
-                "$Version=\"1\";Customer=\"WILE_E_COYOTE\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr  + "\"" + ";Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr +  "\"",
+                "$Version=\"1\"; Customer=\"WILE_E_COYOTE\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr  + "\"" + "; Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr +  "\"",
                 "/acme/pickitem"
                 ),
                 new CookieTestCase("Set-Cookie2",
                 "Shipping=\"FedEx\"; Version=\"1\"; Path=\"/acme\";" + "domain=." + localHostAddr,
-                "$Version=\"1\";Customer=\"WILE_E_COYOTE\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr  + "\"" + ";Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr  + "\"" + ";Shipping=\"FedEx\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr + "\"",
+                "$Version=\"1\"; Customer=\"WILE_E_COYOTE\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr  + "\"" + "; Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr  + "\"" + "; Shipping=\"FedEx\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr + "\"",
                 "/acme/shipping"
                 )
                 };
@@ -187,17 +187,17 @@
         testCases[count++] = new CookieTestCase[]{
                 new CookieTestCase("Set-Cookie2",
                 "Part_Number=\"Rocket_Launcher_0001\"; Version=\"1\"; Path=\"/acme\";" + "domain=." + localHostAddr,
-                "$Version=\"1\";Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";$Domain=\"." + localHostAddr + "\"",
+                "$Version=\"1\"; Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";$Domain=\"." + localHostAddr + "\"",
                 "/acme/ammo"
                 ),
                 new CookieTestCase("Set-Cookie2",
                 "Part_Number=\"Riding_Rocket_0023\"; Version=\"1\"; Path=\"/acme/ammo\";" + "domain=." + localHostAddr,
-                "$Version=\"1\";Part_Number=\"Riding_Rocket_0023\";$Path=\"/acme/ammo\";$Domain=\"." + localHostAddr  + "\"" + ";Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr + "\"",
+                "$Version=\"1\"; Part_Number=\"Riding_Rocket_0023\";$Path=\"/acme/ammo\";$Domain=\"." + localHostAddr  + "\"" + "; Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr + "\"",
                 "/acme/ammo"
                 ),
                 new CookieTestCase("",
                 "",
-                "$Version=\"1\";Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr + "\"",
+                "$Version=\"1\"; Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";" + "$Domain=\"." + localHostAddr + "\"",
                 "/acme/parts"
                 )
                 };
@@ -207,12 +207,12 @@
         testCases[count++] = new CookieTestCase[]{
                 new CookieTestCase("Set-Cookie2",
                 "Part_Number=\"Rocket_Launcher_0001\"; Version=\"1\"; Path=\"/acme\";" + "domain=." + localHostAddr,
-                "$Version=\"1\";Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";$Domain=\"." + localHostAddr + "\"",
+                "$Version=\"1\"; Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\";$Domain=\"." + localHostAddr + "\"",
                 "/acme"
                 ),
                 new CookieTestCase("Set-Cookie2",
                 "Part_Number=\"Rocket_Launcher_2000\"; Version=\"1\"; Path=\"/acme\";" + "domain=." + localHostAddr,
-                "$Version=\"1\";Part_Number=\"Rocket_Launcher_2000\";$Path=\"/acme\";$Domain=\"." + localHostAddr + "\"",
+                "$Version=\"1\"; Part_Number=\"Rocket_Launcher_2000\";$Path=\"/acme\";$Domain=\"." + localHostAddr + "\"",
                 "/acme"
                 )
                 };
@@ -222,17 +222,17 @@
         testCases[count++] = new CookieTestCase[]{
                 new CookieTestCase("Set-Cookie2",
                 "Customer=\"WILE_E_COYOTE\"; Version=\"1\"; Path=\"/acme\"",
-                "$Version=\"1\";Customer=\"WILE_E_COYOTE\";$Path=\"/acme\"",
+                "$Version=\"1\"; Customer=\"WILE_E_COYOTE\";$Path=\"/acme\"",
                 "/acme/login"
                 ),
                 new CookieTestCase("Set-Cookie2",
                 "Part_Number=\"Rocket_Launcher_0001\"; Version=\"1\";Path=\"/acme\"",
-                "$Version=\"1\";Customer=\"WILE_E_COYOTE\";$Path=\"/acme\"" + ";Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\"",
+                "$Version=\"1\"; Customer=\"WILE_E_COYOTE\";$Path=\"/acme\"" + "; Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\"",
                 "/acme/pickitem"
                 ),
                 new CookieTestCase("Set-Cookie2",
                 "Shipping=\"FedEx\"; Version=\"1\"; Path=\"/acme\"",
-                "$Version=\"1\";Customer=\"WILE_E_COYOTE\";$Path=\"/acme\"" + ";Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\"" + ";Shipping=\"FedEx\";$Path=\"/acme\"",
+                "$Version=\"1\"; Customer=\"WILE_E_COYOTE\";$Path=\"/acme\"" + "; Part_Number=\"Rocket_Launcher_0001\";$Path=\"/acme\"" + "; Shipping=\"FedEx\";$Path=\"/acme\"",
                 "/acme/shipping"
                 )
                 };
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/sun/net/www/protocol/http/B6641309.java	Wed Mar 05 11:40:22 2008 +0100
@@ -0,0 +1,129 @@
+/*
+ * Copyright 2008 Sun Microsystems, Inc.  All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/*
+ * @test
+ * @bug 6641309
+ * @summary Wrong Cookie separator used in HttpURLConnection
+ */
+
+import java.net.*;
+import java.util.*;
+import java.io.*;
+import com.sun.net.httpserver.*;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ExecutorService;
+
+public class B6641309
+{
+    com.sun.net.httpserver.HttpServer httpServer;
+    ExecutorService executorService;
+
+    public static void main(String[] args)
+    {
+        new B6641309();
+    }
+
+    public B6641309()
+    {
+        try {
+            startHttpServer();
+            doClient();
+        } catch (IOException ioe) {
+            System.err.println(ioe);
+        }
+    }
+
+    void doClient() {
+        CookieHandler.setDefault(new CookieManager(null, CookiePolicy.ACCEPT_ALL));
+        try {
+            InetSocketAddress address = httpServer.getAddress();
+
+            // GET Request
+            URL url = new URL("http://localhost:" + address.getPort() + "/test/");
+            CookieHandler ch = CookieHandler.getDefault();
+            Map<String,List<String>> header = new HashMap<String,List<String>>();
+            List<String> values = new LinkedList<String>();
+            values.add("Test1Cookie=TEST1; path=/test/");
+            values.add("Test2Cookie=TEST2; path=/test/");
+            header.put("Set-Cookie", values);
+
+            // preload the CookieHandler with a cookie for our URL
+            // so that it will be sent during the first request
+            ch.put(url.toURI(), header);
+            HttpURLConnection uc = (HttpURLConnection)url.openConnection();
+            int resp = uc.getResponseCode();
+            if (resp != 200)
+                throw new RuntimeException("Failed: Response code from GET is not 200");
+
+            System.out.println("Response code from GET = 200 OK");
+
+        } catch (IOException e) {
+            e.printStackTrace();
+        } catch (URISyntaxException e) {
+            e.printStackTrace();
+        } finally {
+            httpServer.stop(1);
+            executorService.shutdown();
+        }
+    }
+
+    /**
+     * Http Server
+     */
+    public void startHttpServer() throws IOException {
+        httpServer = com.sun.net.httpserver.HttpServer.create(new InetSocketAddress(0), 0);
+
+        // create HttpServer context
+        HttpContext ctx = httpServer.createContext("/test/", new MyHandler());
+
+        executorService = Executors.newCachedThreadPool();
+        httpServer.setExecutor(executorService);
+        httpServer.start();
+    }
+
+    class MyHandler implements HttpHandler {
+        public void handle(HttpExchange t) throws IOException {
+            InputStream is = t.getRequestBody();
+            Headers reqHeaders = t.getRequestHeaders();
+            int i = 0;
+            // Read till end of stream
+            do {
+                i = is.read();
+            } while (i != -1);
+            is.close();
+
+            List<String> cookies = reqHeaders.get("Cookie");
+            if (cookies != null) {
+                for (String str : cookies) {
+                    // The separator between the 2 cookies should be
+                    // a semi-colon AND a space
+                    if (str.equals("Test1Cookie=TEST1; Test2Cookie=TEST2"))
+                        t.sendResponseHeaders(200, -1);
+                }
+            }
+            t.sendResponseHeaders(400, -1);
+            t.close();
+        }
+    }
+}