6644726: Cookie management issues

Summary: Many changes to accomodate RFC 2965 and old Netscape specs
Reviewed-by: chegar

src/share/classes/java/net/CookieManager.java

@@ -205,11 +205,31 @@ public class CookieManager extends CookieHandler
         if (cookieJar == null)
             return Collections.unmodifiableMap(cookieMap);
 
+        boolean secureLink = "https".equalsIgnoreCase(uri.getScheme());
         List<HttpCookie> cookies = new java.util.ArrayList<HttpCookie>();
+        String path = uri.getPath();
+        if (path == null || path.isEmpty()) {
+            path = "/";
+        }
         for (HttpCookie cookie : cookieJar.get(uri)) {
             // apply path-matches rule (RFC 2965 sec. 3.3.4)
-            if (pathMatches(uri.getPath(), cookie.getPath())) {
-                cookies.add(cookie);
+            // and check for the possible "secure" tag (i.e. don't send
+            // 'secure' cookies over unsecure links)
+            if (pathMatches(path, cookie.getPath()) &&
+                    (secureLink || !cookie.getSecure())) {
+                // Let's check the authorize port list if it exists
+                String ports = cookie.getPortlist();
+                if (ports != null && !ports.isEmpty()) {
+                    int port = uri.getPort();
+                    if (port == -1) {
+                        port = "https".equals(uri.getScheme()) ? 443 : 80;
+                    }
+                    if (isInPortList(ports, port)) {
+                        cookies.add(cookie);
+                    }
+                } else {
+                    cookies.add(cookie);
+                }
             }
         }
 
@@ -251,8 +271,46 @@ public class CookieManager extends CookieHandler
                 try {
                     List<HttpCookie> cookies = HttpCookie.parse(headerValue);
                     for (HttpCookie cookie : cookies) {
-                        if (shouldAcceptInternal(uri, cookie)) {
-                            cookieJar.add(uri, cookie);
+                        if (cookie.getPath() == null) {
+                            // If no path is specified, then by default
+                            // the path is the directory of the page/doc
+                            String path = uri.getPath();
+                            if (!path.endsWith("/")) {
+                                int i = path.lastIndexOf("/");
+                                if (i > 0) {
+                                    path = path.substring(0, i + 1);
+                                } else {
+                                    path = "/";
+                                }
+                            }
+                            cookie.setPath(path);
+                        }
+                        String ports = cookie.getPortlist();
+                        if (ports != null) {
+                            int port = uri.getPort();
+                            if (port == -1) {
+                                port = "https".equals(uri.getScheme()) ? 443 : 80;
+                            }
+                            if (ports.isEmpty()) {
+                                // Empty port list means this should be restricted
+                                // to the incoming URI port
+                                cookie.setPortlist("" + port );
+                                if (shouldAcceptInternal(uri, cookie)) {
+                                    cookieJar.add(uri, cookie);
+                                }
+                            } else {
+                                // Only store cookies with a port list
+                                // IF the URI port is in that list, as per
+                                // RFC 2965 section 3.3.2
+                                if (isInPortList(ports, port) &&
+                                        shouldAcceptInternal(uri, cookie)) {
+                                    cookieJar.add(uri, cookie);
+                                }
+                            }
+                        } else {
+                            if (shouldAcceptInternal(uri, cookie)) {
+                                cookieJar.add(uri, cookie);
+                            }
                         }
                     }
                 } catch (IllegalArgumentException e) {
@@ -276,6 +334,32 @@ public class CookieManager extends CookieHandler
     }
 
 
+    static private boolean isInPortList(String lst, int port) {
+        int i = lst.indexOf(",");
+        int val = -1;
+        while (i > 0) {
+            try {
+                val = Integer.parseInt(lst.substring(0, i));
+                if (val == port) {
+                    return true;
+                }
+            } catch (NumberFormatException numberFormatException) {
+            }
+            lst = lst.substring(i+1);
+            i = lst.indexOf(",");
+        }
+        if (!lst.isEmpty()) {
+            try {
+                val = Integer.parseInt(lst);
+                if (val == port) {
+                    return true;
+                }
+            } catch (NumberFormatException numberFormatException) {
+            }
+        }
+        return false;
+    }
+
     /*
      * path-matches algorithm, as defined by RFC 2965
      */

src/share/classes/java/net/HttpCookie.java

@@ -92,9 +92,14 @@ public final class HttpCookie implements Cloneable {
 
 
     //
-    // date format used by Netscape's cookie draft
+    // date formats used by Netscape's cookie draft
+    // as well as formats seen on various sites
     //
-    private final static String NETSCAPE_COOKIE_DATE_FORMAT = "EEE',' dd-MMM-yyyy HH:mm:ss 'GMT'";
+    private final static String[] COOKIE_DATE_FORMATS = {
+        "EEE',' dd-MMM-yyyy HH:mm:ss 'GMT'",
+        "EEE',' dd MMM yyyy HH:mm:ss 'GMT'",
+        "EEE MMM dd yyyy HH:mm:ss 'GMT'Z"
+    };
 
     //
     // constant strings represent set-cookie header token
@@ -148,6 +153,7 @@ public final class HttpCookie implements Cloneable {
         secure = false;
 
         whenCreated = System.currentTimeMillis();
+        portlist = null;
     }
 
 
@@ -505,14 +511,14 @@ public final class HttpCookie implements Cloneable {
 
 
     /**
-     * Indicates to the browser whether the cookie should only be sent
-     * using a secure protocol, such as HTTPS or SSL.
+     * Indicates whether the cookie should only be sent using a secure protocol,
+     * such as HTTPS or SSL.
      *
      * <p>The default value is <code>false</code>.
      *
-     * @param flag      if <code>true</code>, sends the cookie from the browser
-     *                  to the server using only when using a secure protocol;
-     *                  if <code>false</code>, sent on any protocol
+     * @param flag      If <code>true</code>, the cookie can only be sent over
+     *                  a secure protocol like https.
+     *                  If <code>false</code>, it can be sent over any protocol.
      *
      * @see #getSecure
      *
@@ -526,12 +532,12 @@ public final class HttpCookie implements Cloneable {
 
 
     /**
-     * Returns <code>true</code> if the browser is sending cookies
-     * only over a secure protocol, or <code>false</code> if the
-     * browser can send cookies using any protocol.
+     * Returns <code>true</code> if sending this cookie should be
+     * restricted to a secure protocol, or <code>false</code> if the
+     * it can be sent using any protocol.
      *
-     * @return          <code>true</code> if the browser can use
-     *                  any standard protocol; otherwise, <code>false</code>
+     * @return          <code>false</code> if the cookie can be sent over
+     *                  any standard protocol; otherwise, <code>true</code>
      *
      * @see #setSecure
      *
@@ -748,6 +754,7 @@ public final class HttpCookie implements Cloneable {
      *
      * @return  a string form of the cookie. The string has the defined format
      */
+    @Override
     public String toString() {
         if (getVersion() > 0) {
             return toRFC2965HeaderString();
@@ -768,6 +775,7 @@ public final class HttpCookie implements Cloneable {
      * @return          <tt>true</tt> if 2 http cookies equal to each other;
      *                  otherwise, <tt>false</tt>
      */
+    @Override
     public boolean equals(Object obj) {
         if (obj == this)
             return true;
@@ -798,6 +806,7 @@ public final class HttpCookie implements Cloneable {
      *
      * @return          this http cookie's hash code
      */
+    @Override
     public int hashCode() {
         int h1 = name.toLowerCase().hashCode();
         int h2 = (domain!=null) ? domain.toLowerCase().hashCode() : 0;
@@ -811,6 +820,7 @@ public final class HttpCookie implements Cloneable {
      *
      * @return          a clone of this http cookie
      */
+    @Override
     public Object clone() {
         try {
             return super.clone();
@@ -978,7 +988,7 @@ public final class HttpCookie implements Cloneable {
             });
         assignors.put("port", new CookieAttributeAssignor(){
                 public void assign(HttpCookie cookie, String attrName, String attrValue) {
-                    if (cookie.getPortlist() == null) cookie.setPortlist(attrValue);
+                    if (cookie.getPortlist() == null) cookie.setPortlist(attrValue == null ? "" : attrValue);
                 }
             });
         assignors.put("secure", new CookieAttributeAssignor(){
@@ -1050,24 +1060,31 @@ public final class HttpCookie implements Cloneable {
         return sb.toString();
     }
 
+    private static SimpleDateFormat[] cDateFormats = null;
+    static {
+            cDateFormats = new SimpleDateFormat[COOKIE_DATE_FORMATS.length];
+            for (int i = 0; i < COOKIE_DATE_FORMATS.length; i++) {
+                cDateFormats[i] = new SimpleDateFormat(COOKIE_DATE_FORMATS[i]);
+                cDateFormats[i].setTimeZone(TimeZone.getTimeZone("GMT"));
+            }
+    }
     /*
-     * @param dateString        a date string in format of
-     *                          "EEE',' dd-MMM-yyyy HH:mm:ss 'GMT'",
-     *                          which defined in Netscape cookie spec
+     * @param dateString        a date string in one of the formats
+     *                          defined in Netscape cookie spec
      *
      * @return                  delta seconds between this cookie's creation
      *                          time and the time specified by dateString
      */
     private long expiryDate2DeltaSeconds(String dateString) {
-        SimpleDateFormat df = new SimpleDateFormat(NETSCAPE_COOKIE_DATE_FORMAT);
-        df.setTimeZone(TimeZone.getTimeZone("GMT"));
+        for (SimpleDateFormat df : cDateFormats) {
+            try {
+                Date date = df.parse(dateString);
+                return (date.getTime() - whenCreated) / 1000;
+            } catch (Exception e) {
 
-        try {
-            Date date = df.parse(dateString);
-            return (date.getTime() - whenCreated) / 1000;
-        } catch (Exception e) {
-            return 0;
+            }
         }
+        return 0;
     }
 
 

src/share/classes/sun/net/www/protocol/http/InMemoryCookieStore.java

@@ -35,7 +35,6 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Collections;
 import java.util.Iterator;
-import java.util.Comparator;
 import java.util.concurrent.locks.ReentrantLock;
 
 /**
@@ -89,7 +88,9 @@ public class InMemoryCookieStore implements CookieStore {
             if (cookie.getMaxAge() != 0) {
                 cookieJar.add(cookie);
                 // and add it to domain index
-                addIndex(domainIndex, cookie.getDomain(), cookie);
+                if (cookie.getDomain() != null) {
+                    addIndex(domainIndex, cookie.getDomain(), cookie);
+                }
                 // add it to uri index, too
                 addIndex(uriIndex, getEffectiveURI(uri), cookie);
             }
@@ -113,12 +114,13 @@ public class InMemoryCookieStore implements CookieStore {
         }
 
         List<HttpCookie> cookies = new ArrayList<HttpCookie>();
+        boolean secureLink = "https".equalsIgnoreCase(uri.getScheme());
         lock.lock();
         try {
             // check domainIndex first
-            getInternal(cookies, domainIndex, new DomainComparator(uri.getHost()));
+            getInternal1(cookies, domainIndex, uri.getHost(), secureLink);
             // check uriIndex then
-            getInternal(cookies, uriIndex, getEffectiveURI(uri));
+            getInternal2(cookies, uriIndex, getEffectiveURI(uri), secureLink);
         } finally {
             lock.unlock();
         }
@@ -217,19 +219,96 @@ public class InMemoryCookieStore implements CookieStore {
     /* ---------------- Private operations -------------- */
 
 
-    static class DomainComparator implements Comparable<String> {
-        String host = null;
+    /*
+     * This is almost the same as HttpCookie.domainMatches except for
+     * one difference: It won't reject cookies when the 'H' part of the
+     * domain contains a dot ('.').
+     * I.E.: RFC 2965 section 3.3.2 says that if host is x.y.domain.com
+     * and the cookie domain is .domain.com, then it should be rejected.
+     * However that's not how the real world works. Browsers don't reject and
+     * some sites, like yahoo.com do actually expect these cookies to be
+     * passed along.
+     * And should be used for 'old' style cookies (aka Netscape type of cookies)
+     */
+    private boolean netscapeDomainMatches(String domain, String host)
+    {
+        if (domain == null || host == null) {
+            return false;
+        }
+
+        // if there's no embedded dot in domain and domain is not .local
+        boolean isLocalDomain = ".local".equalsIgnoreCase(domain);
+        int embeddedDotInDomain = domain.indexOf('.');
+        if (embeddedDotInDomain == 0) {
+            embeddedDotInDomain = domain.indexOf('.', 1);
+        }
+        if (!isLocalDomain && (embeddedDotInDomain == -1 || embeddedDotInDomain == domain.length() - 1)) {
+            return false;
+        }
+
+        // if the host name contains no dot and the domain name is .local
+        int firstDotInHost = host.indexOf('.');
+        if (firstDotInHost == -1 && isLocalDomain) {
+            return true;
+        }
 
-        public DomainComparator(String host) {
-            this.host = host;
+        int domainLength = domain.length();
+        int lengthDiff = host.length() - domainLength;
+        if (lengthDiff == 0) {
+            // if the host name and the domain name are just string-compare euqal
+            return host.equalsIgnoreCase(domain);
+        } else if (lengthDiff > 0) {
+            // need to check H & D component
+            String H = host.substring(0, lengthDiff);
+            String D = host.substring(lengthDiff);
+
+            return (D.equalsIgnoreCase(domain));
+        } else if (lengthDiff == -1) {
+            // if domain is actually .host
+            return (domain.charAt(0) == '.' &&
+                    host.equalsIgnoreCase(domain.substring(1)));
         }
 
-        public int compareTo(String domain) {
-            if (HttpCookie.domainMatches(domain, host)) {
-                return 0;
-            } else {
-                return -1;
+        return false;
+    }
+
+    private void getInternal1(List<HttpCookie> cookies, Map<String, List<HttpCookie>> cookieIndex,
+            String host, boolean secureLink) {
+        // Use a separate list to handle cookies that need to be removed so
+        // that there is no conflict with iterators.
+        ArrayList<HttpCookie> toRemove = new ArrayList<HttpCookie>();
+        for (Map.Entry<String, List<HttpCookie>> entry : cookieIndex.entrySet()) {
+            String domain = entry.getKey();
+            List<HttpCookie> lst = entry.getValue();
+            for (HttpCookie c : lst) {
+                if ((c.getVersion() == 0 && netscapeDomainMatches(domain, host)) ||
+                        (c.getVersion() == 1 && HttpCookie.domainMatches(domain, host))) {
+                    if ((cookieJar.indexOf(c) != -1)) {
+                        // the cookie still in main cookie store
+                        if (!c.hasExpired()) {
+                            // don't add twice and make sure it's the proper
+                            // security level
+                            if ((secureLink || !c.getSecure()) &&
+                                    !cookies.contains(c)) {
+                                cookies.add(c);
+                            }
+                        } else {
+                            toRemove.add(c);
+                        }
+                    } else {
+                        // the cookie has beed removed from main store,
+                        // so also remove it from domain indexed store
+                        toRemove.add(c);
+                    }
+                }
+            }
+            // Clear up the cookies that need to be removed
+            for (HttpCookie c : toRemove) {
+                lst.remove(c);
+                cookieJar.remove(c);
+
             }
+            toRemove.clear();
         }
     }
 
@@ -237,9 +316,9 @@ public class InMemoryCookieStore implements CookieStore {
     // @param cookieIndex       the index
     // @param comparator        the prediction to decide whether or not
     //                          a cookie in index should be returned
-    private <T> void getInternal(List<HttpCookie> cookies,
+    private <T> void getInternal2(List<HttpCookie> cookies,
                                 Map<T, List<HttpCookie>> cookieIndex,
-                                Comparable<T> comparator)
+                                Comparable<T> comparator, boolean secureLink)
     {
         for (T index : cookieIndex.keySet()) {
             if (comparator.compareTo(index) == 0) {
@@ -253,7 +332,8 @@ public class InMemoryCookieStore implements CookieStore {
                             // the cookie still in main cookie store
                             if (!ck.hasExpired()) {
                                 // don't add twice
-                                if (!cookies.contains(ck))
+                                if ((secureLink || !ck.getSecure()) &&
+                                        !cookies.contains(ck))
                                     cookies.add(ck);
                             } else {
                                 it.remove();
@@ -292,14 +372,14 @@ public class InMemoryCookieStore implements CookieStore {
 
 
     //
-    // for cookie purpose, the effective uri should only be scheme://authority
+    // for cookie purpose, the effective uri should only be http://host
     // the path will be taken into account when path-match algorithm applied
     //
     private URI getEffectiveURI(URI uri) {
         URI effectiveURI = null;
         try {
-            effectiveURI = new URI(uri.getScheme(),
-                                   uri.getAuthority(),
+            effectiveURI = new URI("http",
+                                   uri.getHost(),
                                    null,  // path component
                                    null,  // query component
                                    null   // fragment component

test/java/net/CookieHandler/B6644726.java

+/*
+ * 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 6644726
+ * @summary Cookie management issues
+ */
+
+import java.net.*;
+import java.util.*;
+
+public class B6644726 {
+    public static void main(String[] args) throws Exception {
+        testCookieStore();
+    }
+
+    private static void testCookieStore() throws Exception {
+        CookieManager cm = new CookieManager();
+        CookieStore cs = cm.getCookieStore();
+        URI uri = new URI("http://www.s1.sun.com/dir/foo/doc.html");
+        URI suri = new URI("https://www.s1.sun.com/dir/foo/index.html");
+        cm.setCookiePolicy(CookiePolicy.ACCEPT_ALL);
+
+        ArrayList<String> lst = new ArrayList<String>();
+        // Let's test the default path
+        lst.add("myCookie1=foo");
+        // Then some alternate expires format
+        lst.add("myCookie2=bar; path=/dir; expires=Tue, 19 Aug 2025 16:00:00 GMT");
+        lst.add("myCookie3=test; path=/dir; expires=Tue Aug 19 2025 16:00:00 GMT-0100");
+        // Then Netscape draft cookies and domains
+        lst.add("myCookie4=test; domain=.sun.com; path=/dir/foo");
+        HashMap<String, List<String>> map = new HashMap<String, List<String>>();
+        map.put("Set-Cookie", lst);
+        cm.put(uri, map);
+        map.clear();
+        lst.clear();
+        // Test for secure tag
+        lst.add("myCookie5=test; secure");
+        // Test for passing cookies between http and https
+        map.put("Set-Cookie", lst);
+        cm.put(suri, map);
+
+        List<HttpCookie> cookies = cs.getCookies();
+        // There should be 5 cookies if all dates parsed correctly
+        if (cookies.size() != 5) {
+            fail("Should have 5 cookies. Got only "+ cookies.size() + ", expires probably didn't parse correctly");
+        }
+        // Check Path for first Cookie
+        for (HttpCookie c : cookies) {
+            if (c.getName().equals("myCookie1")) {
+                if (!"/dir/foo/".equals(c.getPath())) {
+                    fail("Default path for myCookie1 is " + c.getPath());
+                }
+            }
+        }
+
+        HashMap<String, List<String>> emptyMap = new HashMap<String, List<String>>();
+        // We should get 1 Cookie: MyCookie4, because of the domain
+        Map<String, List<String>>m = cm.get(new URI("http://www.s2.sun.com/dir/foo/doc2.html"),
+                emptyMap);
+        List<String> clst = m.get("Cookie");
+        if (clst.size() != 1) {
+            fail("We should have only 1 cookie, not " + clst.size());
+        } else {
+            if (!clst.get(0).startsWith("myCookie4")) {
+                fail("The cookie should be myCookie4, not " + clst.get(0));
+            }
+        }
+        // We should get 4 cookies for non secure URI, and 5 for the secure one
+        m = cm.get(suri, emptyMap);
+        clst = m.get("Cookie");
+        if (clst.size() != 5) {
+            fail("Cookies didn't cross from http to https. Got only " + clst.size());
+        }
+
+        m = cm.get(uri, emptyMap);
+        clst = m.get("Cookie");
+        if (clst.size() != 4) {
+            fail("We should have gotten only 4 cookies over http (non secure), got " +
+                    clst.size());
+        }
+        if (isIn(clst, "myCookie5=")) {
+            // myCookie5 (the secure one) shouldn't be here
+            fail("Got the secure cookie over a non secure link");
+        }
+
+        // Let's check that empty path is treated correctly
+        uri = new URI("http://www.sun.com/");
+        lst.clear();
+        lst.add("myCookie6=foo");
+        map.clear();
+        map.put("Set-Cookie", lst);
+        cm.put(uri, map);
+        uri = new URI("http://www.sun.com");
+        m = cm.get(uri, emptyMap);
+        clst = m.get("Cookie");
+        if (clst.size() != 1) {
+            fail("Missing a cookie when using an empty path");
+        }
+
+        // And now, the other way around:
+
+        uri = new URI("http://www.sun.com");
+        lst.clear();
+        lst.add("myCookie7=foo");
+        map.clear();
+        map.put("Set-Cookie", lst);
+        cm.put(uri, map);
+        uri = new URI("http://www.sun.com/");
+        m = cm.get(uri, emptyMap);
+        clst = m.get("Cookie");
+        if (!isIn(clst, "myCookie7=")) {
+            fail("Missing a cookie when using an empty path");
+        }
+
+        // Let's make sure the 'Port' optional attributes is enforced
+
+        lst.clear();
+        lst.add("myCookie8=porttest; port");
+        lst.add("myCookie9=porttest; port=\"80,8000\"");
+        lst.add("myCookie10=porttest; port=\"8000\"");
+        map.clear();
+        map.put("Set-Cookie", lst);
+        uri = new URI("http://www.sun.com/");
+        cm.put(uri, map);
+
+        // myCookie10 should have been rejected
+        cookies = cs.getCookies();
+        for (HttpCookie c : cookies) {
+            if (c.getName().equals("myCookie10")) {
+                fail("A cookie with an invalid port list was accepted");
+            }
+        }
+
+        uri = new URI("http://www.sun.com:80/");
+        m = cm.get(uri, emptyMap);
+        clst = m.get("Cookie");
+        // We should find both myCookie8 and myCookie9 but not myCookie10
+        if (!isIn(clst, "myCookie8=") || !isIn(clst, "myCookie9=")) {
+            fail("Missing a cookie on port 80");
+        }
+        uri = new URI("http://www.sun.com:8000/");
+        m = cm.get(uri, emptyMap);
+        clst = m.get("Cookie");
+        // We should find only myCookie9
+        if (!isIn(clst, "myCookie9=")) {
+            fail("Missing a cookie on port 80");
+        }
+        if (isIn(clst, "myCookie8=")) {
+            fail("A cookie with an invalid port list was returned");
+        }
+    }
+
+    private static boolean isIn(List<String> lst, String cookie) {
+        if (lst == null || lst.isEmpty()) {
+            return false;
+        }
+        for (String s : lst) {
+            if (s.startsWith(cookie))
+                return true;
+        }
+        return false;
+    }
+
+    private static void fail(String msg) throws Exception {
+        throw new RuntimeException(msg);
+    }
+}