6644726: Cookie management issues
Summary: Many changes to accomodate RFC 2965 and old Netscape specs
Reviewed-by: chegar
--- 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<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 @@
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 @@
}
+ 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
*/
--- 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.
*
* <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 @@
/**
- * 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 @@
*
* @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 <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 @@
*
* @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;
}
--- 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<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 @@
/* ---------------- 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;
+ }
- 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<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 @@
// @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 @@
// 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
--- /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<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);
+ }
+}