From 6afb301f05800a41e6c7c772e0640fbc54f481fb Mon Sep 17 00:00:00 2001 From: Dan Witte Date: Tue, 19 Oct 2010 17:24:52 -0700 Subject: [PATCH] Bug 591447 - Cookie rowids may collide if PR_Now() winds backward. Part 3: test improvements. r=sdwilsh, a=betaN+ --- extensions/cookie/test/unit/test_eviction.js | 96 ++++++++++++++------ netwerk/cookie/nsCookieService.cpp | 6 +- 2 files changed, 71 insertions(+), 31 deletions(-) diff --git a/extensions/cookie/test/unit/test_eviction.js b/extensions/cookie/test/unit/test_eviction.js index 9be2a419558a..3787cafbbef8 100644 --- a/extensions/cookie/test/unit/test_eviction.js +++ b/extensions/cookie/test/unit/test_eviction.js @@ -2,16 +2,28 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ -function run_test() +let test_generator = do_run_test(); + +function run_test() { + do_test_pending(); + test_generator.next(); +} + +function finish_test() { + do_execute_soon(function() { + test_generator.close(); + do_test_finished(); + }); +} + +function do_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); + // Set up a profile. + let profile = do_get_profile(); // twiddle prefs to convenient values for this test - prefs.setIntPref("network.cookie.purgeAge", 1); - prefs.setIntPref("network.cookie.maxNumber", 1000); + Services.prefs.setIntPref("network.cookie.purgeAge", 1); + Services.prefs.setIntPref("network.cookie.maxNumber", 1000); // eviction is performed based on two limits: when the total number of cookies // exceeds maxNumber + 10% (1100), and when cookies are older than purgeAge @@ -21,50 +33,78 @@ function run_test() // we test the following cases of eviction: // 1) excess and age are satisfied, but only some of the excess are old enough // to be purged. - do_check_eq(testEviction(cm, 1101, 2, 50, 1051), 1051); + force_eviction(1101, 50); + + // Fake a profile change, to ensure eviction affects the database correctly. + do_close_profile(test_generator); + yield; + do_load_profile(); + + do_check_true(check_remaining_cookies(1101, 50, 1051)); // 2) excess and age are satisfied, and all of the excess are old enough // to be purged. - do_check_eq(testEviction(cm, 1101, 2, 100, 1001), 1001); + force_eviction(1101, 100); + do_close_profile(test_generator); + yield; + do_load_profile(); + do_check_true(check_remaining_cookies(1101, 100, 1001)); // 3) excess and age are satisfied, and more than the excess are old enough // to be purged. - do_check_eq(testEviction(cm, 1101, 2, 500, 1001), 1001); + force_eviction(1101, 500); + do_close_profile(test_generator); + yield; + do_load_profile(); + do_check_true(check_remaining_cookies(1101, 500, 1001)); // 4) excess but not age are satisfied. - do_check_eq(testEviction(cm, 2000, 0, 0, 2000), 2000); + force_eviction(2000, 0); + do_close_profile(test_generator); + yield; + do_load_profile(); + do_check_true(check_remaining_cookies(2000, 0, 2000)); // 5) age but not excess are satisfied. - do_check_eq(testEviction(cm, 1100, 2, 200, 1100), 1100); + force_eviction(1100, 200); + do_close_profile(test_generator); + yield; + do_load_profile(); + do_check_true(check_remaining_cookies(1100, 200, 1100)); - cm.removeAll(); - - // reset prefs to defaults - prefs.setIntPref("network.cookie.purgeAge", 30 * 24 * 60 * 60); - prefs.setIntPref("network.cookie.maxNumber", 2000); + finish_test(); } -// test that cookies are evicted by order of lastAccessed time, if both the limit -// on total cookies (maxNumber + 10%) and the purge age are exceeded +// Set 'aNumberTotal' cookies, ensuring that the first 'aNumberOld' cookies +// will be measurably older than the rest. function -testEviction(aCM, aNumberTotal, aSleepDuration, aNumberOld, aNumberToExpect) +force_eviction(aNumberTotal, aNumberOld) { - aCM.removeAll(); + Services.cookiemgr.removeAll(); var expiry = (Date.now() + 1e6) * 1000; var i; for (i = 0; i < aNumberTotal; ++i) { var host = "eviction." + i + ".tests"; - aCM.add(host, "", "test", "eviction", false, false, false, expiry); + Services.cookiemgr.add(host, "", "test", "eviction", false, false, false, + expiry); - if ((i == aNumberOld - 1) && aSleepDuration) { - // sleep a while, to make sure the first batch of cookies is older than - // the second (timer resolution varies on different platforms). - sleep(aSleepDuration * 1000); + if (i == aNumberOld - 1) { + // Sleep a while, to make sure the first batch of cookies is older than + // the second (timer resolution varies on different platforms). This + // delay must be measurably greater than the eviction age threshold. + sleep(1100); } } +} - var enumerator = aCM.enumerator; +// Test that 'aNumberToExpect' cookies remain after purging is complete, and +// that the cookies that remain consist of the set expected given the number of +// of older and newer cookies -- eviction should occur by order of lastAccessed +// time, if both the limit on total cookies (maxNumber + 10%) and the purge age +// are exceeded. +function check_remaining_cookies(aNumberTotal, aNumberOld, aNumberToExpect) { + var enumerator = Services.cookiemgr.enumerator; i = 0; while (enumerator.hasMoreElements()) { @@ -78,7 +118,7 @@ testEviction(aCM, aNumberTotal, aSleepDuration, aNumberOld, aNumberToExpect) } } - return i; + return i == aNumberToExpect; } // delay for a number of milliseconds diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp index 20c0f8dfc417..2f9acd4a1712 100644 --- a/netwerk/cookie/nsCookieService.cpp +++ b/netwerk/cookie/nsCookieService.cpp @@ -3029,7 +3029,7 @@ nsCookieService::PurgeCookies(PRInt64 aCurrentTimeInUsec) #ifdef PR_LOGGING PRUint32 initialCookieCount = mDBState->cookieCount; COOKIE_LOGSTRING(PR_LOG_DEBUG, - ("PurgeCookies(): beginning purge with %ld cookies and %lld age", + ("PurgeCookies(): beginning purge with %ld cookies and %lld oldest age", mDBState->cookieCount, aCurrentTimeInUsec - mDBState->cookieOldestTime)); #endif @@ -3077,7 +3077,7 @@ nsCookieService::PurgeCookies(PRInt64 aCurrentTimeInUsec) for (nsPurgeData::ArrayType::index_type i = purgeList.Length(); i--; ) { nsCookie *cookie = purgeList[i].Cookie(); removedList->AppendElement(cookie, PR_FALSE); - COOKIE_LOGEVICTED(cookie, "Cookie expired or too old"); + COOKIE_LOGEVICTED(cookie, "Cookie too old"); RemoveCookieFromList(purgeList[i], paramsArray); } @@ -3104,7 +3104,7 @@ nsCookieService::PurgeCookies(PRInt64 aCurrentTimeInUsec) COOKIE_LOGSTRING(PR_LOG_DEBUG, ("PurgeCookies(): %ld expired; %ld purged; %ld remain; %lld oldest age", initialCookieCount - postExpiryCookieCount, - mDBState->cookieCount - postExpiryCookieCount, + postExpiryCookieCount - mDBState->cookieCount, mDBState->cookieCount, aCurrentTimeInUsec - mDBState->cookieOldestTime)); }