Bug 526789 - UMR in nsCookieService::CountCookiesFromHostInternal, GetCookiesFromHost, and
GetCookieInternal when given empty host string. r=biesi
--- a/extensions/cookie/test/unit/test_bug481775.js
+++ b/extensions/cookie/test/unit/test_bug481775.js
@@ -1,24 +1,22 @@
const Cc = Components.classes;
const Ci = Components.interfaces;
function run_test() {
- var cs = Cc["@mozilla.org/cookieService;1"].getService(Ci.nsICookieService);
var cm = Cc["@mozilla.org/cookiemanager;1"].getService(Ci.nsICookieManager2);
- var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
var pb = null;
try {
pb = Cc["@mozilla.org/privatebrowsing;1"].getService(Ci.nsIPrivateBrowsingService);
} catch (e) {}
// accept all cookies and clear the table
prefs.setIntPref("network.cookie.lifetimePolicy", 0);
- cs.removeAll();
+ cm.removeAll();
// saturate the cookie table
addCookies(0, 5000);
// check how many cookies we have
var count = getCookieCount();
do_check_neq(count, 0);
@@ -32,17 +30,17 @@ function run_test() {
// saturate the cookie table again
addCookies(5000, 5000);
// check we have the same number of cookies
do_check_eq(getCookieCount(), count);
// remove them all
- cs.removeAll();
+ cm.removeAll();
do_check_eq(getCookieCount(), 0);
// leave private browsing mode
pb.privateBrowsingEnabled = false;
}
// make sure our cookies are back
do_check_eq(getCookieCount(), count);
@@ -50,17 +48,17 @@ function run_test() {
// set a few more, to trigger a purge
addCookies(10000, 1000);
// check we have the same number of cookies
var count = getCookieCount();
do_check_eq(getCookieCount(), count);
// remove them all
- cs.removeAll();
+ cm.removeAll();
do_check_eq(getCookieCount(), 0);
}
function getCookieCount() {
var count = 0;
var cm = Cc["@mozilla.org/cookiemanager;1"].getService(Ci.nsICookieManager2);
var enumerator = cm.enumerator;
while (enumerator.hasMoreElements()) {
new file mode 100644
--- /dev/null
+++ b/extensions/cookie/test/unit/test_bug526789.js
@@ -0,0 +1,95 @@
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+const Cr = Components.results;
+
+function do_check_throws(f, result, stack)
+{
+ if (!stack)
+ stack = Components.stack.caller;
+
+ try {
+ f();
+ } catch (exc) {
+ if (exc.result == result)
+ return;
+ do_throw("expected result " + result + ", caught " + exc, stack);
+ }
+ do_throw("expected result " + result + ", none thrown", stack);
+}
+
+function run_test() {
+ var cs = Cc["@mozilla.org/cookieService;1"].getService(Ci.nsICookieService);
+ var cm = Cc["@mozilla.org/cookiemanager;1"].getService(Ci.nsICookieManager2);
+ var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
+
+ cm.removeAll();
+
+ // test that an empty host results in a no-op
+ var uri = ios.newURI("http://baz.com/", null, null);
+ var emptyuri = ios.newURI("http:///", null, null);
+ var doturi = ios.newURI("http://./", null, null);
+ do_check_eq(emptyuri.asciiHost, "");
+ do_check_eq(doturi.asciiHost, ".");
+ cs.setCookieString(emptyuri, null, "foo2=bar", null);
+ do_check_eq(getCookieCount(), 0);
+ cs.setCookieString(doturi, null, "foo3=bar", null);
+ do_check_eq(getCookieCount(), 0);
+ cs.setCookieString(uri, null, "foo=bar", null);
+ do_check_eq(getCookieCount(), 1);
+
+ do_check_eq(cs.getCookieString(uri, null), "foo=bar");
+ do_check_eq(cs.getCookieString(emptyuri, null), null);
+ do_check_eq(cs.getCookieString(doturi, null), null);
+
+ // test that an empty host to add() or remove() throws
+ var expiry = (Date.now() + 1000) * 1000;
+ do_check_throws(function() {
+ cm.add("", "/", "foo2", "bar", false, false, true, expiry);
+ }, Cr.NS_ERROR_ILLEGAL_VALUE);
+ do_check_eq(getCookieCount(), 1);
+ do_check_throws(function() {
+ cm.add(".", "/", "foo3", "bar", false, false, true, expiry);
+ }, Cr.NS_ERROR_ILLEGAL_VALUE);
+ do_check_eq(getCookieCount(), 1);
+ cm.add("test.com", "/", "foo", "bar", false, false, true, expiry);
+ do_check_eq(getCookieCount(), 2);
+
+ do_check_throws(function() {
+ cm.remove("", "foo2", "/", false);
+ }, Cr.NS_ERROR_ILLEGAL_VALUE);
+ do_check_eq(getCookieCount(), 2);
+ do_check_throws(function() {
+ cm.remove(".", "foo3", "/", false);
+ }, Cr.NS_ERROR_ILLEGAL_VALUE);
+ do_check_eq(getCookieCount(), 2);
+ cm.remove("test.com", "foo", "/", false);
+ do_check_eq(getCookieCount(), 1);
+
+ do_check_eq(cm.countCookiesFromHost("baz.com"), 1);
+ do_check_eq(cm.countCookiesFromHost(""), 0);
+ do_check_eq(cm.countCookiesFromHost("."), 0);
+
+ var e = cm.getCookiesFromHost("baz.com");
+ do_check_true(e.hasMoreElements());
+ do_check_eq(e.getNext().QueryInterface(Ci.nsICookie2).name, "foo");
+ do_check_false(e.hasMoreElements());
+ e = cm.getCookiesFromHost("");
+ do_check_false(e.hasMoreElements());
+ e = cm.getCookiesFromHost(".");
+ do_check_false(e.hasMoreElements());
+
+ cm.removeAll();
+}
+
+function getCookieCount() {
+ var count = 0;
+ var cm = Cc["@mozilla.org/cookiemanager;1"].getService(Ci.nsICookieManager2);
+ var enumerator = cm.enumerator;
+ while (enumerator.hasMoreElements()) {
+ if (!(enumerator.getNext() instanceof Ci.nsICookie2))
+ throw new Error("not a cookie");
+ ++count;
+ }
+ return count;
+}
+
--- a/netwerk/cookie/public/nsICookieManager.idl
+++ b/netwerk/cookie/public/nsICookieManager.idl
@@ -61,13 +61,14 @@ interface nsICookieManager : nsISupports
*/
readonly attribute nsISimpleEnumerator enumerator;
/**
* Called to remove an individual cookie from the cookie list
*
* @param aDomain The host or domain for which the cookie was set
* @param aName The name specified in the cookie
+ * @param aPath The path for which the cookie was set
* @param aBlocked Indicates if cookies from this host should be permanently blocked
*
*/
void remove(in AUTF8String aDomain, in ACString aName, in AUTF8String aPath, in boolean aBlocked);
};
--- a/netwerk/cookie/src/nsCookieService.cpp
+++ b/netwerk/cookie/src/nsCookieService.cpp
@@ -52,16 +52,17 @@
#include "nsIChannel.h"
#include "nsIFile.h"
#include "nsIObserverService.h"
#include "nsILineInputStream.h"
#include "nsIEffectiveTLDService.h"
#include "nsCOMArray.h"
#include "nsArrayEnumerator.h"
+#include "nsEnumeratorUtils.h"
#include "nsAutoPtr.h"
#include "nsReadableUtils.h"
#include "nsCRT.h"
#include "prtime.h"
#include "prprf.h"
#include "nsNetUtil.h"
#include "nsNetCID.h"
#include "nsAppDirectoryServiceDefs.h"
@@ -915,16 +916,19 @@ nsCookieService::Add(const nsACString &a
const nsACString &aPath,
const nsACString &aName,
const nsACString &aValue,
PRBool aIsSecure,
PRBool aIsHttpOnly,
PRBool aIsSession,
PRInt64 aExpiry)
{
+ NS_ENSURE_TRUE(!aDomain.IsEmpty() && !aDomain.EqualsLiteral("."),
+ NS_ERROR_INVALID_ARG);
+
PRInt64 currentTimeInUsec = PR_Now();
nsRefPtr<nsCookie> cookie =
nsCookie::Create(aName, aValue, aDomain, aPath,
aExpiry,
currentTimeInUsec,
currentTimeInUsec,
aIsSession,
@@ -939,33 +943,36 @@ nsCookieService::Add(const nsACString &a
}
NS_IMETHODIMP
nsCookieService::Remove(const nsACString &aHost,
const nsACString &aName,
const nsACString &aPath,
PRBool aBlocked)
{
+ NS_ENSURE_TRUE(!aHost.IsEmpty() && !aHost.EqualsLiteral("."),
+ NS_ERROR_INVALID_ARG);
+
nsListIter matchIter;
if (FindCookie(PromiseFlatCString(aHost),
PromiseFlatCString(aName),
PromiseFlatCString(aPath),
matchIter,
PR_Now() / PR_USEC_PER_SEC)) {
nsRefPtr<nsCookie> cookie = matchIter.current;
RemoveCookieFromList(matchIter);
NotifyChanged(cookie, NS_LITERAL_STRING("deleted").get());
}
// check if we need to add the host to the permissions blacklist.
if (aBlocked && mPermissionService) {
nsCAutoString host(NS_LITERAL_CSTRING("http://"));
// strip off the domain dot, if necessary
- if (!aHost.IsEmpty() && aHost.First() == '.')
+ if (aHost.First() == '.')
host.Append(Substring(aHost, 1, aHost.Length() - 1));
else
host.Append(aHost);
nsCOMPtr<nsIURI> uri;
NS_NewURI(getter_AddRefs(uri), host);
if (uri)
@@ -1242,16 +1249,20 @@ nsCookieService::GetCookieInternal(nsIUR
nsCAutoString hostFromURI, pathFromURI;
if (NS_FAILED(aHostURI->GetAsciiHost(hostFromURI)) ||
NS_FAILED(aHostURI->GetPath(pathFromURI))) {
COOKIE_LOGFAILURE(GET_COOKIE, aHostURI, nsnull, "couldn't get host/path from URI");
return;
}
// trim trailing dots
hostFromURI.Trim(".");
+ if (hostFromURI.IsEmpty()) {
+ COOKIE_LOGFAILURE(GET_COOKIE, aHostURI, nsnull, "empty host");
+ return;
+ }
// insert a leading dot, so we begin the hash lookup with the
// equivalent domain cookie host
hostFromURI.Insert(NS_LITERAL_CSTRING("."), 0);
// check if aHostURI is using an https secure protocol.
// if it isn't, then we can't send a secure cookie over the connection.
// if SchemeIs fails, assume an insecure connection, to be on the safe side
PRBool isSecure;
@@ -1830,16 +1841,18 @@ nsCookieService::IsForeign(nsIURI *aHost
if (NS_FAILED(aHostURI->GetAsciiHost(currentHost)) ||
NS_FAILED(aFirstURI->GetAsciiHost(firstHost))) {
// assume foreign
return PR_TRUE;
}
// trim trailing dots
currentHost.Trim(".");
firstHost.Trim(".");
+ if (currentHost.IsEmpty() || firstHost.IsEmpty())
+ return PR_TRUE;
// fast path: check if the two hosts are identical.
// 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.
@@ -1936,16 +1949,18 @@ nsCookieService::CheckDomain(nsCookieAtt
// get host from aHostURI
nsCAutoString hostFromURI;
if (NS_FAILED(aHostURI->GetAsciiHost(hostFromURI))) {
return PR_FALSE;
}
// trim trailing dots
hostFromURI.Trim(".");
+ if (hostFromURI.IsEmpty())
+ return PR_FALSE;
// if a domain is given, check the host has permission
if (!aCookieAttributes.host.IsEmpty()) {
aCookieAttributes.host.Trim(".");
// switch to lowercase now, to avoid case-insensitive compares everywhere
ToLowerCase(aCookieAttributes.host);
// get the base domain for the host URI.
@@ -2164,18 +2179,19 @@ nsCookieService::CookieExists(nsICookie2
}
// count the number of cookies from a given host, and simultaneously find the
// oldest cookie from the host.
PRUint32
nsCookieService::CountCookiesFromHostInternal(const nsACString &aHost,
nsEnumerationData &aData)
{
+ NS_ASSERTION(!aHost.IsEmpty() && !aHost.EqualsLiteral("."), "empty host");
+
PRUint32 countFromHost = 0;
-
nsCAutoString hostWithDot(NS_LITERAL_CSTRING(".") + aHost);
const char *currentDot = hostWithDot.get();
const char *nextDot = currentDot + 1;
do {
nsCookieEntry *entry = mDBState->hostTable.GetEntry(currentDot);
for (nsListIter iter(entry); iter.current; ++iter) {
// only count non-expired cookies
@@ -2200,29 +2216,36 @@ nsCookieService::CountCookiesFromHostInt
}
// count the number of cookies stored by a particular host. this is provided by the
// nsICookieManager2 interface.
NS_IMETHODIMP
nsCookieService::CountCookiesFromHost(const nsACString &aHost,
PRUint32 *aCountFromHost)
{
+ if (aHost.IsEmpty() || aHost.EqualsLiteral(".")) {
+ *aCountFromHost = 0;
+ return NS_OK;
+ }
+
// we don't care about finding the oldest cookie here, so disable the search
nsEnumerationData data(PR_Now() / PR_USEC_PER_SEC, LL_MININT);
-
*aCountFromHost = CountCookiesFromHostInternal(aHost, data);
return NS_OK;
}
// get an enumerator of cookies stored by a particular host. this is provided by the
// nsICookieManager2 interface.
NS_IMETHODIMP
nsCookieService::GetCookiesFromHost(const nsACString &aHost,
nsISimpleEnumerator **aEnumerator)
{
+ if (aHost.IsEmpty() || aHost.EqualsLiteral("."))
+ return NS_NewEmptyEnumerator(aEnumerator);
+
nsCOMArray<nsICookie> cookieList(mMaxCookiesPerHost);
nsCAutoString hostWithDot(NS_LITERAL_CSTRING(".") + aHost);
PRInt64 currentTime = PR_Now() / PR_USEC_PER_SEC;
const char *currentDot = hostWithDot.get();
const char *nextDot = currentDot + 1;
do {
nsCookieEntry *entry = mDBState->hostTable.GetEntry(currentDot);
@@ -2244,16 +2267,18 @@ nsCookieService::GetCookiesFromHost(cons
// find an exact cookie specified by host, name, and path that hasn't expired.
PRBool
nsCookieService::FindCookie(const nsAFlatCString &aHost,
const nsAFlatCString &aName,
const nsAFlatCString &aPath,
nsListIter &aIter,
PRInt64 aCurrentTime)
{
+ NS_ASSERTION(!aHost.IsEmpty() && !aHost.EqualsLiteral("."), "empty host");
+
nsCookieEntry *entry = mDBState->hostTable.GetEntry(aHost.get());
for (aIter = nsListIter(entry); aIter.current; ++aIter) {
if (aIter.current->Expiry() > aCurrentTime &&
aPath.Equals(aIter.current->Path()) &&
aName.Equals(aIter.current->Name())) {
return PR_TRUE;
}
}