# HG changeset patch # User jccollet # Date 1208423133 -7200 # Node ID c309ca1d3a86eeeaf853fb123b5827cd7ed319a1 # Parent c6ddcfc7ff4dfd73d9e10d3f508a5cc6f600f9da 6644726: Cookie management issues Summary: Many changes to accomodate RFC 2965 and old Netscape specs Reviewed-by: chegar diff -r c6ddcfc7ff4d -r c309ca1d3a86 jdk/src/share/classes/java/net/CookieManager.java --- a/jdk/src/share/classes/java/net/CookieManager.java Wed Apr 16 14:17:54 2008 +0100 +++ b/jdk/src/share/classes/java/net/CookieManager.java Thu Apr 17 11:05:33 2008 +0200 @@ -205,11 +205,31 @@ if (cookieJar == null) return Collections.unmodifiableMap(cookieMap); + boolean secureLink = "https".equalsIgnoreCase(uri.getScheme()); List cookies = new java.util.ArrayList(); + 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 @@ try { List 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 @@ } + 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 */ diff -r c6ddcfc7ff4d -r c309ca1d3a86 jdk/src/share/classes/java/net/HttpCookie.java --- a/jdk/src/share/classes/java/net/HttpCookie.java Wed Apr 16 14:17:54 2008 +0100 +++ b/jdk/src/share/classes/java/net/HttpCookie.java Thu Apr 17 11:05:33 2008 +0200 @@ -92,9 +92,14 @@ // - // 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 @@ secure = false; whenCreated = System.currentTimeMillis(); + portlist = null; } @@ -505,14 +511,14 @@ /** - * 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. * *

The default value is false. * - * @param flag if true, sends the cookie from the browser - * to the server using only when using a secure protocol; - * if false, sent on any protocol + * @param flag If true, the cookie can only be sent over + * a secure protocol like https. + * If false, it can be sent over any protocol. * * @see #getSecure * @@ -526,12 +532,12 @@ /** - * Returns true if the browser is sending cookies - * only over a secure protocol, or false if the - * browser can send cookies using any protocol. + * Returns true if sending this cookie should be + * restricted to a secure protocol, or false if the + * it can be sent using any protocol. * - * @return true if the browser can use - * any standard protocol; otherwise, false + * @return false if the cookie can be sent over + * any standard protocol; otherwise, true * * @see #setSecure * @@ -748,6 +754,7 @@ * * @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 @@ * @return true if 2 http cookies equal to each other; * otherwise, false */ + @Override public boolean equals(Object obj) { if (obj == this) return true; @@ -798,6 +806,7 @@ * * @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 @@ * * @return a clone of this http cookie */ + @Override public Object clone() { try { return super.clone(); @@ -978,7 +988,7 @@ }); 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 @@ 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; } diff -r c6ddcfc7ff4d -r c309ca1d3a86 jdk/src/share/classes/sun/net/www/protocol/http/InMemoryCookieStore.java --- a/jdk/src/share/classes/sun/net/www/protocol/http/InMemoryCookieStore.java Wed Apr 16 14:17:54 2008 +0100 +++ b/jdk/src/share/classes/sun/net/www/protocol/http/InMemoryCookieStore.java Thu Apr 17 11:05:33 2008 +0200 @@ -35,7 +35,6 @@ 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 @@ 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 @@ } List cookies = new ArrayList(); + 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 @@ /* ---------------- Private operations -------------- */ - static class DomainComparator implements Comparable { - 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; + } - public DomainComparator(String host) { - this.host = host; + // 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 int compareTo(String domain) { - if (HttpCookie.domainMatches(domain, host)) { - return 0; - } else { - return -1; + 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))); + } + + return false; + } + + private void getInternal1(List cookies, Map> 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 toRemove = new ArrayList(); + for (Map.Entry> entry : cookieIndex.entrySet()) { + String domain = entry.getKey(); + List 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 @@ // @param cookieIndex the index // @param comparator the prediction to decide whether or not // a cookie in index should be returned - private void getInternal(List cookies, + private void getInternal2(List cookies, Map> cookieIndex, - Comparable comparator) + Comparable comparator, boolean secureLink) { for (T index : cookieIndex.keySet()) { if (comparator.compareTo(index) == 0) { @@ -253,7 +332,8 @@ // 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 @@ // - // 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 diff -r c6ddcfc7ff4d -r c309ca1d3a86 jdk/test/java/net/CookieHandler/B6644726.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/jdk/test/java/net/CookieHandler/B6644726.java Thu Apr 17 11:05:33 2008 +0200 @@ -0,0 +1,189 @@ +/* + * 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 lst = new ArrayList(); + // 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> map = new HashMap>(); + 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 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> emptyMap = new HashMap>(); + // We should get 1 Cookie: MyCookie4, because of the domain + Map>m = cm.get(new URI("http://www.s2.sun.com/dir/foo/doc2.html"), + emptyMap); + List 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 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); + } +}