From 00d881f2c7eff68782886c6fcf54f24c857c3e89 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 1: fix mDefaultDBState. r=sdwilsh, a=betaN+ --- netwerk/cookie/nsCookieService.cpp | 64 ++++++++++++++++++------------ netwerk/cookie/nsCookieService.h | 2 +- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp index 38dd91d62a83..f2f3cc1dcb38 100644 --- a/netwerk/cookie/nsCookieService.cpp +++ b/netwerk/cookie/nsCookieService.cpp @@ -1490,9 +1490,11 @@ nsCookieService::GetCookieFromRow(T &aRow) void nsCookieService::AsyncReadComplete() { - NS_ASSERTION(mDBState == &mDefaultDBState, "not in default db state"); - NS_ASSERTION(mDBState->pendingRead, "no pending read"); - NS_ASSERTION(mDBState->readListener, "no read listener"); + // We may be in the private browsing DB state, with a pending read on the + // default DB state. (This would occur if we started up in private browsing + // mode.) As long as we do all our operations on the default state, we're OK. + NS_ASSERTION(mDefaultDBState.pendingRead, "no pending read"); + NS_ASSERTION(mDefaultDBState.readListener, "no read listener"); // Merge the data read on the background thread with the data synchronously // read on the main thread. Note that transactions on the cookie table may @@ -1506,7 +1508,8 @@ nsCookieService::AsyncReadComplete() if (mDefaultDBState.readSet.GetEntry(tuple.baseDomain)) continue; - AddCookieToList(tuple.baseDomain, tuple.cookie, NULL, PR_FALSE); + AddCookieToList(tuple.baseDomain, tuple.cookie, &mDefaultDBState, NULL, + PR_FALSE); } mDefaultDBState.stmtReadDomain = nsnull; @@ -1525,9 +1528,11 @@ nsCookieService::AsyncReadComplete() void nsCookieService::CancelAsyncRead(PRBool aPurgeReadSet) { - NS_ASSERTION(mDBState == &mDefaultDBState, "not in default db state"); - NS_ASSERTION(mDBState->pendingRead, "no pending read"); - NS_ASSERTION(mDBState->readListener, "no read listener"); + // We may be in the private browsing DB state, with a pending read on the + // default DB state. (This would occur if we started up in private browsing + // mode.) As long as we do all our operations on the default state, we're OK. + NS_ASSERTION(mDefaultDBState.pendingRead, "no pending read"); + NS_ASSERTION(mDefaultDBState.readListener, "no read listener"); // Cancel the pending read, kill the read listener, and empty the array // of data already read in on the background thread. @@ -1622,7 +1627,7 @@ nsCookieService::EnsureReadDomain(const nsCString &aBaseDomain) break; nsCookie* newCookie = GetCookieFromRow(mDefaultDBState.stmtReadDomain); - AddCookieToList(aBaseDomain, newCookie, NULL, PR_FALSE); + AddCookieToList(aBaseDomain, newCookie, &mDefaultDBState, NULL, PR_FALSE); ++readCount; } @@ -1686,7 +1691,7 @@ nsCookieService::EnsureReadComplete() continue; nsCookie* newCookie = GetCookieFromRow(stmt); - AddCookieToList(baseDomain, newCookie, NULL, PR_FALSE); + AddCookieToList(baseDomain, newCookie, &mDefaultDBState, NULL, PR_FALSE); ++readCount; } @@ -1702,6 +1707,13 @@ nsCookieService::EnsureReadComplete() NS_IMETHODIMP nsCookieService::ImportCookies(nsIFile *aCookieFile) { + // Make sure we're in the default DB state. We don't want people importing + // cookies into a private browsing session! + if (mDBState != &mDefaultDBState) { + NS_WARNING("Trying to import cookies in a private browsing session!"); + return NS_ERROR_NOT_AVAILABLE; + } + nsresult rv; nsCOMPtr fileInputStream; rv = NS_NewLocalFileInputStream(getter_AddRefs(fileInputStream), aCookieFile); @@ -1719,7 +1731,7 @@ nsCookieService::ImportCookies(nsIFile *aCookieFile) PRInt32 numInts; PRInt64 expires; PRBool isDomain, isHttpOnly = PR_FALSE; - PRUint32 originalCookieCount = mDBState->cookieCount; + PRUint32 originalCookieCount = mDefaultDBState.cookieCount; PRInt64 currentTimeInUsec = PR_Now(); PRInt64 currentTime = currentTimeInUsec / PR_USEC_PER_SEC; @@ -1756,8 +1768,8 @@ nsCookieService::ImportCookies(nsIFile *aCookieFile) // We will likely be adding a bunch of cookies to the DB, so we use async // batching with storage to make this super fast. nsCOMPtr paramsArray; - if (originalCookieCount == 0 && mDBState->dbConn) { - mDBState->stmtInsert->NewBindingParamsArray(getter_AddRefs(paramsArray)); + if (originalCookieCount == 0 && mDefaultDBState.dbConn) { + mDefaultDBState.stmtInsert->NewBindingParamsArray(getter_AddRefs(paramsArray)); } while (isMore && NS_SUCCEEDED(lineInputStream->ReadLine(buffer, &isMore))) { @@ -1830,7 +1842,7 @@ nsCookieService::ImportCookies(nsIFile *aCookieFile) lastAccessedCounter--; if (originalCookieCount == 0) { - AddCookieToList(baseDomain, newCookie, paramsArray); + AddCookieToList(baseDomain, newCookie, &mDefaultDBState, paramsArray); } else { AddInternal(baseDomain, newCookie, currentTimeInUsec, NULL, NULL, PR_TRUE); @@ -1842,17 +1854,18 @@ nsCookieService::ImportCookies(nsIFile *aCookieFile) PRUint32 length; paramsArray->GetLength(&length); if (length) { - rv = mDBState->stmtInsert->BindParameters(paramsArray); + rv = mDefaultDBState.stmtInsert->BindParameters(paramsArray); NS_ASSERT_SUCCESS(rv); nsCOMPtr handle; - rv = mDBState->stmtInsert->ExecuteAsync(mInsertListener, + rv = mDefaultDBState.stmtInsert->ExecuteAsync(mInsertListener, getter_AddRefs(handle)); NS_ASSERT_SUCCESS(rv); } } - COOKIE_LOGSTRING(PR_LOG_DEBUG, ("ImportCookies(): %ld cookies imported", mDBState->cookieCount)); + COOKIE_LOGSTRING(PR_LOG_DEBUG, ("ImportCookies(): %ld cookies imported", + mDefaultDBState.cookieCount)); return NS_OK; } @@ -2260,7 +2273,7 @@ nsCookieService::AddInternal(const nsCString &aBaseDomain, // Add the cookie to the db. We do not supply a params array for batching // because this might result in removals and additions being out of order. - AddCookieToList(aBaseDomain, aCookie, NULL); + AddCookieToList(aBaseDomain, aCookie, mDBState, NULL); NotifyChanged(aCookie, foundCookie ? NS_LITERAL_STRING("changed").get() : NS_LITERAL_STRING("added").get()); @@ -3280,27 +3293,28 @@ bindCookieParameters(mozIStorageBindingParamsArray *aParamsArray, void nsCookieService::AddCookieToList(const nsCString &aBaseDomain, nsCookie *aCookie, + DBState *aDBState, mozIStorageBindingParamsArray *aParamsArray, PRBool aWriteToDB) { - NS_ASSERTION(!(mDBState->dbConn && !aWriteToDB && aParamsArray), + NS_ASSERTION(!(aDBState->dbConn && !aWriteToDB && aParamsArray), "Not writing to the DB but have a params array?"); - NS_ASSERTION(!(!mDBState->dbConn && aParamsArray), + NS_ASSERTION(!(!aDBState->dbConn && aParamsArray), "Do not have a DB connection but have a params array?"); - nsCookieEntry *entry = mDBState->hostTable.PutEntry(aBaseDomain); + nsCookieEntry *entry = aDBState->hostTable.PutEntry(aBaseDomain); NS_ASSERTION(entry, "can't insert element into a null entry!"); entry->GetCookies().AppendElement(aCookie); - ++mDBState->cookieCount; + ++aDBState->cookieCount; // keep track of the oldest cookie, for when it comes time to purge - if (aCookie->LastAccessed() < mDBState->cookieOldestTime) - mDBState->cookieOldestTime = aCookie->LastAccessed(); + if (aCookie->LastAccessed() < aDBState->cookieOldestTime) + aDBState->cookieOldestTime = aCookie->LastAccessed(); // if it's a non-session cookie and hasn't just been read from the db, write it out. - if (aWriteToDB && !aCookie->IsSession() && mDBState->dbConn) { - mozIStorageStatement *stmt = mDBState->stmtInsert; + if (aWriteToDB && !aCookie->IsSession() && aDBState->dbConn) { + mozIStorageStatement *stmt = aDBState->stmtInsert; nsCOMPtr paramsArray(aParamsArray); if (!paramsArray) { stmt->NewBindingParamsArray(getter_AddRefs(paramsArray)); diff --git a/netwerk/cookie/nsCookieService.h b/netwerk/cookie/nsCookieService.h index e0d971125b39..f1f353abdc84 100644 --- a/netwerk/cookie/nsCookieService.h +++ b/netwerk/cookie/nsCookieService.h @@ -236,7 +236,7 @@ class nsCookieService : public nsICookieService PRBool SetCookieInternal(nsIURI *aHostURI, const nsCString& aBaseDomain, PRBool aRequireHostMatch, CookieStatus aStatus, nsDependentCString &aCookieHeader, PRInt64 aServerTime, PRBool aFromHttp); void AddInternal(const nsCString& aBaseDomain, nsCookie *aCookie, PRInt64 aCurrentTimeInUsec, nsIURI *aHostURI, const char *aCookieHeader, PRBool aFromHttp); void RemoveCookieFromList(const nsListIter &aIter, mozIStorageBindingParamsArray *aParamsArray = NULL); - void AddCookieToList(const nsCString& aBaseDomain, nsCookie *aCookie, mozIStorageBindingParamsArray *aParamsArray, PRBool aWriteToDB = PR_TRUE); + void AddCookieToList(const nsCString& aBaseDomain, nsCookie *aCookie, DBState *aDBState, mozIStorageBindingParamsArray *aParamsArray, PRBool aWriteToDB = PR_TRUE); void UpdateCookieInList(nsCookie *aCookie, PRInt64 aLastAccessed, mozIStorageBindingParamsArray *aParamsArray); 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);