From b87603207c71161689d8b8bf41c993b8b6367acc Mon Sep 17 00:00:00 2001 From: "dwitte%stanford.edu" Date: Sat, 11 Oct 2003 00:06:13 +0000 Subject: [PATCH] refactor cookie prompting helper to where it belongs... this will increase perf a little, for the case where prompting is turned off. b=220624, r=mvl, sr=darin. --- extensions/cookie/nsCookiePermission.cpp | 46 ++++++++++++++-- extensions/cookie/nsICookieManager2.idl | 46 +++++++++++++--- extensions/cookie/nsICookiePermission.idl | 11 +--- netwerk/cookie/public/nsICookieManager2.idl | 46 +++++++++++++--- netwerk/cookie/public/nsICookiePermission.idl | 11 +--- netwerk/cookie/public/nsICookieService.idl | 26 +++++++++ netwerk/cookie/src/nsCookieService.cpp | 55 ++++++++----------- netwerk/cookie/src/nsCookieService.h | 1 - 8 files changed, 169 insertions(+), 73 deletions(-) diff --git a/extensions/cookie/nsCookiePermission.cpp b/extensions/cookie/nsCookiePermission.cpp index f18009f1b3d..99655c68752 100644 --- a/extensions/cookie/nsCookiePermission.cpp +++ b/extensions/cookie/nsCookiePermission.cpp @@ -38,9 +38,11 @@ #include "nsCookiePermission.h" -#include "nsICookie.h" +#include "nsICookie2.h" #include "nsIServiceManager.h" #include "nsICookiePromptService.h" +#include "nsICookieManager2.h" +#include "nsCCookieManager.h" #include "nsIURI.h" #include "nsIPrefService.h" #include "nsIPrefBranch.h" @@ -53,6 +55,8 @@ #include "nsIChannel.h" #include "nsIDOMWindow.h" #include "nsString.h" +#include "nsInt64.h" +#include "prtime.h" /**************************************************************** ************************ nsCookiePermission ******************** @@ -63,6 +67,11 @@ static const char kCookiesAskPermission[] = "network.cookie.warnAboutCookies"; static const char kCookiesDisabledForMailNews[] = "network.cookie.disableCookieForMailNews"; static const char kPermissionType[] = "cookie"; +// XXX these casts and constructs are horrible, but our nsInt64/nsTime +// classes are lacking so we need them for now. see bug 198694. +#define USEC_PER_SEC (nsInt64(1000000)) +#define NOW_IN_SECONDS (nsInt64(PR_Now()) / USEC_PER_SEC) + #ifndef MOZ_PHOENIX // returns PR_TRUE if URI appears to be the URI of a mailnews protocol static PRBool @@ -215,9 +224,7 @@ nsCookiePermission::CanAccess(nsIURI *aURI, NS_IMETHODIMP nsCookiePermission::CanSetCookie(nsIURI *aURI, nsIChannel *aChannel, - nsICookie *aCookie, - PRInt32 aNumCookiesFromHost, - PRBool aChangingCookie, + nsICookie2 *aCookie, PRBool *aResult) { NS_ASSERTION(aURI, "null uri"); @@ -270,9 +277,38 @@ nsCookiePermission::CanSetCookie(nsIURI *aURI, GetInterfaceFromChannel(aChannel, NS_GET_IID(nsIDOMWindow), getter_AddRefs(parent)); + // get some useful information to present to the user: + // whether a previous cookie already exists, and how many cookies this host + // has set + PRBool foundCookie; + PRUint32 countFromHost; + nsCOMPtr cookieManager = do_GetService(NS_COOKIEMANAGER_CONTRACTID, &rv); + if (NS_SUCCEEDED(rv)) + rv = cookieManager->FindMatchingCookie(aCookie, &countFromHost, &foundCookie); + if (NS_FAILED(rv)) return rv; + + // check if the cookie we're trying to set is already expired, and return; + // but only if there's no previous cookie, because then we need to delete the previous + // cookie. we need this check to avoid prompting the user for already-expired cookies. + if (!foundCookie) { + PRBool isSession; + aCookie->GetIsSession(&isSession); + if (!isSession) { + nsInt64 currentTime = NOW_IN_SECONDS; + PRInt64 expiry; + aCookie->GetExpiry(&expiry); + if (nsInt64(expiry) <= currentTime) { + // the cookie has already expired. accept it, and let the backend figure + // out it's expired, so that we get correct logging & notifications. + *aResult = PR_TRUE; + return rv; + } + } + } + PRBool rememberDecision = PR_FALSE; rv = cookiePromptService->CookieDialog(parent, aCookie, hostPort, - aNumCookiesFromHost, aChangingCookie, + countFromHost, foundCookie, &rememberDecision, aResult); if (NS_FAILED(rv)) return rv; diff --git a/extensions/cookie/nsICookieManager2.idl b/extensions/cookie/nsICookieManager2.idl index 47ed6fda6fb..8cade7b0151 100644 --- a/extensions/cookie/nsICookieManager2.idl +++ b/extensions/cookie/nsICookieManager2.idl @@ -37,6 +37,8 @@ #include "nsICookieManager.idl" +interface nsICookie2; + /** * Additions to the frozen nsICookieManager */ @@ -48,15 +50,41 @@ interface nsICookieManager2 : nsICookieManager * Add a cookie. nsICookieService is the normal way to do this. This * method is something of a backdoor. * - * @param aDomain the host or domain for which the cookie is set - * @param aPath path within the domain for which the cookie is valid - * @param aName cookie name - * @param aValue cookie data - * @param aSecure true if the cookie should be secure - * @param aExpires expiration date. 0 means none; a session cookie. + * @param aDomain + * the host or domain for which the cookie is set + * @param aPath + * path within the domain for which the cookie is valid + * @param aName + * cookie name + * @param aValue + * cookie data + * @param aSecure + * true if the cookie should be secure + * @param aExpires + * expiration date. 0 means none; a session cookie. */ [noscript] - void add(in AUTF8String aDomain, in AUTF8String aPath, - in ACString aName, in ACString aValue, - in boolean aSecure, in PRInt32 aExpires); + void add(in AUTF8String aDomain, + in AUTF8String aPath, + in ACString aName, + in ACString aValue, + in boolean aSecure, + in PRInt32 aExpires); + + /** + * Find whether a matching cookie already exists, and how many cookies + * a given host has already set. This is useful when e.g. prompting the + * user whether to accept a given cookie. + * + * @param aCookie + * the cookie to look for + * @param aCountFromHost + * the number of cookies found whose hosts are the same as, or + * subdomains of, the host field of aCookie + * + * @return true if a cookie was found which matches the host, path, and name + * fields of aCookie + */ + boolean findMatchingCookie(in nsICookie2 aCookie, + out unsigned long aCountFromHost); }; diff --git a/extensions/cookie/nsICookiePermission.idl b/extensions/cookie/nsICookiePermission.idl index e1a88edd733..028c74aa2c1 100644 --- a/extensions/cookie/nsICookiePermission.idl +++ b/extensions/cookie/nsICookiePermission.idl @@ -36,7 +36,7 @@ #include "nsISupports.idl" -interface nsICookie; +interface nsICookie2; interface nsIURI; interface nsIChannel; @@ -102,19 +102,12 @@ interface nsICookiePermission : nsISupports * the corresponding to aURI * @param aCookie * the cookie being added to the cookie database - * @param aNumCookiesFromHost - * the number of cookies this host already has set - * @param aChangingCookie - * PR_TRUE if the cookie is being modified; otherwise, the cookie - * is new * * @return true if the cookie can be set. */ boolean canSetCookie(in nsIURI aURI, in nsIChannel aChannel, - in nsICookie aCookie, - in long aNumCookiesFromHost, - in boolean aChangingCookie); + in nsICookie2 aCookie); }; %{ C++ diff --git a/netwerk/cookie/public/nsICookieManager2.idl b/netwerk/cookie/public/nsICookieManager2.idl index 47ed6fda6fb..8cade7b0151 100644 --- a/netwerk/cookie/public/nsICookieManager2.idl +++ b/netwerk/cookie/public/nsICookieManager2.idl @@ -37,6 +37,8 @@ #include "nsICookieManager.idl" +interface nsICookie2; + /** * Additions to the frozen nsICookieManager */ @@ -48,15 +50,41 @@ interface nsICookieManager2 : nsICookieManager * Add a cookie. nsICookieService is the normal way to do this. This * method is something of a backdoor. * - * @param aDomain the host or domain for which the cookie is set - * @param aPath path within the domain for which the cookie is valid - * @param aName cookie name - * @param aValue cookie data - * @param aSecure true if the cookie should be secure - * @param aExpires expiration date. 0 means none; a session cookie. + * @param aDomain + * the host or domain for which the cookie is set + * @param aPath + * path within the domain for which the cookie is valid + * @param aName + * cookie name + * @param aValue + * cookie data + * @param aSecure + * true if the cookie should be secure + * @param aExpires + * expiration date. 0 means none; a session cookie. */ [noscript] - void add(in AUTF8String aDomain, in AUTF8String aPath, - in ACString aName, in ACString aValue, - in boolean aSecure, in PRInt32 aExpires); + void add(in AUTF8String aDomain, + in AUTF8String aPath, + in ACString aName, + in ACString aValue, + in boolean aSecure, + in PRInt32 aExpires); + + /** + * Find whether a matching cookie already exists, and how many cookies + * a given host has already set. This is useful when e.g. prompting the + * user whether to accept a given cookie. + * + * @param aCookie + * the cookie to look for + * @param aCountFromHost + * the number of cookies found whose hosts are the same as, or + * subdomains of, the host field of aCookie + * + * @return true if a cookie was found which matches the host, path, and name + * fields of aCookie + */ + boolean findMatchingCookie(in nsICookie2 aCookie, + out unsigned long aCountFromHost); }; diff --git a/netwerk/cookie/public/nsICookiePermission.idl b/netwerk/cookie/public/nsICookiePermission.idl index e1a88edd733..028c74aa2c1 100644 --- a/netwerk/cookie/public/nsICookiePermission.idl +++ b/netwerk/cookie/public/nsICookiePermission.idl @@ -36,7 +36,7 @@ #include "nsISupports.idl" -interface nsICookie; +interface nsICookie2; interface nsIURI; interface nsIChannel; @@ -102,19 +102,12 @@ interface nsICookiePermission : nsISupports * the corresponding to aURI * @param aCookie * the cookie being added to the cookie database - * @param aNumCookiesFromHost - * the number of cookies this host already has set - * @param aChangingCookie - * PR_TRUE if the cookie is being modified; otherwise, the cookie - * is new * * @return true if the cookie can be set. */ boolean canSetCookie(in nsIURI aURI, in nsIChannel aChannel, - in nsICookie aCookie, - in long aNumCookiesFromHost, - in boolean aChangingCookie); + in nsICookie2 aCookie); }; %{ C++ diff --git a/netwerk/cookie/public/nsICookieService.idl b/netwerk/cookie/public/nsICookieService.idl index c1abe65d88b..1efb883cf01 100644 --- a/netwerk/cookie/public/nsICookieService.idl +++ b/netwerk/cookie/public/nsICookieService.idl @@ -47,6 +47,32 @@ interface nsIChannel; * Provides methods for setting and getting cookies in the context of a * page load. See nsICookieManager for methods to manipulate the cookie * database directly. This separation of interface is mainly historical. + * + * This service broadcasts the following notifications when the cookie + * list is changed, or a cookie is rejected: + * + * topic : "cookie-changed" + * broadcast whenever the cookie list changes in some way. there + * are four possible data strings for this notification; one + * notification will be broadcast for each change, and will involve + * a single cookie. + * subject: an nsICookie2 interface pointer representing the cookie object + * that changed. + * data : "deleted" + * a cookie was deleted. the subject is the deleted cookie. + * "added" + * a cookie was added. the subject is the added cookie. + * "changed" + * a cookie was changed. the subject is the new cookie. + * "cleared" + * the entire cookie list was cleared. the subject is null. + * + * topic : "cookie-rejected" + * broadcast whenever a cookie was rejected from being set as a + * result of user prefs. + * subject: an nsIURI interface pointer representing the host URI of the + * rejected cookie. + * data : none. */ [scriptable, uuid(011C3190-1434-11d6-A618-0010A401EB10)] interface nsICookieService : nsISupports diff --git a/netwerk/cookie/src/nsCookieService.cpp b/netwerk/cookie/src/nsCookieService.cpp index 44695aa8f4f..9a1555a226d 100644 --- a/netwerk/cookie/src/nsCookieService.cpp +++ b/netwerk/cookie/src/nsCookieService.cpp @@ -1416,19 +1416,6 @@ nsCookieService::SetCookieInternal(nsIURI *aHostURI, return newCookie; } - // count the number of cookies from this host, and find whether a previous cookie - // has been set, for prompting purposes - PRUint32 countFromHost; - const PRBool foundCookie = FindCookiesFromHost(cookie, countFromHost, currentTime); - - // check if the cookie we're trying to set is already expired, and return. - // but we need to check there's no previous cookie first, because servers use - // this as a trick for deleting previous cookies. - if (!foundCookie && !cookie->IsSession() && cookie->Expiry() <= currentTime) { - COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, cookieHeader, "cookie has already expired"); - return newCookie; - } - // check permissions from site permission list, or ask the user, // to determine if we can set the cookie if (mPermissionService) { @@ -1437,8 +1424,7 @@ nsCookieService::SetCookieInternal(nsIURI *aHostURI, // needs one to prompt, so right now it has to fend for itself to get one mPermissionService->CanSetCookie(aHostURI, aChannel, - NS_STATIC_CAST(nsICookie*, NS_STATIC_CAST(nsCookie*, cookie)), - countFromHost, foundCookie, + NS_STATIC_CAST(nsICookie2*, NS_STATIC_CAST(nsCookie*, cookie)), &permission); if (!permission) { COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, cookieHeader, "cookie rejected by permission manager"); @@ -2300,21 +2286,28 @@ nsCookieService::RemoveExpiredCookies(nsInt64 aCurrentTime, } } -// count the number of cookies from this host, and find whether a previous cookie -// has been set, for prompting purposes. -PRBool -nsCookieService::FindCookiesFromHost(nsCookie *aCookie, - PRUint32 &aCountFromHost, - nsInt64 aCurrentTime) +// find whether a previous cookie has been set, and count the number of cookies from +// this host, for prompting purposes. this is provided by the nsICookieManager2 +// interface. +NS_IMETHODIMP +nsCookieService::FindMatchingCookie(nsICookie2 *aCookie, + PRUint32 *aCountFromHost, + PRBool *aFoundCookie) { - aCountFromHost = 0; - PRBool foundCookie = PR_FALSE; + NS_ENSURE_ARG_POINTER(aCookie); + + *aFoundCookie = PR_FALSE; + *aCountFromHost = 0; + nsInt64 currentTime = NOW_IN_SECONDS; + + // cast aCookie to an nsCookie*, for faster access to its members + nsCookie *cookie = NS_STATIC_CAST(nsCookie*, aCookie); + + const nsAFlatCString &host = cookie->Host(); + const nsAFlatCString &path = cookie->Path(); + const nsAFlatCString &name = cookie->Name(); nsCookie *cookieInList; - const nsAFlatCString &host = aCookie->Host(); - const nsAFlatCString &path = aCookie->Path(); - const nsAFlatCString &name = aCookie->Name(); - PRInt32 count = mCookieList.Count(); for (PRInt32 i = 0; i < count; ++i) { cookieInList = NS_STATIC_CAST(nsCookie*, mCookieList.ElementAt(i)); @@ -2322,19 +2315,19 @@ nsCookieService::FindCookiesFromHost(nsCookie *aCookie, // only count session or non-expired cookies if (IsInDomain(cookieInList->Host(), host, cookieInList->IsDomain()) && - (cookieInList->IsSession() || cookieInList->Expiry() > aCurrentTime)) { - ++aCountFromHost; + (cookieInList->IsSession() || cookieInList->Expiry() > currentTime)) { + ++*aCountFromHost; // check if we've found the previous cookie if (path.Equals(cookieInList->Path()) && host.Equals(cookieInList->Host()) && name.Equals(cookieInList->Name())) { - foundCookie = PR_TRUE; + *aFoundCookie = PR_TRUE; } } } - return foundCookie; + return NS_OK; } // find a position to store a cookie (either replacing an existing cookie, or adding diff --git a/netwerk/cookie/src/nsCookieService.h b/netwerk/cookie/src/nsCookieService.h index cfbb43e7af8..912dc3f639a 100644 --- a/netwerk/cookie/src/nsCookieService.h +++ b/netwerk/cookie/src/nsCookieService.h @@ -106,7 +106,6 @@ class nsCookieService : public nsICookieService PRBool GetExpiry(nsCookieAttributes &aCookie, nsInt64 aServerTime, nsInt64 aCurrentTime, nsCookieStatus aStatus); void RemoveAllFromMemory(); void RemoveExpiredCookies(nsInt64 aCurrentTime, PRInt32 &aOldestPosition); - PRBool FindCookiesFromHost(nsCookie *aCookie, PRUint32 &aCountFromHost, nsInt64 aCurrentTime); PRBool FindPosition(nsCookie *aCookie, PRInt32 &aInsertPosition, PRInt32 &aDeletePosition, nsInt64 aCurrentTime); void UpdateCookieIcon();