relanding bug 385299 and updating tests.

netwerk/cookie/src/nsCookieService.cpp

@@ -54,6 +54,7 @@
 #include "nsIFile.h"
 #include "nsIObserverService.h"
 #include "nsILineInputStream.h"
+#include "nsIEffectiveTLDService.h"
 
 #include "nsCOMArray.h"
 #include "nsArrayEnumerator.h"
@@ -62,7 +63,6 @@
 #include "nsCRT.h"
 #include "prtime.h"
 #include "prprf.h"
-#include "prnetdb.h"
 #include "nsNetUtil.h"
 #include "nsNetCID.h"
 #include "nsAppDirectoryServiceDefs.h"
@@ -422,6 +422,10 @@ nsCookieService::Init()
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
+  nsresult rv;
+  mTLDService = do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   // init our pref and observer
   nsCOMPtr<nsIPrefBranch2> prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID);
   if (prefBranch) {
@@ -433,7 +437,7 @@ nsCookieService::Init()
 
   // ignore failure here, since it's non-fatal (we can run fine without
   // persistent storage - e.g. if there's no profile)
-  nsresult rv = InitDB();
+  rv = InitDB();
   if (NS_FAILED(rv))
     COOKIE_LOGSTRING(PR_LOG_WARNING, ("Init(): InitDB() gave error %x", rv));
 
@@ -611,11 +615,9 @@ nsCookieService::Observe(nsISupports     *aSubject,
 
     if (!nsCRT::strcmp(aData, NS_LITERAL_STRING("shutdown-cleanse").get()) && mDBConn) {
       // clear the cookie file
-      if (mDBConn) {
+      nsresult rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("DELETE FROM moz_cookies"));
-        nsresult rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("DELETE FROM moz_cookies"));
+      if (NS_FAILED(rv))
-        if (NS_FAILED(rv))
+        NS_WARNING("db delete failed");
-          NS_WARNING("db delete failed");
-      }
     }
 
   } else if (!strcmp(aTopic, "profile-do-change")) {
@@ -1128,7 +1130,6 @@ nsCookieService::GetCookieInternal(nsIURI      *aHostURI,
   // insert a leading dot, so we begin the hash lookup with the
   // equivalent domain cookie host
   hostFromURI.Insert(NS_LITERAL_CSTRING("."), 0);
-  ToLowerCase(hostFromURI);
 
   // check if aHostURI is using an https secure protocol.
   // if it isn't, then we can't send a secure cookie over the connection.
@@ -1700,58 +1701,6 @@ nsCookieService::ParseAttributes(nsDependentCString &aCookieHeader,
  * private domain & permission compliance enforcement functions
  ******************************************************************************/
 
-// returns PR_TRUE if aHost is an IP address
-PRBool
-nsCookieService::IsIPAddress(const nsAFlatCString &aHost)
-{
-  PRNetAddr addr;
-  return (PR_StringToNetAddr(aHost.get(), &addr) == PR_SUCCESS);
-}
-
-PRBool
-nsCookieService::IsInDomain(const nsACString &aDomain,
-                            const nsACString &aHost,
-                            PRBool           aIsDomain)
-{
-  // if we have a non-domain cookie, require an exact match between domain and host.
-  // RFC2109 specifies this behavior; it allows a site to prevent its subdomains
-  // from accessing a cookie, for whatever reason.
-  if (!aIsDomain) {
-    return aDomain.Equals(aHost);
-  }
-
-  // we have a domain cookie; test the following two cases:
-  /*
-   * normal case for hostName = x<domainName>
-   *    e.g., hostName = home.netscape.com
-   *          domainName = .netscape.com
-   *
-   * special case for domainName = .hostName
-   *    e.g., hostName = netscape.com
-   *          domainName = .netscape.com
-   */
-  // the lengthDifference tests are for efficiency, so we do only one .Equals()
-  PRUint32 domainLength = aDomain.Length();
-  PRInt32 lengthDifference = aHost.Length() - domainLength;
-  // case for host & domain equal
-  // (e.g. .netscape.com & .netscape.com)
-  // this gives us slightly more efficiency, since we don't have
-  // to call up Substring().
-  if (lengthDifference == 0) {
-    return aDomain.Equals(aHost);
-  }
-  // normal case
-  if (lengthDifference > 0) {
-    return aDomain.Equals(Substring(aHost, lengthDifference, domainLength));
-  }
-  // special case
-  if (lengthDifference == -1) {
-    return Substring(aDomain, 1, domainLength - 1).Equals(aHost);
-  }
-  // no match
-  return PR_FALSE;
-}
-
 PRBool
 nsCookieService::IsForeign(nsIURI *aHostURI,
                            nsIURI *aFirstURI)
@@ -1761,15 +1710,6 @@ nsCookieService::IsForeign(nsIURI *aHostURI,
     return PR_FALSE;
   }
 
-  // chrome URLs are never foreign (otherwise sidebar cookies won't work).
-  // eventually we want to have a protocol whitelist here,
-  // _or_ do something smart with nsIProtocolHandler::protocolFlags.
-  PRBool isChrome = PR_FALSE;
-  nsresult rv = aFirstURI->SchemeIs("chrome", &isChrome);
-  if (NS_SUCCEEDED(rv) && isChrome) {
-    return PR_FALSE;
-  }
-
   // Get hosts
   nsCAutoString currentHost, firstHost;
   if (NS_FAILED(aHostURI->GetAsciiHost(currentHost)) ||
@@ -1779,46 +1719,53 @@ nsCookieService::IsForeign(nsIURI *aHostURI,
   // trim trailing dots
   currentHost.Trim(".");
   firstHost.Trim(".");
-  ToLowerCase(currentHost);
-  ToLowerCase(firstHost);
 
-  // determine if it's foreign. we have a new algorithm for doing this,
+  // fast path: check if the two hosts are identical.
-  // since the old behavior was broken:
+  // this also covers two special cases:
+  // 1) if we're dealing with IP addresses, require an exact match. this
+  // eliminates any chance of IP address funkiness (e.g. the alias 127.1
+  // domain-matching 99.54.127.1). bug 105917 originally noted the requirement
+  // to deal with IP addresses. note that GetBaseDomain() below will return an
+  // error if the URI is an IP address.
+  // 2) we also need this for the (rare) case where the site is actually an eTLD,
+  // e.g. http://co.tv; GetBaseDomain() will throw an error and we might
+  // erroneously think currentHost is foreign. so we consider this case non-
+  // foreign only if the hosts exactly match.
+  if (firstHost.Equals(currentHost))
+    return PR_FALSE;
 
-  // first ensure we're not dealing with IP addresses; if we are, require an
+  // chrome URLs are never foreign (otherwise sidebar cookies won't work).
-  // exact match. we can't avoid this, otherwise the algo below will allow two
+  // eventually we want to have a protocol whitelist here,
-  // IP's such as 128.12.96.5 and 213.12.96.5 to match.
+  // _or_ do something smart with nsIProtocolHandler::protocolFlags.
-  if (IsIPAddress(firstHost)) {
+  PRBool isChrome = PR_FALSE;
-    return !IsInDomain(firstHost, currentHost, PR_FALSE);
+  nsresult rv = aFirstURI->SchemeIs("chrome", &isChrome);
+  if (NS_SUCCEEDED(rv) && isChrome) {
+    return PR_FALSE;
   }
 
-  // next, allow a one-subdomain-level "fuzz" in the comparison. first, we need
+  // get the base domain for the originating URI.
-  // to find how many subdomain levels each host has; we only do the looser
+  // e.g. for "images.bbc.co.uk", this would be "bbc.co.uk".
-  // comparison if they have the same number of levels. e.g.
+  nsCAutoString baseDomain;
-  //  firstHost = weather.yahoo.com, currentHost = cookies.yahoo.com -> match
+  rv = mTLDService->GetBaseDomain(aFirstURI, 0, baseDomain);
-  //  firstHost =     a.b.yahoo.com, currentHost =       b.yahoo.com -> no match
+  if (NS_FAILED(rv)) {
-  //  firstHost =         yahoo.com, currentHost = weather.yahoo.com -> no match
+    // URI is an IP, eTLD, or something else went wrong - assume foreign
-  //  (since the normal test (next) will catch this case and give a match.)
+    return PR_TRUE;
-  // also, we can only do this if they have >=2 subdomain levels, to avoid
+  }  
-  // matching yahoo.com with netscape.com (yes, this breaks for .co.nz etc...)
+  baseDomain.Trim(".");
-  PRUint32 dotsInFirstHost = firstHost.CountChar('.');
-  if (dotsInFirstHost == currentHost.CountChar('.') &&
-      dotsInFirstHost >= 2) {
-    // we have enough dots - check IsInDomain(choppedFirstHost, currentHost)
-    PRInt32 dot1 = firstHost.FindChar('.');
-    return !IsInDomain(Substring(firstHost, dot1, firstHost.Length() - dot1), currentHost);
-  }
 
-  // don't have enough dots to chop firstHost, or the subdomain levels differ;
+  // ensure the host domain is derived from the base domain.
-  // so we just do the plain old check, IsInDomain(firstHost, currentHost).
+  // we prepend dots before the comparison to ensure e.g.
-  return !IsInDomain(NS_LITERAL_CSTRING(".") + firstHost, currentHost);
+  // "mybbc.co.uk" isn't matched as a superset of "bbc.co.uk".
+  currentHost.Insert(NS_LITERAL_CSTRING("."), 0);
+  baseDomain.Insert(NS_LITERAL_CSTRING("."), 0);
+  return !StringEndsWith(currentHost, baseDomain);
 }
 
 PRUint32
-nsCookieService::CheckPrefs(nsIURI         *aHostURI,
+nsCookieService::CheckPrefs(nsIURI     *aHostURI,
-                            nsIURI         *aFirstURI,
+                            nsIURI     *aFirstURI,
-                            nsIChannel     *aChannel,
+                            nsIChannel *aChannel,
-                            const char     *aCookieHeader)
+                            const char *aCookieHeader)
 {
   // pref tree:
   // 0) get the scheme strings from the two URI's
@@ -1895,6 +1842,8 @@ PRBool
 nsCookieService::CheckDomain(nsCookieAttributes &aCookieAttributes,
                              nsIURI             *aHostURI)
 {
+  nsresult rv;
+
   // get host from aHostURI
   nsCAutoString hostFromURI;
   if (NS_FAILED(aHostURI->GetAsciiHost(hostFromURI))) {
@@ -1902,7 +1851,6 @@ nsCookieService::CheckDomain(nsCookieAttributes &aCookieAttributes,
   }
   // trim trailing dots
   hostFromURI.Trim(".");
-  ToLowerCase(hostFromURI);
 
   // if a domain is given, check the host has permission
   if (!aCookieAttributes.host.IsEmpty()) {
@@ -1910,32 +1858,33 @@ nsCookieService::CheckDomain(nsCookieAttributes &aCookieAttributes,
     // switch to lowercase now, to avoid case-insensitive compares everywhere
     ToLowerCase(aCookieAttributes.host);
 
-    // check whether the host is an IP address, and override isDomain to
+    // get the base domain for the host URI.
-    // make the cookie a non-domain one. this will require an exact host
+    // e.g. for "images.bbc.co.uk", this would be "bbc.co.uk", which
-    // match for the cookie, so we eliminate any chance of IP address
+    // represents the lowest level domain a cookie can be set for.
-    // funkiness (e.g. the alias 127.1 domain-matching 99.54.127.1).
+    nsCAutoString baseDomain;
-    // bug 105917 originally noted the requirement to deal with IP addresses.
+    rv = mTLDService->GetBaseDomain(aHostURI, 0, baseDomain);
-    if (IsIPAddress(aCookieAttributes.host)) {
+    baseDomain.Trim(".");
-      return IsInDomain(aCookieAttributes.host, hostFromURI, PR_FALSE);
+    if (NS_FAILED(rv)) {
-    }
+      // check whether the host is an IP address, and leave the cookie as
+      // a non-domain one. this will require an exact host match for the cookie,
+      // so we eliminate any chance of IP address funkiness (e.g. the alias 127.1
+      // domain-matching 99.54.127.1). bug 105917 originally noted the
+      // requirement to deal with IP addresses.
+      if (rv == NS_ERROR_HOST_IS_IP_ADDRESS)
+        return hostFromURI.Equals(aCookieAttributes.host);
 
-    /*
-     * verify that this host has the authority to set for this domain.   We do
-     * this by making sure that the host is in the domain.  We also require
-     * that a domain have at least one embedded period to prevent domains of the form
-     * ".com" and ".edu"
-     */
-    PRInt32 dot = aCookieAttributes.host.FindChar('.');
-    if (dot == kNotFound) {
-      // fail dot test
       return PR_FALSE;
     }
 
-    // prepend a dot, and check if the host is in the domain
+    // ensure the proposed domain is derived from the base domain; and also
+    // that the host domain is derived from the proposed domain (per RFC2109).
+    // we prepend a dot before the comparison to ensure e.g.
+    // "mybbc.co.uk" isn't matched as a superset of "bbc.co.uk".
+    hostFromURI.Insert(NS_LITERAL_CSTRING("."), 0);
     aCookieAttributes.host.Insert(NS_LITERAL_CSTRING("."), 0);
-    if (!IsInDomain(aCookieAttributes.host, hostFromURI)) {
+    baseDomain.Insert(NS_LITERAL_CSTRING("."), 0);
-      return PR_FALSE;
+    return StringEndsWith(aCookieAttributes.host, baseDomain) &&
-    }
+           StringEndsWith(hostFromURI, aCookieAttributes.host);
 
     /*
      * note: RFC2109 section 4.3.2 requires that we check the following:
@@ -1944,18 +1893,18 @@ nsCookieService::CheckDomain(nsCookieAttributes &aCookieAttributes,
      * entire .co.nz domain. however, it's only a only a partial solution and
      * it breaks sites (IE doesn't enforce it), so we don't perform this check.
      */
+  }
+
+  // block any URIs without a host that aren't file:/// URIs
+  if (hostFromURI.IsEmpty()) {
+    PRBool isFileURI = PR_FALSE;
+    aHostURI->SchemeIs("file", &isFileURI);
+    if (!isFileURI)
+      return PR_FALSE;
+  }
 
   // no domain specified, use hostFromURI
-  } else {
+  aCookieAttributes.host = hostFromURI;
-    // block any URIs without a host that aren't file:/// URIs
-    if (hostFromURI.IsEmpty()) {
-      PRBool isFileURI = PR_FALSE;
-      aHostURI->SchemeIs("file", &isFileURI);
-      if (!isFileURI)
-        return PR_FALSE;
-    }
-    aCookieAttributes.host = hostFromURI;
-  }
 
   return PR_TRUE;
 }

netwerk/cookie/src/nsCookieService.h

@@ -58,6 +58,7 @@ class nsAutoVoidArray;
 
 class nsIPrefBranch;
 class nsICookiePermission;
+class nsIEffectiveTLDService;
 class nsIPrefBranch;
 class nsIObserverService;
 class nsIURI;
@@ -176,11 +177,9 @@ class nsCookieService : public nsICookieService
     void                          UpdateCookieInList(nsCookie *aCookie, PRInt64 aLastAccessed);
     static PRBool                 GetTokenValue(nsASingleFragmentCString::const_char_iterator &aIter, nsASingleFragmentCString::const_char_iterator &aEndIter, nsDependentCSubstring &aTokenString, nsDependentCSubstring &aTokenValue, PRBool &aEqualsFound);
     static PRBool                 ParseAttributes(nsDependentCString &aCookieHeader, nsCookieAttributes &aCookie);
-    static PRBool                 IsIPAddress(const nsAFlatCString &aHost);
+    PRBool                        IsForeign(nsIURI *aHostURI, nsIURI *aFirstURI);
-    static PRBool                 IsInDomain(const nsACString &aDomain, const nsACString &aHost, PRBool aIsDomain = PR_TRUE);
-    static PRBool                 IsForeign(nsIURI *aHostURI, nsIURI *aFirstURI);
     PRUint32                      CheckPrefs(nsIURI *aHostURI, nsIURI *aFirstURI, nsIChannel *aChannel, const char *aCookieHeader);
-    static PRBool                 CheckDomain(nsCookieAttributes &aCookie, nsIURI *aHostURI);
+    PRBool                        CheckDomain(nsCookieAttributes &aCookie, nsIURI *aHostURI);
     static PRBool                 CheckPath(nsCookieAttributes &aCookie, nsIURI *aHostURI);
     static PRBool                 GetExpiry(nsCookieAttributes &aCookie, PRInt64 aServerTime, PRInt64 aCurrentTime);
     void                          RemoveAllFromMemory();
@@ -193,12 +192,13 @@ class nsCookieService : public nsICookieService
 
   protected:
     // cached members
-    nsCOMPtr<mozIStorageConnection> mDBConn;
+    nsCOMPtr<mozIStorageConnection>  mDBConn;
-    nsCOMPtr<mozIStorageStatement> mStmtInsert;
+    nsCOMPtr<mozIStorageStatement>   mStmtInsert;
-    nsCOMPtr<mozIStorageStatement> mStmtDelete;
+    nsCOMPtr<mozIStorageStatement>   mStmtDelete;
-    nsCOMPtr<mozIStorageStatement> mStmtUpdate;
+    nsCOMPtr<mozIStorageStatement>   mStmtUpdate;
-    nsCOMPtr<nsIObserverService>  mObserverService;
+    nsCOMPtr<nsIObserverService>     mObserverService;
-    nsCOMPtr<nsICookiePermission> mPermissionService;
+    nsCOMPtr<nsICookiePermission>    mPermissionService;
+    nsCOMPtr<nsIEffectiveTLDService> mTLDService;
 
     // impl members
     nsTHashtable<nsCookieEntry>   mHostTable;

netwerk/test/TestCookie.cpp

@@ -525,7 +525,7 @@ main(PRInt32 argc, char *argv[])
       rv[1] = CheckResult(cookie.get(), MUST_EQUAL, "test=foreign");
       SetACookie(cookieService, "http://moose.yahoo.com/", "http://canada.yahoo.com/", "test=foreign; domain=.yahoo.com", nsnull);
       GetACookie(cookieService, "http://yahoo.com/", "http://sport.yahoo.com/", getter_Copies(cookie));
-      rv[2] = CheckResult(cookie.get(), MUST_BE_NULL);
+      rv[2] = CheckResult(cookie.get(), MUST_EQUAL, "test=foreign");
       GetACookie(cookieService, "http://sport.yahoo.com/", "http://yahoo.com/", getter_Copies(cookie));
       rv[3] = CheckResult(cookie.get(), MUST_EQUAL, "test=foreign");
       SetACookie(cookieService, "http://jack.yahoo.com/", "http://jill.yahoo.com/", "test=foreign; domain=.yahoo.com; max-age=0", nsnull);
@@ -534,7 +534,7 @@ main(PRInt32 argc, char *argv[])
 
       SetACookie(cookieService, "http://moose.yahoo.com/", "http://foo.moose.yahoo.com/", "test=foreign; domain=.yahoo.com", nsnull);
       GetACookie(cookieService, "http://yahoo.com/", "http://yahoo.com/", getter_Copies(cookie));
-      rv[5] = CheckResult(cookie.get(), MUST_BE_NULL);
+      rv[5] = CheckResult(cookie.get(), MUST_EQUAL, "test=foreign");
       SetACookie(cookieService, "http://foo.bar.yahoo.com/", "http://yahoo.com/", "test=foreign; domain=.yahoo.com", nsnull);
       GetACookie(cookieService, "http://yahoo.com/", "http://yahoo.com/", getter_Copies(cookie));
       rv[6] = CheckResult(cookie.get(), MUST_EQUAL, "test=foreign");