- tor-commits - lists.torproject.org

[torbrowser/master] Use the predefined COMPRESSED_NAME
by erinn＠torproject.org 04 Aug '12

04 Aug '12

commit ba5724c720ccbf06b5e3db19018dbc91ecb28e45 Author: Shondoit Walker <shondoit(a)gmail.com> Date: Thu Aug 2 09:25:07 2012 +0200 Use the predefined COMPRESSED_NAME COMPRESSED_NAME is already defined according to the USE_OBFSPROXY flag as introduced in commit a08b546605d13d82e6f664bf8e2d8b3caf61327c. --- build-scripts/linux-alpha.mk | 5 +---- build-scripts/osx-alpha.mk | 7 ++----- build-scripts/windows-alpha.mk | 3 --- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/build-scripts/linux-alpha.mk b/build-scripts/linux-alpha.mk index ceb2014..8427a10 100644 --- a/build-scripts/linux-alpha.mk +++ b/build-scripts/linux-alpha.mk @@ -377,10 +377,7 @@ bundle-localized: bundle-localized_$(LANGCODE).stamp compressed-bundle-localized: bundle-localized_$(LANGCODE).stamp -rm -f $(DISTDIR)/$(COMPRESSED_NAME)_$(LANGCODE).tar.gz -mkdir $(DISTDIR) -ifeq ($(USE_OBFSPROXY),1) - tar -cvf - $(NAME)_$(LANGCODE) |tardy -unu 0 -una root -gnu 0 -gna wheel |gzip -c9 >$(DISTDIR)/$(DEFAULT_COMPRESSED_BASENAME)obfsproxy-$(LANGCODE).tar.gz -endif - tar -cvf - $(NAME)_$(LANGCODE) |tardy -unu 0 -una root -gnu 0 -gna wheel |gzip -c9 >$(DISTDIR)/$(DEFAULT_COMPRESSED_BASENAME)$(LANGCODE).tar.gz + tar -cvf - $(NAME)_$(LANGCODE) |tardy -unu 0 -una root -gnu 0 -gna wheel |gzip -c9 >$(DISTDIR)/$(COMPRESSED_NAME)$(LANGCODE).tar.gz rm *.zip *.xpi copy-files_%: generic-bundle.stamp diff --git a/build-scripts/osx-alpha.mk b/build-scripts/osx-alpha.mk index 377fba9..ad75326 100644 --- a/build-scripts/osx-alpha.mk +++ b/build-scripts/osx-alpha.mk @@ -374,13 +374,10 @@ bundle-localized_%.stamp: bundle-localized: bundle-localized_$(LANGCODE).stamp compressed-bundle-localized: bundle-localized_$(LANGCODE).stamp - -rm -f $(DISTDIR)/$(COMPRESSED_NAME)_$(LANGCODE).zip + -rm -f $(DISTDIR)/$(COMPRESSED_NAME)$(LANGCODE).zip -mkdir $(DISTDIR) #hdiutil create -volname "Tor Browser Bundle for OS X" -format UDBZ -imagekey zlib-level=9 -srcfolder $(DISTDIR)/tmp/ $(DISTDIR)/$(DEFAULT_COMPRESSED_BASENAME)$(LANGCODE).dmg - zip -r $(DISTDIR)/$(DEFAULT_COMPRESSED_BASENAME)$(LANGCODE).zip $(NAME)_$(LANGCODE).app -ifeq ($(USE_OBFSPROXY),1) - zip -r $(DISTDIR)/$(DEFAULT_COMPRESSED_BASENAME)obfsproxy-$(LANGCODE).zip $(NAME)_$(LANGCODE).app -endif + zip -r $(DISTDIR)/$(COMPRESSED_NAME)$(LANGCODE).zip $(NAME)_$(LANGCODE).app -rm -f *.zip *.xpi copy-files_%: generic-bundle.stamp diff --git a/build-scripts/windows-alpha.mk b/build-scripts/windows-alpha.mk index cb9e1ae..5847a34 100644 --- a/build-scripts/windows-alpha.mk +++ b/build-scripts/windows-alpha.mk @@ -373,9 +373,6 @@ bundle-localized: bundle-localized_$(LANGCODE).stamp compressed-bundle-localized: $(SEVENZIP) bundle-localized_$(LANGCODE).stamp rm -f $(COMPRESSED_NAME)_$(LANGCODE).exe -ifeq ($(USE_OBFSPROXY),1) - cd $(NAME)_$(LANGCODE); $(SEVENZIP) a -mx9 -sfx7z.sfx ../$(COMPRESSED_NAME)-obfsproxy_$(LANGCODE).exe $(NAME) -endif cd $(NAME)_$(LANGCODE); $(SEVENZIP) a -mx9 -sfx7z.sfx ../$(COMPRESSED_NAME)_$(LANGCODE).exe $(NAME) copy-files_%: generic-bundle.stamp

1 0

[torbrowser/master] Merge branch 'maint-2.3'
by erinn＠torproject.org 04 Aug '12

04 Aug '12

commit a82e8fb15132a8e71b3f21a209999b218f989e4e Merge: 71dd3ae ba5724c Author: Erinn Clark <erinn(a)torproject.org> Date: Sat Aug 4 21:05:40 2012 +0100 Merge branch 'maint-2.3' build-scripts/linux-alpha.mk | 5 +---- build-scripts/osx-alpha.mk | 7 ++----- build-scripts/versions-alpha.mk | 2 +- build-scripts/windows-alpha.mk | 5 +---- 4 files changed, 5 insertions(+), 14 deletions(-)

1 0

[torbrowser/master] Merge branch 'maint-2.3'
by mikeperry＠torproject.org 04 Aug '12

04 Aug '12

commit 71dd3ae3159edb99ae3e50dac865389e558be2ff Merge: fdae985 f2be42f Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sat Aug 4 12:46:41 2012 -0700 Merge branch 'maint-2.3' ...solate-the-Image-Cache-per-url-bar-domain.patch | 910 -------------------- 1 files changed, 0 insertions(+), 910 deletions(-)

1 0

[torbrowser/master] Remove crashy cache isolation patch for now :/.
by mikeperry＠torproject.org 04 Aug '12

04 Aug '12

commit f2be42f3984acb20c58c05cf5534396fef635161 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Fri Aug 3 22:14:23 2012 -0700 Remove crashy cache isolation patch for now :/. It asserts during debug builds, and also may have issues with XUL dialogs? --- ...solate-the-Image-Cache-per-url-bar-domain.patch | 910 -------------------- 1 files changed, 0 insertions(+), 910 deletions(-) diff --git a/src/current-patches/firefox/alpha/0021-Isolate-the-Image-Cache-per-url-bar-domain.patch b/src/current-patches/firefox/alpha/0021-Isolate-the-Image-Cache-per-url-bar-domain.patch deleted file mode 100644 index b21536c..0000000 --- a/src/current-patches/firefox/alpha/0021-Isolate-the-Image-Cache-per-url-bar-domain.patch +++ /dev/null @@ -1,910 +0,0 @@ -From 1b18f231fce26c0b6d3c2bb2c6e30e30c8e05a1e Mon Sep 17 00:00:00 2001 -From: Mike Perry <mikeperry-git(a)torproject.org> -Date: Mon, 30 Jul 2012 18:37:36 -0700 -Subject: [PATCH 21/21] Isolate the Image Cache per url bar domain. - -Also adds a new API to mozIThirdPartyUtil to allow you to get the url bar URI -for a channel or nsIDocument. ---- - content/base/src/ThirdPartyUtil.cpp | 52 +++++ - content/base/src/ThirdPartyUtil.h | 2 + - content/base/src/nsContentUtils.cpp | 13 +- - embedding/browser/webBrowser/nsContextMenuInfo.cpp | 29 ++- - extensions/cookie/nsCookiePermission.cpp | 3 + - image/public/imgILoader.idl | 4 +- - image/src/imgLoader.cpp | 200 ++++++++++++-------- - image/src/imgLoader.h | 13 +- - image/src/imgRequest.cpp | 9 +- - image/src/imgRequest.h | 3 + - layout/generic/nsImageFrame.cpp | 11 +- - netwerk/base/public/mozIThirdPartyUtil.idl | 21 ++ - netwerk/cookie/nsICookiePermission.idl | 1 + - toolkit/system/gnome/nsAlertsIconListener.cpp | 3 +- - widget/cocoa/nsMenuItemIconX.mm | 9 +- - 15 files changed, 272 insertions(+), 101 deletions(-) - -diff --git a/content/base/src/ThirdPartyUtil.cpp b/content/base/src/ThirdPartyUtil.cpp -index 6a415e9..62333f3 100644 ---- a/content/base/src/ThirdPartyUtil.cpp -+++ b/content/base/src/ThirdPartyUtil.cpp -@@ -40,6 +40,9 @@ - #include "nsIServiceManager.h" - #include "nsIHttpChannelInternal.h" - #include "nsIDOMWindow.h" -+#include "nsICookiePermission.h" -+#include "nsIDOMDocument.h" -+#include "nsIDocument.h" - #include "nsILoadContext.h" - #include "nsIPrincipal.h" - #include "nsIScriptObjectPrincipal.h" -@@ -54,6 +57,7 @@ ThirdPartyUtil::Init() - - nsresult rv; - mTLDService = do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID, &rv); -+ mCookiePermissions = do_GetService(NS_COOKIEPERMISSION_CONTRACTID); - return rv; - } - -@@ -315,3 +319,51 @@ ThirdPartyUtil::GetBaseDomain(nsIURI* aHostURI, - - return NS_OK; - } -+ -+NS_IMETHODIMP -+ThirdPartyUtil::GetFirstPartyURI(nsIChannel *aChannel, -+ nsIDocument *aDoc, -+ nsIURI **aOutput) -+{ -+ nsresult rv = NS_ERROR_NULL_POINTER; -+ -+ if (!aChannel && aDoc) { -+ aChannel = aDoc->GetChannel(); -+ } -+ -+ // If aChannel is specified or available, use the official route -+ // for sure -+ if (aChannel) { -+ rv = mCookiePermissions->GetOriginatingURI(aChannel, aOutput); -+ } -+ -+ // If the channel was missing, closed or broken, try the -+ // window hierarchy directly. -+ // -+ // This might fail to work for first-party loads themselves, but -+ // we don't need this codepath for that case. -+ if (NS_FAILED(rv) && aDoc) { -+ nsCOMPtr<nsIDOMWindow> top; -+ nsCOMPtr<nsIDOMDocument> topDDoc; -+ -+ aDoc->GetWindow()->GetTop(getter_AddRefs(top)); -+ top->GetDocument(getter_AddRefs(topDDoc)); -+ -+ nsCOMPtr<nsIDocument> topDoc(do_QueryInterface(topDDoc)); -+ *aOutput = topDoc->GetOriginalURI(); -+ -+ if (*aOutput) -+ rv = NS_OK; -+ } -+ -+ // TODO: We could provide a route through the loadgroup + notification -+ // callbacks too, but either channel or document was always available -+ // in the cases where this function was originally needed (the image cache). -+ // The notification callbacks also appear to suffers from the same limitation -+ // as the document path. See nsICookiePermissions.GetOriginatingURI() for -+ // details. -+ -+ return rv; -+} -+ -+ -diff --git a/content/base/src/ThirdPartyUtil.h b/content/base/src/ThirdPartyUtil.h -index 58ddb15..ba70162 100644 ---- a/content/base/src/ThirdPartyUtil.h -+++ b/content/base/src/ThirdPartyUtil.h -@@ -46,6 +46,7 @@ - class nsIURI; - class nsIChannel; - class nsIDOMWindow; -+class nsICookiePermission; - - class ThirdPartyUtil : public mozIThirdPartyUtil - { -@@ -61,6 +62,7 @@ private: - static already_AddRefed<nsIURI> GetURIFromWindow(nsIDOMWindow* aWin); - - nsCOMPtr<nsIEffectiveTLDService> mTLDService; -+ nsCOMPtr<nsICookiePermission> mCookiePermissions; - }; - - #endif -diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp -index 5c85697..4368609 100644 ---- a/content/base/src/nsContentUtils.cpp -+++ b/content/base/src/nsContentUtils.cpp -@@ -179,6 +179,7 @@ static NS_DEFINE_CID(kXTFServiceCID, NS_XTFSERVICE_CID); - #include "nsIDOMHTMLInputElement.h" - #include "nsParserConstants.h" - #include "nsIWebNavigation.h" -+#include "mozIThirdPartyUtil.h" - - #ifdef IBMBIDI - #include "nsIBidiKeyboard.h" -@@ -2629,8 +2630,6 @@ nsContentUtils::LoadImage(nsIURI* aURI, nsIDocument* aLoadingDocument, - nsCOMPtr<nsILoadGroup> loadGroup = aLoadingDocument->GetDocumentLoadGroup(); - NS_ASSERTION(loadGroup, "Could not get loadgroup; onload may fire too early"); - -- nsIURI *documentURI = aLoadingDocument->GetDocumentURI(); -- - // check for a Content Security Policy to pass down to the channel that - // will get created to load the image - nsCOMPtr<nsIChannelPolicy> channelPolicy; -@@ -2647,11 +2646,15 @@ nsContentUtils::LoadImage(nsIURI* aURI, nsIDocument* aLoadingDocument, - - // Make the URI immutable so people won't change it under us - NS_TryToSetImmutable(aURI); -+ -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ thirdPartySvc->GetFirstPartyURI(nsnull, aLoadingDocument, -+ getter_AddRefs(firstPartyURI)); - -- // XXXbz using "documentURI" for the initialDocumentURI is not quite -- // right, but the best we can do here... - return imgLoader->LoadImage(aURI, /* uri to load */ -- documentURI, /* initialDocumentURI */ -+ firstPartyURI, /* firstPartyURI */ - aReferrer, /* referrer */ - aLoadingPrincipal, /* loading principal */ - loadGroup, /* loadgroup */ -diff --git a/embedding/browser/webBrowser/nsContextMenuInfo.cpp b/embedding/browser/webBrowser/nsContextMenuInfo.cpp -index 045482a..78f5fc4 100644 ---- a/embedding/browser/webBrowser/nsContextMenuInfo.cpp -+++ b/embedding/browser/webBrowser/nsContextMenuInfo.cpp -@@ -62,6 +62,7 @@ - #include "nsIChannelPolicy.h" - #include "nsIContentSecurityPolicy.h" - #include "nsIContentPolicy.h" -+#include "mozIThirdPartyUtil.h" - - //***************************************************************************** - // class nsContextMenuInfo -@@ -305,15 +306,15 @@ nsContextMenuInfo::GetBackgroundImageRequestInternal(nsIDOMNode *aDOMNode, imgIR - nsCOMPtr<nsIPrincipal> principal; - nsCOMPtr<nsIChannelPolicy> channelPolicy; - nsCOMPtr<nsIContentSecurityPolicy> csp; -- if (doc) { -- principal = doc->NodePrincipal(); -- nsresult rv = principal->GetCsp(getter_AddRefs(csp)); -- NS_ENSURE_SUCCESS(rv, rv); -- if (csp) { -- channelPolicy = do_CreateInstance("@mozilla.org/nschannelpolicy;1"); -- channelPolicy->SetContentSecurityPolicy(csp); -- channelPolicy->SetLoadType(nsIContentPolicy::TYPE_IMAGE); -- } -+ NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE); -+ -+ principal = doc->NodePrincipal(); -+ nsresult rv = principal->GetCsp(getter_AddRefs(csp)); -+ NS_ENSURE_SUCCESS(rv, rv); -+ if (csp) { -+ channelPolicy = do_CreateInstance("@mozilla.org/nschannelpolicy;1"); -+ channelPolicy->SetContentSecurityPolicy(csp); -+ channelPolicy->SetLoadType(nsIContentPolicy::TYPE_IMAGE); - } - - while (true) { -@@ -340,8 +341,14 @@ nsContextMenuInfo::GetBackgroundImageRequestInternal(nsIDOMNode *aDOMNode, imgIR - nsCOMPtr<imgILoader> il(do_GetService( - "@mozilla.org/image/loader;1")); - NS_ENSURE_TRUE(il, NS_ERROR_FAILURE); -- -- return il->LoadImage(bgUri, nsnull, nsnull, principal, nsnull, -+ -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ thirdPartySvc->GetFirstPartyURI(nsnull, doc, -+ getter_AddRefs(firstPartyURI)); -+ -+ return il->LoadImage(bgUri, firstPartyURI, nsnull, principal, nsnull, - nsnull, nsnull, nsIRequest::LOAD_NORMAL, nsnull, - nsnull, channelPolicy, aRequest); - } -diff --git a/extensions/cookie/nsCookiePermission.cpp b/extensions/cookie/nsCookiePermission.cpp -index 577ee7e..c15ddc9 100644 ---- a/extensions/cookie/nsCookiePermission.cpp -+++ b/extensions/cookie/nsCookiePermission.cpp -@@ -445,6 +445,9 @@ nsCookiePermission::GetOriginatingURI(nsIChannel *aChannel, - - return NS_OK; - } -+ -+ // TODO: Why don't we just use this here: -+ // httpChannelInternal->GetDocumentURI(aURI); - } - - // find the associated window and its top window -diff --git a/image/public/imgILoader.idl b/image/public/imgILoader.idl -index 611b939..db9320b 100644 ---- a/image/public/imgILoader.idl -+++ b/image/public/imgILoader.idl -@@ -71,7 +71,7 @@ interface imgILoader : nsISupports - /** - * Start the load and decode of an image. - * @param aURI the URI to load -- * @param aInitialDocumentURI the URI that 'initiated' the load -- used for 3rd party cookie blocking -+ * @param aFirstPartyURI the urlbar URI that 'initiated' the load -- used for 3rd party blocking - * @param aReferrerURI the 'referring' URI - * @param aLoadingPrincipal the principal of the loading document - * @param aLoadGroup Loadgroup to put the image load into -@@ -90,7 +90,7 @@ interface imgILoader : nsISupports - * goes away. - */ - imgIRequest loadImage(in nsIURI aURI, -- in nsIURI aInitialDocumentURL, -+ in nsIURI aFirstPartyURI, - in nsIURI aReferrerURI, - in nsIPrincipal aLoadingPrincipal, - in nsILoadGroup aLoadGroup, -diff --git a/image/src/imgLoader.cpp b/image/src/imgLoader.cpp -index 36c8e6a..dd16771 100644 ---- a/image/src/imgLoader.cpp -+++ b/image/src/imgLoader.cpp -@@ -92,6 +92,7 @@ - #include "nsIHttpChannelInternal.h" - #include "nsIContentSecurityPolicy.h" - #include "nsIChannelPolicy.h" -+#include "mozIThirdPartyUtil.h" - - #include "nsContentUtils.h" - -@@ -504,7 +505,7 @@ static nsresult NewImageChannel(nsIChannel **aResult, - // aLoadingPrincipal and false otherwise. - bool *aForcePrincipalCheckForCacheEntry, - nsIURI *aURI, -- nsIURI *aInitialDocumentURI, -+ nsIURI *aFirstPartyURI, - nsIURI *aReferringURI, - nsILoadGroup *aLoadGroup, - const nsCString& aAcceptHeader, -@@ -556,7 +557,7 @@ static nsresult NewImageChannel(nsIChannel **aResult, - - nsCOMPtr<nsIHttpChannelInternal> httpChannelInternal = do_QueryInterface(newHttpChannel); - NS_ENSURE_TRUE(httpChannelInternal, NS_ERROR_UNEXPECTED); -- httpChannelInternal->SetDocumentURI(aInitialDocumentURI); -+ httpChannelInternal->SetDocumentURI(aFirstPartyURI); - newHttpChannel->SetReferrer(aReferringURI); - } - -@@ -999,34 +1000,61 @@ NS_IMETHODIMP imgLoader::ClearCache(bool chrome) - /* void removeEntry(in nsIURI uri); */ - NS_IMETHODIMP imgLoader::RemoveEntry(nsIURI *uri) - { -- if (RemoveFromCache(uri)) -+ if (RemoveMatchingUrlsFromCache(uri)) - return NS_OK; - - return NS_ERROR_NOT_AVAILABLE; - } - -+static PLDHashOperator EnumAllEntries(const nsACString&, -+ nsRefPtr<imgCacheEntry> &aData, -+ void *data) -+{ -+ nsTArray<nsRefPtr<imgCacheEntry> > *entries = -+ reinterpret_cast<nsTArray<nsRefPtr<imgCacheEntry> > *>(data); -+ -+ entries->AppendElement(aData); -+ -+ return PL_DHASH_NEXT; -+} -+ - /* imgIRequest findEntry(in nsIURI uri); */ - NS_IMETHODIMP imgLoader::FindEntryProperties(nsIURI *uri, nsIProperties **_retval) - { - nsRefPtr<imgCacheEntry> entry; -- nsCAutoString spec; - imgCacheTable &cache = GetCache(uri); -- -- uri->GetSpec(spec); - *_retval = nsnull; - -- if (cache.Get(spec, getter_AddRefs(entry)) && entry) { -- if (gCacheTracker && entry->HasNoProxies()) -- gCacheTracker->MarkUsed(entry); -+ // We must traverse the whole cache in O(N) looking for the first -+ // matching URI. -+ // -+ // TODO: For now, it's ok to pick at random here. The images should be -+ // identical unless there is a cache-tracking attack. And even if they -+ // are not identical due to attack, this code is only used for save -+ // dialogs at this point, so no differentiating info is leaked to -+ // content. -+ nsTArray<nsRefPtr<imgCacheEntry> > entries; -+ cache.Enumerate(EnumAllEntries, &entries); -+ -+ for (PRUint32 i = 0; i < entries.Length(); ++i) { -+ bool isEqual = false; - -- nsRefPtr<imgRequest> request = getter_AddRefs(entry->GetRequest()); -+ nsRefPtr<imgRequest> request = getter_AddRefs(entries[i]->GetRequest()); - if (request) { -- *_retval = request->Properties(); -- NS_ADDREF(*_retval); -+ request->mURI->Equals(uri, &isEqual); -+ if (isEqual) { -+ if (gCacheTracker && entries[i]->HasNoProxies()) -+ gCacheTracker->MarkUsed(entries[i]); -+ -+ *_retval = request->Properties(); -+ } - } - } - -- return NS_OK; -+ if (*_retval) -+ return NS_OK; -+ -+ return NS_ERROR_NOT_AVAILABLE; - } - - void imgLoader::Shutdown() -@@ -1054,20 +1082,18 @@ void imgLoader::MinimizeCaches() - EvictEntries(sChromeCacheQueue); - } - --bool imgLoader::PutIntoCache(nsIURI *key, imgCacheEntry *entry) -+bool imgLoader::PutIntoCache(nsCAutoString key, -+ imgCacheEntry *entry) - { -- imgCacheTable &cache = GetCache(key); -- -- nsCAutoString spec; -- key->GetSpec(spec); -- -- LOG_STATIC_FUNC_WITH_PARAM(gImgLog, "imgLoader::PutIntoCache", "uri", spec.get()); -+ LOG_STATIC_FUNC_WITH_PARAM(gImgLog, "imgLoader::PutIntoCache", "uri", key.get()); -+ imgCacheTable &cache = GetCache(entry->mRequest->mURI); -+ imgCacheQueue &queue = GetCacheQueue(entry->mRequest->mURI); - - // Check to see if this request already exists in the cache and is being - // loaded on a different thread. If so, don't allow this entry to be added to - // the cache. - nsRefPtr<imgCacheEntry> tmpCacheEntry; -- if (cache.Get(spec, getter_AddRefs(tmpCacheEntry)) && tmpCacheEntry) { -+ if (cache.Get(key, getter_AddRefs(tmpCacheEntry)) && tmpCacheEntry) { - PR_LOG(gImgLog, PR_LOG_DEBUG, - ("[this=%p] imgLoader::PutIntoCache -- Element already in the cache", nsnull)); - nsRefPtr<imgRequest> tmpRequest = getter_AddRefs(tmpCacheEntry->GetRequest()); -@@ -1077,13 +1103,13 @@ bool imgLoader::PutIntoCache(nsIURI *key, imgCacheEntry *entry) - PR_LOG(gImgLog, PR_LOG_DEBUG, - ("[this=%p] imgLoader::PutIntoCache -- Replacing cached element", nsnull)); - -- RemoveFromCache(key); -+ RemoveKeyFromCache(cache, queue, key); - } else { - PR_LOG(gImgLog, PR_LOG_DEBUG, - ("[this=%p] imgLoader::PutIntoCache -- Element NOT already in the cache", nsnull)); - } - -- if (!cache.Put(spec, entry)) -+ if (!cache.Put(key, entry)) - return false; - - // We can be called to resurrect an evicted entry. -@@ -1099,7 +1125,6 @@ bool imgLoader::PutIntoCache(nsIURI *key, imgCacheEntry *entry) - addrv = gCacheTracker->AddObject(entry); - - if (NS_SUCCEEDED(addrv)) { -- imgCacheQueue &queue = GetCacheQueue(key); - queue.Push(entry); - } - } -@@ -1205,7 +1230,7 @@ void imgLoader::CheckCacheLimits(imgCacheTable &cache, imgCacheQueue &queue) - - bool imgLoader::ValidateRequestWithNewChannel(imgRequest *request, - nsIURI *aURI, -- nsIURI *aInitialDocumentURI, -+ nsIURI *aFirstPartyURI, - nsIURI *aReferrerURI, - nsILoadGroup *aLoadGroup, - imgIDecoderObserver *aObserver, -@@ -1257,7 +1282,7 @@ bool imgLoader::ValidateRequestWithNewChannel(imgRequest *request, - rv = NewImageChannel(getter_AddRefs(newChannel), - &forcePrincipalCheck, - aURI, -- aInitialDocumentURI, -+ aFirstPartyURI, - aReferrerURI, - aLoadGroup, - mAcceptHeader, -@@ -1326,7 +1351,7 @@ bool imgLoader::ValidateRequestWithNewChannel(imgRequest *request, - - bool imgLoader::ValidateEntry(imgCacheEntry *aEntry, - nsIURI *aURI, -- nsIURI *aInitialDocumentURI, -+ nsIURI *aFirstPartyURI, - nsIURI *aReferrerURI, - nsILoadGroup *aLoadGroup, - imgIDecoderObserver *aObserver, -@@ -1432,7 +1457,7 @@ bool imgLoader::ValidateEntry(imgCacheEntry *aEntry, - if (validateRequest && aCanMakeNewChannel) { - LOG_SCOPE(gImgLog, "imgLoader::ValidateRequest |cache hit| must validate"); - -- return ValidateRequestWithNewChannel(request, aURI, aInitialDocumentURI, -+ return ValidateRequestWithNewChannel(request, aURI, aFirstPartyURI, - aReferrerURI, aLoadGroup, aObserver, - aCX, aLoadFlags, aExistingRequest, - aProxyRequest, aPolicy, -@@ -1442,16 +1467,32 @@ bool imgLoader::ValidateEntry(imgCacheEntry *aEntry, - return !validateRequest; - } - -- --bool imgLoader::RemoveFromCache(nsIURI *aKey) -+bool imgLoader::RemoveMatchingUrlsFromCache(nsIURI *aKey) - { -- if (!aKey) return false; -- -+ bool rv = true; - imgCacheTable &cache = GetCache(aKey); -- imgCacheQueue &queue = GetCacheQueue(aKey); - -- nsCAutoString spec; -- aKey->GetSpec(spec); -+ // We have to make a temporary, since RemoveFromCache removes the element -+ // from the queue, invalidating iterators. -+ nsTArray<nsRefPtr<imgCacheEntry> > entries; -+ cache.Enumerate(EnumAllEntries, &entries); -+ -+ for (PRUint32 i = 0; i < entries.Length(); ++i) { -+ bool isEqual = false; -+ -+ entries[i]->mRequest->mURI->Equals(aKey, &isEqual); -+ if (isEqual && !RemoveFromCache(entries[i])) -+ rv = false; -+ } -+ -+ return rv; -+} -+ -+bool imgLoader::RemoveKeyFromCache(imgCacheTable &cache, -+ imgCacheQueue &queue, -+ nsCAutoString spec) -+{ -+ if (spec.IsEmpty()) return false; - - LOG_STATIC_FUNC_WITH_PARAM(gImgLog, "imgLoader::RemoveFromCache", "uri", spec.get()); - -@@ -1485,12 +1526,13 @@ bool imgLoader::RemoveFromCache(imgCacheEntry *entry) - - nsRefPtr<imgRequest> request(getter_AddRefs(entry->GetRequest())); - if (request) { -- nsCOMPtr<nsIURI> key; -- if (NS_SUCCEEDED(request->GetURI(getter_AddRefs(key))) && key) { -- imgCacheTable &cache = GetCache(key); -- imgCacheQueue &queue = GetCacheQueue(key); -- nsCAutoString spec; -- key->GetSpec(spec); -+ nsCOMPtr<nsIURI> imgURI = request->mURI; -+ nsCOMPtr<nsIURI> firstPartyURI = request->mFirstPartyURI; -+ -+ if (imgURI && firstPartyURI) { -+ imgCacheTable &cache = GetCache(imgURI); -+ imgCacheQueue &queue = GetCacheQueue(imgURI); -+ nsCAutoString spec = GetCacheKey(firstPartyURI, imgURI); - - LOG_STATIC_FUNC_WITH_PARAM(gImgLog, "imgLoader::RemoveFromCache", "entry's uri", spec.get()); - -@@ -1513,18 +1555,6 @@ bool imgLoader::RemoveFromCache(imgCacheEntry *entry) - return false; - } - --static PLDHashOperator EnumEvictEntries(const nsACString&, -- nsRefPtr<imgCacheEntry> &aData, -- void *data) --{ -- nsTArray<nsRefPtr<imgCacheEntry> > *entries = -- reinterpret_cast<nsTArray<nsRefPtr<imgCacheEntry> > *>(data); -- -- entries->AppendElement(aData); -- -- return PL_DHASH_NEXT; --} -- - nsresult imgLoader::EvictEntries(imgCacheTable &aCacheToClear) - { - LOG_STATIC_FUNC(gImgLog, "imgLoader::EvictEntries table"); -@@ -1532,7 +1562,7 @@ nsresult imgLoader::EvictEntries(imgCacheTable &aCacheToClear) - // We have to make a temporary, since RemoveFromCache removes the element - // from the queue, invalidating iterators. - nsTArray<nsRefPtr<imgCacheEntry> > entries; -- aCacheToClear.Enumerate(EnumEvictEntries, &entries); -+ aCacheToClear.Enumerate(EnumAllEntries, &entries); - - for (PRUint32 i = 0; i < entries.Length(); ++i) - if (!RemoveFromCache(entries[i])) -@@ -1565,11 +1595,10 @@ nsresult imgLoader::EvictEntries(imgCacheQueue &aQueueToClear) - nsIRequest::VALIDATE_NEVER | \ - nsIRequest::VALIDATE_ONCE_PER_SESSION) - -- --/* imgIRequest loadImage (in nsIURI aURI, in nsIURI initialDocumentURI, in nsIPrincipal loadingPrincipal, in nsILoadGroup aLoadGroup, in imgIDecoderObserver aObserver, in nsISupports aCX, in nsLoadFlags aLoadFlags, in nsISupports cacheKey, in imgIRequest aRequest); */ -+/* imgIRequest loadImage (in nsIURI aURI, in nsIURI aUrlBarURI, in nsIPrincipal loadingPrincipal, in nsILoadGroup aLoadGroup, in imgIDecoderObserver aObserver, in nsISupports aCX, in nsLoadFlags aLoadFlags, in nsISupports cacheKey, in imgIRequest aRequest); */ - - NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, -- nsIURI *aInitialDocumentURI, -+ nsIURI *aFirstPartyURI, - nsIURI *aReferrerURI, - nsIPrincipal* aLoadingPrincipal, - nsILoadGroup *aLoadGroup, -@@ -1588,8 +1617,8 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - if (!aURI) - return NS_ERROR_NULL_POINTER; - -- nsCAutoString spec; -- aURI->GetSpec(spec); -+ nsCAutoString spec = GetCacheKey(aFirstPartyURI, aURI); -+ - LOG_SCOPE_WITH_PARAM(gImgLog, "imgLoader::LoadImage", "aURI", spec.get()); - - *_retval = nsnull; -@@ -1641,7 +1670,7 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - imgCacheTable &cache = GetCache(aURI); - - if (cache.Get(spec, getter_AddRefs(entry)) && entry) { -- if (ValidateEntry(entry, aURI, aInitialDocumentURI, aReferrerURI, -+ if (ValidateEntry(entry, aURI, aFirstPartyURI, aReferrerURI, - aLoadGroup, aObserver, aCX, requestFlags, true, - aRequest, _retval, aPolicy, aLoadingPrincipal, corsmode)) { - request = getter_AddRefs(entry->GetRequest()); -@@ -1680,7 +1709,7 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - rv = NewImageChannel(getter_AddRefs(newChannel), - &forcePrincipalCheck, - aURI, -- aInitialDocumentURI, -+ aFirstPartyURI, - aReferrerURI, - aLoadGroup, - mAcceptHeader, -@@ -1702,8 +1731,8 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - do_CreateInstance(NS_LOADGROUP_CONTRACTID); - newChannel->SetLoadGroup(loadGroup); - -- request->Init(aURI, aURI, loadGroup, newChannel, entry, aCX, -- aLoadingPrincipal, corsmode); -+ request->Init(aURI, aURI, aFirstPartyURI, loadGroup, newChannel, entry, -+ aCX, aLoadingPrincipal, corsmode); - - // Pass the inner window ID of the loading document, if possible. - nsCOMPtr<nsIDocument> doc = do_QueryInterface(aCX); -@@ -1751,7 +1780,7 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - } - - // Try to add the new request into the cache. -- PutIntoCache(aURI, entry); -+ PutIntoCache(spec, entry); - } else { - LOG_MSG_WITH_PARAM(gImgLog, - "imgLoader::LoadImage |cache hit|", "request", request); -@@ -1811,6 +1840,21 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - return NS_OK; - } - -+nsCAutoString imgLoader::GetCacheKey(nsIURI *firstPartyURI, nsIURI *imgURI) -+{ -+ nsCAutoString spec, hostKey; -+ imgURI->GetSpec(spec); -+ -+ // FIXME: Should we use mozIThirdPartyUtil to get a domain from this? -+ firstPartyURI->GetHost(hostKey); -+ -+ // Make a new key using host -+ // FIXME: This might involve a couple more copies than necessary.. -+ // But man, 18 string types? Who knows which one I need to use to do -+ // this cheaply.. -+ return hostKey + nsCAutoString("&") + spec; -+} -+ - /* imgIRequest loadImageWithChannel(in nsIChannel channel, in imgIDecoderObserver aObserver, in nsISupports cx, out nsIStreamListener); */ - NS_IMETHODIMP imgLoader::LoadImageWithChannel(nsIChannel *channel, imgIDecoderObserver *aObserver, nsISupports *aCX, nsIStreamListener **listener, imgIRequest **_retval) - { -@@ -1821,22 +1865,27 @@ NS_IMETHODIMP imgLoader::LoadImageWithChannel(nsIChannel *channel, imgIDecoderOb - nsCOMPtr<nsIURI> uri; - channel->GetURI(getter_AddRefs(uri)); - -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ thirdPartySvc->GetFirstPartyURI(channel, nsnull, -+ getter_AddRefs(firstPartyURI)); -+ - nsLoadFlags requestFlags = nsIRequest::LOAD_NORMAL; - channel->GetLoadFlags(&requestFlags); - - nsRefPtr<imgCacheEntry> entry; -+ imgCacheTable &cache = GetCache(uri); -+ nsCAutoString spec = GetCacheKey(firstPartyURI, uri); - - if (requestFlags & nsIRequest::LOAD_BYPASS_CACHE) { -- RemoveFromCache(uri); -+ imgCacheQueue &queue = GetCacheQueue(uri); -+ RemoveKeyFromCache(cache, queue, spec); - } else { - // Look in the cache for our URI, and then validate it. - // XXX For now ignore aCacheKey. We will need it in the future - // for correctly dealing with image load requests that are a result - // of post data. -- imgCacheTable &cache = GetCache(uri); -- nsCAutoString spec; -- -- uri->GetSpec(spec); - - if (cache.Get(spec, getter_AddRefs(entry)) && entry) { - // We don't want to kick off another network load. So we ask -@@ -1908,7 +1957,7 @@ NS_IMETHODIMP imgLoader::LoadImageWithChannel(nsIChannel *channel, imgIDecoderOb - channel->GetOriginalURI(getter_AddRefs(originalURI)); - - // No principal specified here, because we're not passed one. -- request->Init(originalURI, uri, channel, channel, entry, -+ request->Init(originalURI, uri, firstPartyURI, channel, channel, entry, - aCX, nsnull, imgIRequest::CORS_NONE); - - ProxyListener *pl = new ProxyListener(static_cast<nsIStreamListener *>(request.get())); -@@ -1920,7 +1969,7 @@ NS_IMETHODIMP imgLoader::LoadImageWithChannel(nsIChannel *channel, imgIDecoderOb - NS_RELEASE(pl); - - // Try to add the new request into the cache. -- PutIntoCache(originalURI, entry); -+ PutIntoCache(GetCacheKey(originalURI, firstPartyURI), entry); - - rv = CreateNewProxyForRequest(request, loadGroup, aObserver, - requestFlags, nsnull, _retval); -@@ -2207,6 +2256,7 @@ NS_IMETHODIMP imgCacheValidator::OnStartRequest(nsIRequest *aRequest, nsISupport - - PRInt32 corsmode = mRequest->GetCORSMode(); - nsCOMPtr<nsIPrincipal> loadingPrincipal = mRequest->GetLoadingPrincipal(); -+ nsCOMPtr<nsIURI> firstPartyURI = mRequest->mFirstPartyURI; - - // Doom the old request's cache entry - mRequest->RemoveFromCache(); -@@ -2217,16 +2267,16 @@ NS_IMETHODIMP imgCacheValidator::OnStartRequest(nsIRequest *aRequest, nsISupport - // We use originalURI here to fulfil the imgIRequest contract on GetURI. - nsCOMPtr<nsIURI> originalURI; - channel->GetOriginalURI(getter_AddRefs(originalURI)); -- mNewRequest->Init(originalURI, uri, aRequest, channel, mNewEntry, -- mContext, loadingPrincipal, -- corsmode); -+ mNewRequest->Init(originalURI, uri, firstPartyURI, aRequest, channel, -+ mNewEntry, mContext, loadingPrincipal, corsmode); - - mDestListener = new ProxyListener(mNewRequest); - - // Try to add the new request into the cache. Note that the entry must be in - // the cache before the proxies' ownership changes, because adding a proxy - // changes the caching behaviour for imgRequests. -- sImgLoader.PutIntoCache(originalURI, mNewEntry); -+ sImgLoader.PutIntoCache(imgLoader::GetCacheKey(firstPartyURI, originalURI), -+ mNewEntry); - - PRUint32 count = mProxies.Count(); - for (PRInt32 i = count-1; i>=0; i--) { -diff --git a/image/src/imgLoader.h b/image/src/imgLoader.h -index c0ba1d4..22671b3 100644 ---- a/image/src/imgLoader.h -+++ b/image/src/imgLoader.h -@@ -261,10 +261,11 @@ public: - - static nsresult InitCache(); - -- static bool RemoveFromCache(nsIURI *aKey); -+ static nsCAutoString GetCacheKey(nsIURI *firstPartyURI, -+ nsIURI *imgURI); - static bool RemoveFromCache(imgCacheEntry *entry); -- -- static bool PutIntoCache(nsIURI *key, imgCacheEntry *entry); -+ static bool PutIntoCache(nsCAutoString key, imgCacheEntry *entry); -+ static bool RemoveMatchingUrlsFromCache(nsIURI *aKey); - - // Returns true if we should prefer evicting cache entry |two| over cache - // entry |one|. -@@ -349,9 +350,15 @@ private: // methods - static void CacheEntriesChanged(nsIURI *aURI, PRInt32 sizediff = 0); - static void CheckCacheLimits(imgCacheTable &cache, imgCacheQueue &queue); - -+ static bool RemoveKeyFromCache(imgCacheTable &cache, -+ imgCacheQueue &queue, -+ nsCAutoString key); -+ -+ - private: // data - friend class imgCacheEntry; - friend class imgMemoryReporter; -+ friend class imgRequest; - - static imgCacheTable sCache; - static imgCacheQueue sCacheQueue; -diff --git a/image/src/imgRequest.cpp b/image/src/imgRequest.cpp -index 85ec2fa..cc40e86 100644 ---- a/image/src/imgRequest.cpp -+++ b/image/src/imgRequest.cpp -@@ -137,6 +137,7 @@ imgRequest::~imgRequest() - - nsresult imgRequest::Init(nsIURI *aURI, - nsIURI *aCurrentURI, -+ nsIURI *aFirstPartyURI, - nsIRequest *aRequest, - nsIChannel *aChannel, - imgCacheEntry *aCacheEntry, -@@ -158,6 +159,7 @@ nsresult imgRequest::Init(nsIURI *aURI, - - mURI = aURI; - mCurrentURI = aCurrentURI; -+ mFirstPartyURI = aFirstPartyURI; - mRequest = aRequest; - mChannel = aChannel; - mTimedChannel = do_QueryInterface(mChannel); -@@ -351,8 +353,11 @@ void imgRequest::RemoveFromCache() - // mCacheEntry is nulled out when we have no more observers. - if (mCacheEntry) - imgLoader::RemoveFromCache(mCacheEntry); -- else -- imgLoader::RemoveFromCache(mURI); -+ else { -+ imgLoader::RemoveKeyFromCache(imgLoader::GetCache(mURI), -+ imgLoader::GetCacheQueue(mURI), -+ imgLoader::GetCacheKey(mFirstPartyURI, mURI)); -+ } - } - - mCacheEntry = nsnull; -diff --git a/image/src/imgRequest.h b/image/src/imgRequest.h -index 7b200df..6ae7ae7 100644 ---- a/image/src/imgRequest.h -+++ b/image/src/imgRequest.h -@@ -91,6 +91,7 @@ public: - - nsresult Init(nsIURI *aURI, - nsIURI *aCurrentURI, -+ nsIURI *aFirstPartyURI, - nsIRequest *aRequest, - nsIChannel *aChannel, - imgCacheEntry *aCacheEntry, -@@ -221,6 +222,8 @@ private: - nsCOMPtr<nsIURI> mURI; - // The URI of the resource we ended up loading after all redirects, etc. - nsCOMPtr<nsIURI> mCurrentURI; -+ // The first party that triggered the load -- for cookie + cache isolation -+ nsCOMPtr<nsIURI> mFirstPartyURI; - // The principal of the document which loaded this image. Used when validating for CORS. - nsCOMPtr<nsIPrincipal> mLoadingPrincipal; - // The principal of this image. -diff --git a/layout/generic/nsImageFrame.cpp b/layout/generic/nsImageFrame.cpp -index 807aa6c..7bb866b 100644 ---- a/layout/generic/nsImageFrame.cpp -+++ b/layout/generic/nsImageFrame.cpp -@@ -96,6 +96,7 @@ - #include "nsLayoutErrors.h" - #include "nsBidiUtils.h" - #include "nsBidiPresUtils.h" -+#include "mozIThirdPartyUtil.h" - - #include "gfxRect.h" - #include "ImageLayers.h" -@@ -1760,9 +1761,17 @@ nsImageFrame::LoadIcon(const nsAString& aSpec, - - // For icon loads, we don't need to merge with the loadgroup flags - nsLoadFlags loadFlags = nsIRequest::LOAD_NORMAL; -+ -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ // XXX: Should we pass the loadgroup, too? Is document ever likely -+ // to be unset? -+ thirdPartySvc->GetFirstPartyURI(nsnull, aPresContext->Document(), -+ getter_AddRefs(firstPartyURI)); - - return il->LoadImage(realURI, /* icon URI */ -- nsnull, /* initial document URI; this is only -+ firstPartyURI, /* initial document URI; this is only - relevant for cookies, so does not - apply to icons. */ - nsnull, /* referrer (not relevant for icons) */ -diff --git a/netwerk/base/public/mozIThirdPartyUtil.idl b/netwerk/base/public/mozIThirdPartyUtil.idl -index ad41985..fd2cb38 100644 ---- a/netwerk/base/public/mozIThirdPartyUtil.idl -+++ b/netwerk/base/public/mozIThirdPartyUtil.idl -@@ -40,6 +40,7 @@ - interface nsIURI; - interface nsIDOMWindow; - interface nsIChannel; -+interface nsIDocument; - - /** - * Utility functions for determining whether a given URI, channel, or window -@@ -173,6 +174,26 @@ interface mozIThirdPartyUtil : nsISupports - * @return the base domain. - */ - AUTF8String getBaseDomain(in nsIURI aHostURI); -+ -+ -+ /** -+ * getFirstPartyURI -+ * -+ * Obtain the top-level url bar URI for either a channel or a document. -+ * Either parameter may be null (but not both). -+ * -+ * @param aChannel -+ * An arbitrary channel for some content element of a first party -+ * load. Can be null. -+ * -+ * @param aDoc -+ * An arbitrary third party document. Can be null. -+ * -+ * @return the first party url bar URI for the load. -+ */ -+ nsIURI getFirstPartyURI(in nsIChannel aChannel, -+ in nsIDocument aDoc); -+ - }; - - %{ C++ -diff --git a/netwerk/cookie/nsICookiePermission.idl b/netwerk/cookie/nsICookiePermission.idl -index c7f765b..d81e2e6 100644 ---- a/netwerk/cookie/nsICookiePermission.idl -+++ b/netwerk/cookie/nsICookiePermission.idl -@@ -40,6 +40,7 @@ - interface nsICookie2; - interface nsIURI; - interface nsIChannel; -+interface nsIDocument; - - typedef long nsCookieAccess; - -diff --git a/toolkit/system/gnome/nsAlertsIconListener.cpp b/toolkit/system/gnome/nsAlertsIconListener.cpp -index 250e4fb..614e70f 100644 ---- a/toolkit/system/gnome/nsAlertsIconListener.cpp -+++ b/toolkit/system/gnome/nsAlertsIconListener.cpp -@@ -271,7 +271,8 @@ nsAlertsIconListener::StartRequest(const nsAString & aImageUrl) - if (!il) - return ShowAlert(NULL); - -- return il->LoadImage(imageUri, nsnull, nsnull, nsnull, nsnull, this, -+ // XXX: Hrmm.... Bypass cache, or isolate to imageUrl? -+ return il->LoadImage(imageUri, imageUri, nsnull, nsnull, nsnull, this, - nsnull, nsIRequest::LOAD_NORMAL, nsnull, nsnull, - nsnull, getter_AddRefs(mIconRequest)); - } -diff --git a/widget/cocoa/nsMenuItemIconX.mm b/widget/cocoa/nsMenuItemIconX.mm -index 9505ea2..bc9b856 100644 ---- a/widget/cocoa/nsMenuItemIconX.mm -+++ b/widget/cocoa/nsMenuItemIconX.mm -@@ -63,6 +63,7 @@ - #include "gfxImageSurface.h" - #include "imgIContainer.h" - #include "nsCocoaUtils.h" -+#include "mozIThirdPartyUtil.h" - - static const PRUint32 kIconWidth = 16; - static const PRUint32 kIconHeight = 16; -@@ -338,9 +339,15 @@ nsMenuItemIconX::LoadIcon(nsIURI* aIconURI) - [mNativeMenuItem setImage:sPlaceholderIconImage]; - } - -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ thirdPartySvc->GetFirstPartyURI(nsnull, document, -+ getter_AddRefs(firstPartyURI)); -+ - // Passing in null for channelPolicy here since nsMenuItemIconX::LoadIcon is - // not exposed to web content -- rv = loader->LoadImage(aIconURI, nsnull, nsnull, nsnull, loadGroup, this, -+ rv = loader->LoadImage(aIconURI, firstPartyURI, nsnull, nsnull, loadGroup, this, - nsnull, nsIRequest::LOAD_NORMAL, nsnull, nsnull, - nsnull, getter_AddRefs(mIconRequest)); - if (NS_FAILED(rv)) return rv; --- -1.7.5.4 -

1 0

[torbrowser/maint-2.3] Remove crashy cache isolation patch for now :/.
by mikeperry＠torproject.org 04 Aug '12

04 Aug '12

commit f2be42f3984acb20c58c05cf5534396fef635161 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Fri Aug 3 22:14:23 2012 -0700 Remove crashy cache isolation patch for now :/. It asserts during debug builds, and also may have issues with XUL dialogs? --- ...solate-the-Image-Cache-per-url-bar-domain.patch | 910 -------------------- 1 files changed, 0 insertions(+), 910 deletions(-) diff --git a/src/current-patches/firefox/alpha/0021-Isolate-the-Image-Cache-per-url-bar-domain.patch b/src/current-patches/firefox/alpha/0021-Isolate-the-Image-Cache-per-url-bar-domain.patch deleted file mode 100644 index b21536c..0000000 --- a/src/current-patches/firefox/alpha/0021-Isolate-the-Image-Cache-per-url-bar-domain.patch +++ /dev/null @@ -1,910 +0,0 @@ -From 1b18f231fce26c0b6d3c2bb2c6e30e30c8e05a1e Mon Sep 17 00:00:00 2001 -From: Mike Perry <mikeperry-git(a)torproject.org> -Date: Mon, 30 Jul 2012 18:37:36 -0700 -Subject: [PATCH 21/21] Isolate the Image Cache per url bar domain. - -Also adds a new API to mozIThirdPartyUtil to allow you to get the url bar URI -for a channel or nsIDocument. ---- - content/base/src/ThirdPartyUtil.cpp | 52 +++++ - content/base/src/ThirdPartyUtil.h | 2 + - content/base/src/nsContentUtils.cpp | 13 +- - embedding/browser/webBrowser/nsContextMenuInfo.cpp | 29 ++- - extensions/cookie/nsCookiePermission.cpp | 3 + - image/public/imgILoader.idl | 4 +- - image/src/imgLoader.cpp | 200 ++++++++++++-------- - image/src/imgLoader.h | 13 +- - image/src/imgRequest.cpp | 9 +- - image/src/imgRequest.h | 3 + - layout/generic/nsImageFrame.cpp | 11 +- - netwerk/base/public/mozIThirdPartyUtil.idl | 21 ++ - netwerk/cookie/nsICookiePermission.idl | 1 + - toolkit/system/gnome/nsAlertsIconListener.cpp | 3 +- - widget/cocoa/nsMenuItemIconX.mm | 9 +- - 15 files changed, 272 insertions(+), 101 deletions(-) - -diff --git a/content/base/src/ThirdPartyUtil.cpp b/content/base/src/ThirdPartyUtil.cpp -index 6a415e9..62333f3 100644 ---- a/content/base/src/ThirdPartyUtil.cpp -+++ b/content/base/src/ThirdPartyUtil.cpp -@@ -40,6 +40,9 @@ - #include "nsIServiceManager.h" - #include "nsIHttpChannelInternal.h" - #include "nsIDOMWindow.h" -+#include "nsICookiePermission.h" -+#include "nsIDOMDocument.h" -+#include "nsIDocument.h" - #include "nsILoadContext.h" - #include "nsIPrincipal.h" - #include "nsIScriptObjectPrincipal.h" -@@ -54,6 +57,7 @@ ThirdPartyUtil::Init() - - nsresult rv; - mTLDService = do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID, &rv); -+ mCookiePermissions = do_GetService(NS_COOKIEPERMISSION_CONTRACTID); - return rv; - } - -@@ -315,3 +319,51 @@ ThirdPartyUtil::GetBaseDomain(nsIURI* aHostURI, - - return NS_OK; - } -+ -+NS_IMETHODIMP -+ThirdPartyUtil::GetFirstPartyURI(nsIChannel *aChannel, -+ nsIDocument *aDoc, -+ nsIURI **aOutput) -+{ -+ nsresult rv = NS_ERROR_NULL_POINTER; -+ -+ if (!aChannel && aDoc) { -+ aChannel = aDoc->GetChannel(); -+ } -+ -+ // If aChannel is specified or available, use the official route -+ // for sure -+ if (aChannel) { -+ rv = mCookiePermissions->GetOriginatingURI(aChannel, aOutput); -+ } -+ -+ // If the channel was missing, closed or broken, try the -+ // window hierarchy directly. -+ // -+ // This might fail to work for first-party loads themselves, but -+ // we don't need this codepath for that case. -+ if (NS_FAILED(rv) && aDoc) { -+ nsCOMPtr<nsIDOMWindow> top; -+ nsCOMPtr<nsIDOMDocument> topDDoc; -+ -+ aDoc->GetWindow()->GetTop(getter_AddRefs(top)); -+ top->GetDocument(getter_AddRefs(topDDoc)); -+ -+ nsCOMPtr<nsIDocument> topDoc(do_QueryInterface(topDDoc)); -+ *aOutput = topDoc->GetOriginalURI(); -+ -+ if (*aOutput) -+ rv = NS_OK; -+ } -+ -+ // TODO: We could provide a route through the loadgroup + notification -+ // callbacks too, but either channel or document was always available -+ // in the cases where this function was originally needed (the image cache). -+ // The notification callbacks also appear to suffers from the same limitation -+ // as the document path. See nsICookiePermissions.GetOriginatingURI() for -+ // details. -+ -+ return rv; -+} -+ -+ -diff --git a/content/base/src/ThirdPartyUtil.h b/content/base/src/ThirdPartyUtil.h -index 58ddb15..ba70162 100644 ---- a/content/base/src/ThirdPartyUtil.h -+++ b/content/base/src/ThirdPartyUtil.h -@@ -46,6 +46,7 @@ - class nsIURI; - class nsIChannel; - class nsIDOMWindow; -+class nsICookiePermission; - - class ThirdPartyUtil : public mozIThirdPartyUtil - { -@@ -61,6 +62,7 @@ private: - static already_AddRefed<nsIURI> GetURIFromWindow(nsIDOMWindow* aWin); - - nsCOMPtr<nsIEffectiveTLDService> mTLDService; -+ nsCOMPtr<nsICookiePermission> mCookiePermissions; - }; - - #endif -diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp -index 5c85697..4368609 100644 ---- a/content/base/src/nsContentUtils.cpp -+++ b/content/base/src/nsContentUtils.cpp -@@ -179,6 +179,7 @@ static NS_DEFINE_CID(kXTFServiceCID, NS_XTFSERVICE_CID); - #include "nsIDOMHTMLInputElement.h" - #include "nsParserConstants.h" - #include "nsIWebNavigation.h" -+#include "mozIThirdPartyUtil.h" - - #ifdef IBMBIDI - #include "nsIBidiKeyboard.h" -@@ -2629,8 +2630,6 @@ nsContentUtils::LoadImage(nsIURI* aURI, nsIDocument* aLoadingDocument, - nsCOMPtr<nsILoadGroup> loadGroup = aLoadingDocument->GetDocumentLoadGroup(); - NS_ASSERTION(loadGroup, "Could not get loadgroup; onload may fire too early"); - -- nsIURI *documentURI = aLoadingDocument->GetDocumentURI(); -- - // check for a Content Security Policy to pass down to the channel that - // will get created to load the image - nsCOMPtr<nsIChannelPolicy> channelPolicy; -@@ -2647,11 +2646,15 @@ nsContentUtils::LoadImage(nsIURI* aURI, nsIDocument* aLoadingDocument, - - // Make the URI immutable so people won't change it under us - NS_TryToSetImmutable(aURI); -+ -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ thirdPartySvc->GetFirstPartyURI(nsnull, aLoadingDocument, -+ getter_AddRefs(firstPartyURI)); - -- // XXXbz using "documentURI" for the initialDocumentURI is not quite -- // right, but the best we can do here... - return imgLoader->LoadImage(aURI, /* uri to load */ -- documentURI, /* initialDocumentURI */ -+ firstPartyURI, /* firstPartyURI */ - aReferrer, /* referrer */ - aLoadingPrincipal, /* loading principal */ - loadGroup, /* loadgroup */ -diff --git a/embedding/browser/webBrowser/nsContextMenuInfo.cpp b/embedding/browser/webBrowser/nsContextMenuInfo.cpp -index 045482a..78f5fc4 100644 ---- a/embedding/browser/webBrowser/nsContextMenuInfo.cpp -+++ b/embedding/browser/webBrowser/nsContextMenuInfo.cpp -@@ -62,6 +62,7 @@ - #include "nsIChannelPolicy.h" - #include "nsIContentSecurityPolicy.h" - #include "nsIContentPolicy.h" -+#include "mozIThirdPartyUtil.h" - - //***************************************************************************** - // class nsContextMenuInfo -@@ -305,15 +306,15 @@ nsContextMenuInfo::GetBackgroundImageRequestInternal(nsIDOMNode *aDOMNode, imgIR - nsCOMPtr<nsIPrincipal> principal; - nsCOMPtr<nsIChannelPolicy> channelPolicy; - nsCOMPtr<nsIContentSecurityPolicy> csp; -- if (doc) { -- principal = doc->NodePrincipal(); -- nsresult rv = principal->GetCsp(getter_AddRefs(csp)); -- NS_ENSURE_SUCCESS(rv, rv); -- if (csp) { -- channelPolicy = do_CreateInstance("@mozilla.org/nschannelpolicy;1"); -- channelPolicy->SetContentSecurityPolicy(csp); -- channelPolicy->SetLoadType(nsIContentPolicy::TYPE_IMAGE); -- } -+ NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE); -+ -+ principal = doc->NodePrincipal(); -+ nsresult rv = principal->GetCsp(getter_AddRefs(csp)); -+ NS_ENSURE_SUCCESS(rv, rv); -+ if (csp) { -+ channelPolicy = do_CreateInstance("@mozilla.org/nschannelpolicy;1"); -+ channelPolicy->SetContentSecurityPolicy(csp); -+ channelPolicy->SetLoadType(nsIContentPolicy::TYPE_IMAGE); - } - - while (true) { -@@ -340,8 +341,14 @@ nsContextMenuInfo::GetBackgroundImageRequestInternal(nsIDOMNode *aDOMNode, imgIR - nsCOMPtr<imgILoader> il(do_GetService( - "@mozilla.org/image/loader;1")); - NS_ENSURE_TRUE(il, NS_ERROR_FAILURE); -- -- return il->LoadImage(bgUri, nsnull, nsnull, principal, nsnull, -+ -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ thirdPartySvc->GetFirstPartyURI(nsnull, doc, -+ getter_AddRefs(firstPartyURI)); -+ -+ return il->LoadImage(bgUri, firstPartyURI, nsnull, principal, nsnull, - nsnull, nsnull, nsIRequest::LOAD_NORMAL, nsnull, - nsnull, channelPolicy, aRequest); - } -diff --git a/extensions/cookie/nsCookiePermission.cpp b/extensions/cookie/nsCookiePermission.cpp -index 577ee7e..c15ddc9 100644 ---- a/extensions/cookie/nsCookiePermission.cpp -+++ b/extensions/cookie/nsCookiePermission.cpp -@@ -445,6 +445,9 @@ nsCookiePermission::GetOriginatingURI(nsIChannel *aChannel, - - return NS_OK; - } -+ -+ // TODO: Why don't we just use this here: -+ // httpChannelInternal->GetDocumentURI(aURI); - } - - // find the associated window and its top window -diff --git a/image/public/imgILoader.idl b/image/public/imgILoader.idl -index 611b939..db9320b 100644 ---- a/image/public/imgILoader.idl -+++ b/image/public/imgILoader.idl -@@ -71,7 +71,7 @@ interface imgILoader : nsISupports - /** - * Start the load and decode of an image. - * @param aURI the URI to load -- * @param aInitialDocumentURI the URI that 'initiated' the load -- used for 3rd party cookie blocking -+ * @param aFirstPartyURI the urlbar URI that 'initiated' the load -- used for 3rd party blocking - * @param aReferrerURI the 'referring' URI - * @param aLoadingPrincipal the principal of the loading document - * @param aLoadGroup Loadgroup to put the image load into -@@ -90,7 +90,7 @@ interface imgILoader : nsISupports - * goes away. - */ - imgIRequest loadImage(in nsIURI aURI, -- in nsIURI aInitialDocumentURL, -+ in nsIURI aFirstPartyURI, - in nsIURI aReferrerURI, - in nsIPrincipal aLoadingPrincipal, - in nsILoadGroup aLoadGroup, -diff --git a/image/src/imgLoader.cpp b/image/src/imgLoader.cpp -index 36c8e6a..dd16771 100644 ---- a/image/src/imgLoader.cpp -+++ b/image/src/imgLoader.cpp -@@ -92,6 +92,7 @@ - #include "nsIHttpChannelInternal.h" - #include "nsIContentSecurityPolicy.h" - #include "nsIChannelPolicy.h" -+#include "mozIThirdPartyUtil.h" - - #include "nsContentUtils.h" - -@@ -504,7 +505,7 @@ static nsresult NewImageChannel(nsIChannel **aResult, - // aLoadingPrincipal and false otherwise. - bool *aForcePrincipalCheckForCacheEntry, - nsIURI *aURI, -- nsIURI *aInitialDocumentURI, -+ nsIURI *aFirstPartyURI, - nsIURI *aReferringURI, - nsILoadGroup *aLoadGroup, - const nsCString& aAcceptHeader, -@@ -556,7 +557,7 @@ static nsresult NewImageChannel(nsIChannel **aResult, - - nsCOMPtr<nsIHttpChannelInternal> httpChannelInternal = do_QueryInterface(newHttpChannel); - NS_ENSURE_TRUE(httpChannelInternal, NS_ERROR_UNEXPECTED); -- httpChannelInternal->SetDocumentURI(aInitialDocumentURI); -+ httpChannelInternal->SetDocumentURI(aFirstPartyURI); - newHttpChannel->SetReferrer(aReferringURI); - } - -@@ -999,34 +1000,61 @@ NS_IMETHODIMP imgLoader::ClearCache(bool chrome) - /* void removeEntry(in nsIURI uri); */ - NS_IMETHODIMP imgLoader::RemoveEntry(nsIURI *uri) - { -- if (RemoveFromCache(uri)) -+ if (RemoveMatchingUrlsFromCache(uri)) - return NS_OK; - - return NS_ERROR_NOT_AVAILABLE; - } - -+static PLDHashOperator EnumAllEntries(const nsACString&, -+ nsRefPtr<imgCacheEntry> &aData, -+ void *data) -+{ -+ nsTArray<nsRefPtr<imgCacheEntry> > *entries = -+ reinterpret_cast<nsTArray<nsRefPtr<imgCacheEntry> > *>(data); -+ -+ entries->AppendElement(aData); -+ -+ return PL_DHASH_NEXT; -+} -+ - /* imgIRequest findEntry(in nsIURI uri); */ - NS_IMETHODIMP imgLoader::FindEntryProperties(nsIURI *uri, nsIProperties **_retval) - { - nsRefPtr<imgCacheEntry> entry; -- nsCAutoString spec; - imgCacheTable &cache = GetCache(uri); -- -- uri->GetSpec(spec); - *_retval = nsnull; - -- if (cache.Get(spec, getter_AddRefs(entry)) && entry) { -- if (gCacheTracker && entry->HasNoProxies()) -- gCacheTracker->MarkUsed(entry); -+ // We must traverse the whole cache in O(N) looking for the first -+ // matching URI. -+ // -+ // TODO: For now, it's ok to pick at random here. The images should be -+ // identical unless there is a cache-tracking attack. And even if they -+ // are not identical due to attack, this code is only used for save -+ // dialogs at this point, so no differentiating info is leaked to -+ // content. -+ nsTArray<nsRefPtr<imgCacheEntry> > entries; -+ cache.Enumerate(EnumAllEntries, &entries); -+ -+ for (PRUint32 i = 0; i < entries.Length(); ++i) { -+ bool isEqual = false; - -- nsRefPtr<imgRequest> request = getter_AddRefs(entry->GetRequest()); -+ nsRefPtr<imgRequest> request = getter_AddRefs(entries[i]->GetRequest()); - if (request) { -- *_retval = request->Properties(); -- NS_ADDREF(*_retval); -+ request->mURI->Equals(uri, &isEqual); -+ if (isEqual) { -+ if (gCacheTracker && entries[i]->HasNoProxies()) -+ gCacheTracker->MarkUsed(entries[i]); -+ -+ *_retval = request->Properties(); -+ } - } - } - -- return NS_OK; -+ if (*_retval) -+ return NS_OK; -+ -+ return NS_ERROR_NOT_AVAILABLE; - } - - void imgLoader::Shutdown() -@@ -1054,20 +1082,18 @@ void imgLoader::MinimizeCaches() - EvictEntries(sChromeCacheQueue); - } - --bool imgLoader::PutIntoCache(nsIURI *key, imgCacheEntry *entry) -+bool imgLoader::PutIntoCache(nsCAutoString key, -+ imgCacheEntry *entry) - { -- imgCacheTable &cache = GetCache(key); -- -- nsCAutoString spec; -- key->GetSpec(spec); -- -- LOG_STATIC_FUNC_WITH_PARAM(gImgLog, "imgLoader::PutIntoCache", "uri", spec.get()); -+ LOG_STATIC_FUNC_WITH_PARAM(gImgLog, "imgLoader::PutIntoCache", "uri", key.get()); -+ imgCacheTable &cache = GetCache(entry->mRequest->mURI); -+ imgCacheQueue &queue = GetCacheQueue(entry->mRequest->mURI); - - // Check to see if this request already exists in the cache and is being - // loaded on a different thread. If so, don't allow this entry to be added to - // the cache. - nsRefPtr<imgCacheEntry> tmpCacheEntry; -- if (cache.Get(spec, getter_AddRefs(tmpCacheEntry)) && tmpCacheEntry) { -+ if (cache.Get(key, getter_AddRefs(tmpCacheEntry)) && tmpCacheEntry) { - PR_LOG(gImgLog, PR_LOG_DEBUG, - ("[this=%p] imgLoader::PutIntoCache -- Element already in the cache", nsnull)); - nsRefPtr<imgRequest> tmpRequest = getter_AddRefs(tmpCacheEntry->GetRequest()); -@@ -1077,13 +1103,13 @@ bool imgLoader::PutIntoCache(nsIURI *key, imgCacheEntry *entry) - PR_LOG(gImgLog, PR_LOG_DEBUG, - ("[this=%p] imgLoader::PutIntoCache -- Replacing cached element", nsnull)); - -- RemoveFromCache(key); -+ RemoveKeyFromCache(cache, queue, key); - } else { - PR_LOG(gImgLog, PR_LOG_DEBUG, - ("[this=%p] imgLoader::PutIntoCache -- Element NOT already in the cache", nsnull)); - } - -- if (!cache.Put(spec, entry)) -+ if (!cache.Put(key, entry)) - return false; - - // We can be called to resurrect an evicted entry. -@@ -1099,7 +1125,6 @@ bool imgLoader::PutIntoCache(nsIURI *key, imgCacheEntry *entry) - addrv = gCacheTracker->AddObject(entry); - - if (NS_SUCCEEDED(addrv)) { -- imgCacheQueue &queue = GetCacheQueue(key); - queue.Push(entry); - } - } -@@ -1205,7 +1230,7 @@ void imgLoader::CheckCacheLimits(imgCacheTable &cache, imgCacheQueue &queue) - - bool imgLoader::ValidateRequestWithNewChannel(imgRequest *request, - nsIURI *aURI, -- nsIURI *aInitialDocumentURI, -+ nsIURI *aFirstPartyURI, - nsIURI *aReferrerURI, - nsILoadGroup *aLoadGroup, - imgIDecoderObserver *aObserver, -@@ -1257,7 +1282,7 @@ bool imgLoader::ValidateRequestWithNewChannel(imgRequest *request, - rv = NewImageChannel(getter_AddRefs(newChannel), - &forcePrincipalCheck, - aURI, -- aInitialDocumentURI, -+ aFirstPartyURI, - aReferrerURI, - aLoadGroup, - mAcceptHeader, -@@ -1326,7 +1351,7 @@ bool imgLoader::ValidateRequestWithNewChannel(imgRequest *request, - - bool imgLoader::ValidateEntry(imgCacheEntry *aEntry, - nsIURI *aURI, -- nsIURI *aInitialDocumentURI, -+ nsIURI *aFirstPartyURI, - nsIURI *aReferrerURI, - nsILoadGroup *aLoadGroup, - imgIDecoderObserver *aObserver, -@@ -1432,7 +1457,7 @@ bool imgLoader::ValidateEntry(imgCacheEntry *aEntry, - if (validateRequest && aCanMakeNewChannel) { - LOG_SCOPE(gImgLog, "imgLoader::ValidateRequest |cache hit| must validate"); - -- return ValidateRequestWithNewChannel(request, aURI, aInitialDocumentURI, -+ return ValidateRequestWithNewChannel(request, aURI, aFirstPartyURI, - aReferrerURI, aLoadGroup, aObserver, - aCX, aLoadFlags, aExistingRequest, - aProxyRequest, aPolicy, -@@ -1442,16 +1467,32 @@ bool imgLoader::ValidateEntry(imgCacheEntry *aEntry, - return !validateRequest; - } - -- --bool imgLoader::RemoveFromCache(nsIURI *aKey) -+bool imgLoader::RemoveMatchingUrlsFromCache(nsIURI *aKey) - { -- if (!aKey) return false; -- -+ bool rv = true; - imgCacheTable &cache = GetCache(aKey); -- imgCacheQueue &queue = GetCacheQueue(aKey); - -- nsCAutoString spec; -- aKey->GetSpec(spec); -+ // We have to make a temporary, since RemoveFromCache removes the element -+ // from the queue, invalidating iterators. -+ nsTArray<nsRefPtr<imgCacheEntry> > entries; -+ cache.Enumerate(EnumAllEntries, &entries); -+ -+ for (PRUint32 i = 0; i < entries.Length(); ++i) { -+ bool isEqual = false; -+ -+ entries[i]->mRequest->mURI->Equals(aKey, &isEqual); -+ if (isEqual && !RemoveFromCache(entries[i])) -+ rv = false; -+ } -+ -+ return rv; -+} -+ -+bool imgLoader::RemoveKeyFromCache(imgCacheTable &cache, -+ imgCacheQueue &queue, -+ nsCAutoString spec) -+{ -+ if (spec.IsEmpty()) return false; - - LOG_STATIC_FUNC_WITH_PARAM(gImgLog, "imgLoader::RemoveFromCache", "uri", spec.get()); - -@@ -1485,12 +1526,13 @@ bool imgLoader::RemoveFromCache(imgCacheEntry *entry) - - nsRefPtr<imgRequest> request(getter_AddRefs(entry->GetRequest())); - if (request) { -- nsCOMPtr<nsIURI> key; -- if (NS_SUCCEEDED(request->GetURI(getter_AddRefs(key))) && key) { -- imgCacheTable &cache = GetCache(key); -- imgCacheQueue &queue = GetCacheQueue(key); -- nsCAutoString spec; -- key->GetSpec(spec); -+ nsCOMPtr<nsIURI> imgURI = request->mURI; -+ nsCOMPtr<nsIURI> firstPartyURI = request->mFirstPartyURI; -+ -+ if (imgURI && firstPartyURI) { -+ imgCacheTable &cache = GetCache(imgURI); -+ imgCacheQueue &queue = GetCacheQueue(imgURI); -+ nsCAutoString spec = GetCacheKey(firstPartyURI, imgURI); - - LOG_STATIC_FUNC_WITH_PARAM(gImgLog, "imgLoader::RemoveFromCache", "entry's uri", spec.get()); - -@@ -1513,18 +1555,6 @@ bool imgLoader::RemoveFromCache(imgCacheEntry *entry) - return false; - } - --static PLDHashOperator EnumEvictEntries(const nsACString&, -- nsRefPtr<imgCacheEntry> &aData, -- void *data) --{ -- nsTArray<nsRefPtr<imgCacheEntry> > *entries = -- reinterpret_cast<nsTArray<nsRefPtr<imgCacheEntry> > *>(data); -- -- entries->AppendElement(aData); -- -- return PL_DHASH_NEXT; --} -- - nsresult imgLoader::EvictEntries(imgCacheTable &aCacheToClear) - { - LOG_STATIC_FUNC(gImgLog, "imgLoader::EvictEntries table"); -@@ -1532,7 +1562,7 @@ nsresult imgLoader::EvictEntries(imgCacheTable &aCacheToClear) - // We have to make a temporary, since RemoveFromCache removes the element - // from the queue, invalidating iterators. - nsTArray<nsRefPtr<imgCacheEntry> > entries; -- aCacheToClear.Enumerate(EnumEvictEntries, &entries); -+ aCacheToClear.Enumerate(EnumAllEntries, &entries); - - for (PRUint32 i = 0; i < entries.Length(); ++i) - if (!RemoveFromCache(entries[i])) -@@ -1565,11 +1595,10 @@ nsresult imgLoader::EvictEntries(imgCacheQueue &aQueueToClear) - nsIRequest::VALIDATE_NEVER | \ - nsIRequest::VALIDATE_ONCE_PER_SESSION) - -- --/* imgIRequest loadImage (in nsIURI aURI, in nsIURI initialDocumentURI, in nsIPrincipal loadingPrincipal, in nsILoadGroup aLoadGroup, in imgIDecoderObserver aObserver, in nsISupports aCX, in nsLoadFlags aLoadFlags, in nsISupports cacheKey, in imgIRequest aRequest); */ -+/* imgIRequest loadImage (in nsIURI aURI, in nsIURI aUrlBarURI, in nsIPrincipal loadingPrincipal, in nsILoadGroup aLoadGroup, in imgIDecoderObserver aObserver, in nsISupports aCX, in nsLoadFlags aLoadFlags, in nsISupports cacheKey, in imgIRequest aRequest); */ - - NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, -- nsIURI *aInitialDocumentURI, -+ nsIURI *aFirstPartyURI, - nsIURI *aReferrerURI, - nsIPrincipal* aLoadingPrincipal, - nsILoadGroup *aLoadGroup, -@@ -1588,8 +1617,8 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - if (!aURI) - return NS_ERROR_NULL_POINTER; - -- nsCAutoString spec; -- aURI->GetSpec(spec); -+ nsCAutoString spec = GetCacheKey(aFirstPartyURI, aURI); -+ - LOG_SCOPE_WITH_PARAM(gImgLog, "imgLoader::LoadImage", "aURI", spec.get()); - - *_retval = nsnull; -@@ -1641,7 +1670,7 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - imgCacheTable &cache = GetCache(aURI); - - if (cache.Get(spec, getter_AddRefs(entry)) && entry) { -- if (ValidateEntry(entry, aURI, aInitialDocumentURI, aReferrerURI, -+ if (ValidateEntry(entry, aURI, aFirstPartyURI, aReferrerURI, - aLoadGroup, aObserver, aCX, requestFlags, true, - aRequest, _retval, aPolicy, aLoadingPrincipal, corsmode)) { - request = getter_AddRefs(entry->GetRequest()); -@@ -1680,7 +1709,7 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - rv = NewImageChannel(getter_AddRefs(newChannel), - &forcePrincipalCheck, - aURI, -- aInitialDocumentURI, -+ aFirstPartyURI, - aReferrerURI, - aLoadGroup, - mAcceptHeader, -@@ -1702,8 +1731,8 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - do_CreateInstance(NS_LOADGROUP_CONTRACTID); - newChannel->SetLoadGroup(loadGroup); - -- request->Init(aURI, aURI, loadGroup, newChannel, entry, aCX, -- aLoadingPrincipal, corsmode); -+ request->Init(aURI, aURI, aFirstPartyURI, loadGroup, newChannel, entry, -+ aCX, aLoadingPrincipal, corsmode); - - // Pass the inner window ID of the loading document, if possible. - nsCOMPtr<nsIDocument> doc = do_QueryInterface(aCX); -@@ -1751,7 +1780,7 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - } - - // Try to add the new request into the cache. -- PutIntoCache(aURI, entry); -+ PutIntoCache(spec, entry); - } else { - LOG_MSG_WITH_PARAM(gImgLog, - "imgLoader::LoadImage |cache hit|", "request", request); -@@ -1811,6 +1840,21 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIURI *aURI, - return NS_OK; - } - -+nsCAutoString imgLoader::GetCacheKey(nsIURI *firstPartyURI, nsIURI *imgURI) -+{ -+ nsCAutoString spec, hostKey; -+ imgURI->GetSpec(spec); -+ -+ // FIXME: Should we use mozIThirdPartyUtil to get a domain from this? -+ firstPartyURI->GetHost(hostKey); -+ -+ // Make a new key using host -+ // FIXME: This might involve a couple more copies than necessary.. -+ // But man, 18 string types? Who knows which one I need to use to do -+ // this cheaply.. -+ return hostKey + nsCAutoString("&") + spec; -+} -+ - /* imgIRequest loadImageWithChannel(in nsIChannel channel, in imgIDecoderObserver aObserver, in nsISupports cx, out nsIStreamListener); */ - NS_IMETHODIMP imgLoader::LoadImageWithChannel(nsIChannel *channel, imgIDecoderObserver *aObserver, nsISupports *aCX, nsIStreamListener **listener, imgIRequest **_retval) - { -@@ -1821,22 +1865,27 @@ NS_IMETHODIMP imgLoader::LoadImageWithChannel(nsIChannel *channel, imgIDecoderOb - nsCOMPtr<nsIURI> uri; - channel->GetURI(getter_AddRefs(uri)); - -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ thirdPartySvc->GetFirstPartyURI(channel, nsnull, -+ getter_AddRefs(firstPartyURI)); -+ - nsLoadFlags requestFlags = nsIRequest::LOAD_NORMAL; - channel->GetLoadFlags(&requestFlags); - - nsRefPtr<imgCacheEntry> entry; -+ imgCacheTable &cache = GetCache(uri); -+ nsCAutoString spec = GetCacheKey(firstPartyURI, uri); - - if (requestFlags & nsIRequest::LOAD_BYPASS_CACHE) { -- RemoveFromCache(uri); -+ imgCacheQueue &queue = GetCacheQueue(uri); -+ RemoveKeyFromCache(cache, queue, spec); - } else { - // Look in the cache for our URI, and then validate it. - // XXX For now ignore aCacheKey. We will need it in the future - // for correctly dealing with image load requests that are a result - // of post data. -- imgCacheTable &cache = GetCache(uri); -- nsCAutoString spec; -- -- uri->GetSpec(spec); - - if (cache.Get(spec, getter_AddRefs(entry)) && entry) { - // We don't want to kick off another network load. So we ask -@@ -1908,7 +1957,7 @@ NS_IMETHODIMP imgLoader::LoadImageWithChannel(nsIChannel *channel, imgIDecoderOb - channel->GetOriginalURI(getter_AddRefs(originalURI)); - - // No principal specified here, because we're not passed one. -- request->Init(originalURI, uri, channel, channel, entry, -+ request->Init(originalURI, uri, firstPartyURI, channel, channel, entry, - aCX, nsnull, imgIRequest::CORS_NONE); - - ProxyListener *pl = new ProxyListener(static_cast<nsIStreamListener *>(request.get())); -@@ -1920,7 +1969,7 @@ NS_IMETHODIMP imgLoader::LoadImageWithChannel(nsIChannel *channel, imgIDecoderOb - NS_RELEASE(pl); - - // Try to add the new request into the cache. -- PutIntoCache(originalURI, entry); -+ PutIntoCache(GetCacheKey(originalURI, firstPartyURI), entry); - - rv = CreateNewProxyForRequest(request, loadGroup, aObserver, - requestFlags, nsnull, _retval); -@@ -2207,6 +2256,7 @@ NS_IMETHODIMP imgCacheValidator::OnStartRequest(nsIRequest *aRequest, nsISupport - - PRInt32 corsmode = mRequest->GetCORSMode(); - nsCOMPtr<nsIPrincipal> loadingPrincipal = mRequest->GetLoadingPrincipal(); -+ nsCOMPtr<nsIURI> firstPartyURI = mRequest->mFirstPartyURI; - - // Doom the old request's cache entry - mRequest->RemoveFromCache(); -@@ -2217,16 +2267,16 @@ NS_IMETHODIMP imgCacheValidator::OnStartRequest(nsIRequest *aRequest, nsISupport - // We use originalURI here to fulfil the imgIRequest contract on GetURI. - nsCOMPtr<nsIURI> originalURI; - channel->GetOriginalURI(getter_AddRefs(originalURI)); -- mNewRequest->Init(originalURI, uri, aRequest, channel, mNewEntry, -- mContext, loadingPrincipal, -- corsmode); -+ mNewRequest->Init(originalURI, uri, firstPartyURI, aRequest, channel, -+ mNewEntry, mContext, loadingPrincipal, corsmode); - - mDestListener = new ProxyListener(mNewRequest); - - // Try to add the new request into the cache. Note that the entry must be in - // the cache before the proxies' ownership changes, because adding a proxy - // changes the caching behaviour for imgRequests. -- sImgLoader.PutIntoCache(originalURI, mNewEntry); -+ sImgLoader.PutIntoCache(imgLoader::GetCacheKey(firstPartyURI, originalURI), -+ mNewEntry); - - PRUint32 count = mProxies.Count(); - for (PRInt32 i = count-1; i>=0; i--) { -diff --git a/image/src/imgLoader.h b/image/src/imgLoader.h -index c0ba1d4..22671b3 100644 ---- a/image/src/imgLoader.h -+++ b/image/src/imgLoader.h -@@ -261,10 +261,11 @@ public: - - static nsresult InitCache(); - -- static bool RemoveFromCache(nsIURI *aKey); -+ static nsCAutoString GetCacheKey(nsIURI *firstPartyURI, -+ nsIURI *imgURI); - static bool RemoveFromCache(imgCacheEntry *entry); -- -- static bool PutIntoCache(nsIURI *key, imgCacheEntry *entry); -+ static bool PutIntoCache(nsCAutoString key, imgCacheEntry *entry); -+ static bool RemoveMatchingUrlsFromCache(nsIURI *aKey); - - // Returns true if we should prefer evicting cache entry |two| over cache - // entry |one|. -@@ -349,9 +350,15 @@ private: // methods - static void CacheEntriesChanged(nsIURI *aURI, PRInt32 sizediff = 0); - static void CheckCacheLimits(imgCacheTable &cache, imgCacheQueue &queue); - -+ static bool RemoveKeyFromCache(imgCacheTable &cache, -+ imgCacheQueue &queue, -+ nsCAutoString key); -+ -+ - private: // data - friend class imgCacheEntry; - friend class imgMemoryReporter; -+ friend class imgRequest; - - static imgCacheTable sCache; - static imgCacheQueue sCacheQueue; -diff --git a/image/src/imgRequest.cpp b/image/src/imgRequest.cpp -index 85ec2fa..cc40e86 100644 ---- a/image/src/imgRequest.cpp -+++ b/image/src/imgRequest.cpp -@@ -137,6 +137,7 @@ imgRequest::~imgRequest() - - nsresult imgRequest::Init(nsIURI *aURI, - nsIURI *aCurrentURI, -+ nsIURI *aFirstPartyURI, - nsIRequest *aRequest, - nsIChannel *aChannel, - imgCacheEntry *aCacheEntry, -@@ -158,6 +159,7 @@ nsresult imgRequest::Init(nsIURI *aURI, - - mURI = aURI; - mCurrentURI = aCurrentURI; -+ mFirstPartyURI = aFirstPartyURI; - mRequest = aRequest; - mChannel = aChannel; - mTimedChannel = do_QueryInterface(mChannel); -@@ -351,8 +353,11 @@ void imgRequest::RemoveFromCache() - // mCacheEntry is nulled out when we have no more observers. - if (mCacheEntry) - imgLoader::RemoveFromCache(mCacheEntry); -- else -- imgLoader::RemoveFromCache(mURI); -+ else { -+ imgLoader::RemoveKeyFromCache(imgLoader::GetCache(mURI), -+ imgLoader::GetCacheQueue(mURI), -+ imgLoader::GetCacheKey(mFirstPartyURI, mURI)); -+ } - } - - mCacheEntry = nsnull; -diff --git a/image/src/imgRequest.h b/image/src/imgRequest.h -index 7b200df..6ae7ae7 100644 ---- a/image/src/imgRequest.h -+++ b/image/src/imgRequest.h -@@ -91,6 +91,7 @@ public: - - nsresult Init(nsIURI *aURI, - nsIURI *aCurrentURI, -+ nsIURI *aFirstPartyURI, - nsIRequest *aRequest, - nsIChannel *aChannel, - imgCacheEntry *aCacheEntry, -@@ -221,6 +222,8 @@ private: - nsCOMPtr<nsIURI> mURI; - // The URI of the resource we ended up loading after all redirects, etc. - nsCOMPtr<nsIURI> mCurrentURI; -+ // The first party that triggered the load -- for cookie + cache isolation -+ nsCOMPtr<nsIURI> mFirstPartyURI; - // The principal of the document which loaded this image. Used when validating for CORS. - nsCOMPtr<nsIPrincipal> mLoadingPrincipal; - // The principal of this image. -diff --git a/layout/generic/nsImageFrame.cpp b/layout/generic/nsImageFrame.cpp -index 807aa6c..7bb866b 100644 ---- a/layout/generic/nsImageFrame.cpp -+++ b/layout/generic/nsImageFrame.cpp -@@ -96,6 +96,7 @@ - #include "nsLayoutErrors.h" - #include "nsBidiUtils.h" - #include "nsBidiPresUtils.h" -+#include "mozIThirdPartyUtil.h" - - #include "gfxRect.h" - #include "ImageLayers.h" -@@ -1760,9 +1761,17 @@ nsImageFrame::LoadIcon(const nsAString& aSpec, - - // For icon loads, we don't need to merge with the loadgroup flags - nsLoadFlags loadFlags = nsIRequest::LOAD_NORMAL; -+ -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ // XXX: Should we pass the loadgroup, too? Is document ever likely -+ // to be unset? -+ thirdPartySvc->GetFirstPartyURI(nsnull, aPresContext->Document(), -+ getter_AddRefs(firstPartyURI)); - - return il->LoadImage(realURI, /* icon URI */ -- nsnull, /* initial document URI; this is only -+ firstPartyURI, /* initial document URI; this is only - relevant for cookies, so does not - apply to icons. */ - nsnull, /* referrer (not relevant for icons) */ -diff --git a/netwerk/base/public/mozIThirdPartyUtil.idl b/netwerk/base/public/mozIThirdPartyUtil.idl -index ad41985..fd2cb38 100644 ---- a/netwerk/base/public/mozIThirdPartyUtil.idl -+++ b/netwerk/base/public/mozIThirdPartyUtil.idl -@@ -40,6 +40,7 @@ - interface nsIURI; - interface nsIDOMWindow; - interface nsIChannel; -+interface nsIDocument; - - /** - * Utility functions for determining whether a given URI, channel, or window -@@ -173,6 +174,26 @@ interface mozIThirdPartyUtil : nsISupports - * @return the base domain. - */ - AUTF8String getBaseDomain(in nsIURI aHostURI); -+ -+ -+ /** -+ * getFirstPartyURI -+ * -+ * Obtain the top-level url bar URI for either a channel or a document. -+ * Either parameter may be null (but not both). -+ * -+ * @param aChannel -+ * An arbitrary channel for some content element of a first party -+ * load. Can be null. -+ * -+ * @param aDoc -+ * An arbitrary third party document. Can be null. -+ * -+ * @return the first party url bar URI for the load. -+ */ -+ nsIURI getFirstPartyURI(in nsIChannel aChannel, -+ in nsIDocument aDoc); -+ - }; - - %{ C++ -diff --git a/netwerk/cookie/nsICookiePermission.idl b/netwerk/cookie/nsICookiePermission.idl -index c7f765b..d81e2e6 100644 ---- a/netwerk/cookie/nsICookiePermission.idl -+++ b/netwerk/cookie/nsICookiePermission.idl -@@ -40,6 +40,7 @@ - interface nsICookie2; - interface nsIURI; - interface nsIChannel; -+interface nsIDocument; - - typedef long nsCookieAccess; - -diff --git a/toolkit/system/gnome/nsAlertsIconListener.cpp b/toolkit/system/gnome/nsAlertsIconListener.cpp -index 250e4fb..614e70f 100644 ---- a/toolkit/system/gnome/nsAlertsIconListener.cpp -+++ b/toolkit/system/gnome/nsAlertsIconListener.cpp -@@ -271,7 +271,8 @@ nsAlertsIconListener::StartRequest(const nsAString & aImageUrl) - if (!il) - return ShowAlert(NULL); - -- return il->LoadImage(imageUri, nsnull, nsnull, nsnull, nsnull, this, -+ // XXX: Hrmm.... Bypass cache, or isolate to imageUrl? -+ return il->LoadImage(imageUri, imageUri, nsnull, nsnull, nsnull, this, - nsnull, nsIRequest::LOAD_NORMAL, nsnull, nsnull, - nsnull, getter_AddRefs(mIconRequest)); - } -diff --git a/widget/cocoa/nsMenuItemIconX.mm b/widget/cocoa/nsMenuItemIconX.mm -index 9505ea2..bc9b856 100644 ---- a/widget/cocoa/nsMenuItemIconX.mm -+++ b/widget/cocoa/nsMenuItemIconX.mm -@@ -63,6 +63,7 @@ - #include "gfxImageSurface.h" - #include "imgIContainer.h" - #include "nsCocoaUtils.h" -+#include "mozIThirdPartyUtil.h" - - static const PRUint32 kIconWidth = 16; - static const PRUint32 kIconHeight = 16; -@@ -338,9 +339,15 @@ nsMenuItemIconX::LoadIcon(nsIURI* aIconURI) - [mNativeMenuItem setImage:sPlaceholderIconImage]; - } - -+ nsCOMPtr<nsIURI> firstPartyURI; -+ nsCOMPtr<mozIThirdPartyUtil> thirdPartySvc -+ = do_GetService(THIRDPARTYUTIL_CONTRACTID); -+ thirdPartySvc->GetFirstPartyURI(nsnull, document, -+ getter_AddRefs(firstPartyURI)); -+ - // Passing in null for channelPolicy here since nsMenuItemIconX::LoadIcon is - // not exposed to web content -- rv = loader->LoadImage(aIconURI, nsnull, nsnull, nsnull, loadGroup, this, -+ rv = loader->LoadImage(aIconURI, firstPartyURI, nsnull, nsnull, loadGroup, this, - nsnull, nsIRequest::LOAD_NORMAL, nsnull, nsnull, - nsnull, getter_AddRefs(mIconRequest)); - if (NS_FAILED(rv)) return rv; --- -1.7.5.4 -

1 0

[stem/master] Updated some of Damian's suggested revisions. Not sure of others.
by atagar＠torproject.org 04 Aug '12

04 Aug '12

commit 305f7fdb30f420a751f2de29f250170c19569bb6 Author: Erik <eislo(a)wesleyan.edu> Date: Thu Jul 12 15:24:50 2012 -0400 Updated some of Damian's suggested revisions. Not sure of others. --- stem/descriptor/export.py | 36 ++++++++++++++++++------------------ 1 files changed, 18 insertions(+), 18 deletions(-) diff --git a/stem/descriptor/export.py b/stem/descriptor/export.py index 2859fee..c847b81 100644 --- a/stem/descriptor/export.py +++ b/stem/descriptor/export.py @@ -1,19 +1,19 @@ import os, csv, sets, cStringIO -def descriptor_csv_exp(descr, incl_fields=[], excl_fields=[]): +def get_csv_line(descriptor, include_fields=[], exclude_fields=[]): """ Takes a single descriptor object, puts it in a list, and passes it to descriptors_csv_exp to build a csv. :param object descr: single descriptor object to export as csv. """ - descriptor = [descr] - for desc in descriptors_csv_exp(descriptor, incl_fields, excl_fields): + descr = [descriptor] + for desc in get_csv_lines(descr, include_fields, exclude_fields): return desc -def descriptors_csv_exp(descrs, incl_fields=[], excl_fields=[], head=False): +def get_csv_lines(descriptors, include_fields=[], exclude_fields=[], head=False): """ Builds a csv file based on attributes of descriptors. @@ -25,12 +25,12 @@ def descriptors_csv_exp(descrs, incl_fields=[], excl_fields=[], head=False): :returns: generator for csv strings, one line per descr object. """ - + _temp_file = cStringIO.StringIO() first = True - for desc in descrs: + for desc in descriptors: attr = vars(desc) # Defining incl_fields and the dwriter object requires having access @@ -38,18 +38,18 @@ def descriptors_csv_exp(descrs, incl_fields=[], excl_fields=[], head=False): if first: # define incl_fields, 4 cases where final case is incl_fields already # defined and excl_fields left blank, so no action is necessary. - if not incl_fields and excl_fields: + if not include_fields and exclude_fields: _incl = sets.Set(attr.keys()) - incl_fields = list(_incl.difference(excl_fields)) + include_fields = list(_incl.difference(exclude_fields)) - elif not incl_fields and not excl_fields: - incl_fields = attr.keys() + elif not include_fields and not exclude_fields: + include_fields = attr.keys() - elif incl_fields and excl_fields: - _incl = sets.Set(incl_fields) - incl_fields = list(_incl.difference(excl_fields)) + elif include_fields and exclude_fields: + _incl = sets.Set(include_fields) + include_fields = list(_incl.difference(exclude_fields)) - dwriter = csv.DictWriter(_temp_file, incl_fields) + dwriter = csv.DictWriter(_temp_file, include_fields) first = False if head: @@ -58,7 +58,7 @@ def descriptors_csv_exp(descrs, incl_fields=[], excl_fields=[], head=False): # Need to remove fields that aren't wanted for dwriter. final = {} for at in attr: - if at in incl_fields: + if at in include_fields: final[at] = attr[at] dwriter.writerow(final) @@ -71,7 +71,7 @@ def descriptors_csv_exp(descrs, incl_fields=[], excl_fields=[], head=False): _temp_file.close() -def csv_file_exp(descrs, doc_loc, header=True, incl_f=[], excl_f=[]): +def csv_file_exp(descriptors, document_location, header=True, include_fields=[], exclude_fields=[]): """ Writes descriptor attributes to a csv file on disk. @@ -82,8 +82,8 @@ def csv_file_exp(descrs, doc_loc, header=True, incl_f=[], excl_f=[]): :param list incl_f: list of attribute fields to include in the csv line. :param list excl_f: list of attribute fields to exclude from csv line. """ - doc = open(doc_loc, 'w') + doc = open(document_location, 'w') - for line in descriptors_csv_exp(descrs, incl_fields=incl_f, excl_fields=excl_f, head=header): + for line in get_csv_lines(descriptors, include_fields=include_fields, exclude_fields=exclude_fields, head=header): doc.write(line)

1 0

[stem/master] Early unit test builds with lots of holes to fill.
by atagar＠torproject.org 04 Aug '12

04 Aug '12

commit 4ea5cf2e2d8f7bb27db96d35d8fc0538b2745e70 Author: Erik <eislo(a)wesleyan.edu> Date: Tue Jul 17 17:46:16 2012 -0400 Early unit test builds with lots of holes to fill. Problems encountered include mocking functions that utilize keyword args, passing descriptor objects to functions being tested rather than just dictionaries, and determining if, in the case of the user defining both include_fields and exclude_fields, an error should be thrown or the functions should simply remove any overlap from the two lists. --- run_tests.py | 4 ++ stem/descriptor/export.py | 93 ++++++++++++++++++++-------------------- test/mocking.py | 3 +- test/unit/descriptor/export.py | 52 ++++++++++++++++++++++ 4 files changed, 105 insertions(+), 47 deletions(-) diff --git a/run_tests.py b/run_tests.py index 8a9a37d..bed8e50 100755 --- a/run_tests.py +++ b/run_tests.py @@ -50,6 +50,8 @@ import test.integ.util.proc import test.integ.util.system import test.integ.process import test.integ.version +#TODO move this by other descriptor tests +import test.unit.descriptor.export import stem.prereq import stem.util.conf @@ -121,7 +123,9 @@ UNIT_TESTS = ( test.unit.response.protocolinfo.TestProtocolInfoResponse, test.unit.response.authchallenge.TestAuthChallengeResponse, test.unit.connection.authentication.TestAuthenticate, + test.unit.descriptor.export.TestExport, ) +# TODO move export test to proper location INTEG_TESTS = ( test.integ.util.conf.TestConf, diff --git a/stem/descriptor/export.py b/stem/descriptor/export.py index c847b81..dd7f324 100644 --- a/stem/descriptor/export.py +++ b/stem/descriptor/export.py @@ -1,80 +1,82 @@ import os, csv, sets, cStringIO -def get_csv_line(descriptor, include_fields=[], exclude_fields=[]): +def export_csv(descriptor, include_fields=[], exclude_fields=[]): """ Takes a single descriptor object, puts it in a list, and passes it to descriptors_csv_exp to build a csv. - - :param object descr: single descriptor object to export as csv. + + :param object descriptor: single descriptor whose. + :param list include_fields: list of attribute fields to include in the csv string. + :param list exclude_fields: list of attribute fields to exclude from csv string. + + :returns: single csv line as a string with one descriptor attribute per cell. """ descr = [descriptor] - for desc in get_csv_lines(descr, include_fields, exclude_fields): - return desc + return export_csvs(descr, include_fields=include_fields, exclude_fields=exclude_fields) -def get_csv_lines(descriptors, include_fields=[], exclude_fields=[], head=False): +def export_csvs(descriptors, include_fields=[], exclude_fields=[], header=False): """ - Builds a csv file based on attributes of descriptors. - - :param list descrs: List of descriptor objects to export as a csv line. - :param list incl_fields: list of attribute fields to include in the csv line. - :param list excl_fields: list of attribute fields to exclude from csv line. - :param bool head: whether or not a header is requested; shouldn't be needed - outside of csv_file_exp's call of this function. - - :returns: generator for csv strings, one line per descr object. + Returns a string that is in csv format, ready to be placed in a .csv file. + + :param list descrs: List of descriptor objects whose attributes will be written. + :param list include_fields: list of attribute fields to include in the csv string. + :param list exclude_fields: list of attribute fields to exclude from csv string. + :param bool header: whether or not a header is requested; probably won't be + needed outside of csv_file_exp's call of this function. + + :returns: csv string with one descriptor per line and one attribute per cell. """ - + _temp_file = cStringIO.StringIO() - + first = True - + for desc in descriptors: attr = vars(desc) - + # Defining incl_fields and the dwriter object requires having access # to a descriptor object. if first: + # All descriptor objects should be of the same type + # (i.e. server_descriptor.RelayDescriptor) + desc_type = type(desc) + # define incl_fields, 4 cases where final case is incl_fields already # defined and excl_fields left blank, so no action is necessary. if not include_fields and exclude_fields: _incl = sets.Set(attr.keys()) include_fields = list(_incl.difference(exclude_fields)) - + elif not include_fields and not exclude_fields: include_fields = attr.keys() - + elif include_fields and exclude_fields: _incl = sets.Set(include_fields) include_fields = list(_incl.difference(exclude_fields)) - - dwriter = csv.DictWriter(_temp_file, include_fields) + + dwriter = csv.DictWriter(_temp_file, include_fields, extrasaction='ignore') first = False - - if head: + + if header: dwriter.writeheader() - - # Need to remove fields that aren't wanted for dwriter. - final = {} - for at in attr: - if at in include_fields: - final[at] = attr[at] - - dwriter.writerow(final) - yield _temp_file.getvalue() - # Clear cString wrapper for new descriptor. - _temp_file.reset() - _temp_file.truncate() - - - _temp_file.close() - -def csv_file_exp(descriptors, document_location, header=True, include_fields=[], exclude_fields=[]): + if desc_type == type(desc): + dwriter.writerow(attr) + else: + raise ValueError('More than one type of descriptor was provided.') + + return _temp_file.getvalue() + # cStringIO files are closed automatically when the current scope is exited. + +def export_csv_file(descriptors, document_location, header=True, include_fields=[], exclude_fields=[]): """ Writes descriptor attributes to a csv file on disk. - + + Calls get_csv_lines with the given argument, then writes the returned string + to a file location specified by document_location. + :param list descrs: descriptor objects with attributes to export as csv file. :param str doc_loc: location and file name for csv file to be written to. This overwrites existing files. @@ -83,7 +85,6 @@ def csv_file_exp(descriptors, document_location, header=True, include_fields=[], :param list excl_f: list of attribute fields to exclude from csv line. """ doc = open(document_location, 'w') - - for line in get_csv_lines(descriptors, include_fields=include_fields, exclude_fields=exclude_fields, head=header): + + for line in export_csvs(descriptors, include_fields=include_fields, exclude_fields=exclude_fields, head=header): doc.write(line) - diff --git a/test/mocking.py b/test/mocking.py index f4198ec..198bf70 100644 --- a/test/mocking.py +++ b/test/mocking.py @@ -50,7 +50,8 @@ def no_op(): return _no_op def return_value(value): - def _return_value(*args): return value + def _return_value(*args, **kwargs): + return value return _return_value def return_true(): return return_value(True) diff --git a/test/unit/descriptor/export.py b/test/unit/descriptor/export.py new file mode 100644 index 0000000..b1b78e3 --- /dev/null +++ b/test/unit/descriptor/export.py @@ -0,0 +1,52 @@ +""" +Unit testing code for the stem.descriptor.export module +""" +import unittest +import stem.descriptor.export as export +import test.mocking as mocking + +SINGLE_DESCR_DICT = {'average_bandwidth': 5242880, 'onion_key': 'RSA PUB = JAIK', 'address': '79.139.135.90', '_digest': None, 'exit_policy': ['reject *:*'], 'fingerprint': '0045EB8B820DC410197B'} + + + +class TestExport(unittest.TestCase): + def tearDown(self): + mocking.revert_mocking() + + def test_export_csv(self): + """ + Tests the export_csv function which takes a single descriptor object. + """ + + # TODO we should be passing descriptor objects not just dicts. + csv_string = '5242880, RSA PUB = JAIK, 79.139.135.90,,[\'reject *:*\'], 0045EB8B820DC410197B' + mocking.mock(export.export_csvs, mocking.return_value(csv_string)) + self.assertEqual(csv_string, export.export_csv(SINGLE_DESCR_DICT)) + + csv_string = '79.139.135.90,,[\'reject *:*\'], 0045EB8B820DC410197B' + mocking.mock(export.export_csvs, mocking.return_value(csv_string)) + self.assertEqual(csv_string, export.export_csv(SINGLE_DESCR_DICT, exclude_fields=['average_bandwidth', 'onion_key'])) + + csv_string = 'RSA PUB = JAIK, 79.139.135.90,' + mocking.mock(export.export_csvs, mocking.return_value(csv_string)) + self.assertEqual(csv_string, export.export_csv(SINGLE_DESCR_DICT, include_fields=['onion_key', 'address'])) + + # TODO 1 or two more cases to handle (subcases of overlap/no overlap + # incl & excl.) + + + # TODO Make sure to undo mocking here or we won't be testing the next function. + + def test_export_csvs(self): + """ + Test the export_csvs function which takes a list of descriptor objects. + """ + pass + + def test_export_csv_file(self): + """ + Tests the export_csv_file function. + """ + pass + # mocking.mock(open, mocking.return_for_args(##)) + # mocking.mock(export.export_csvs, ##)

1 0

[stem/master] First complete build of unit tests for torexport.
by atagar＠torproject.org 04 Aug '12

04 Aug '12

commit 9440f4ffa20aa7ba2a0e3297cbf5b245bf2c1c91 Author: Erik <eislo(a)wesleyan.edu> Date: Mon Jul 30 18:09:13 2012 -0400 First complete build of unit tests for torexport. There is a bug in test_export_csv_file which will be fixed tomorrow morning, otherwise all tests are functional and should be passing. --- stem/descriptor/export.py | 53 ++++++++------- test/mocking.py | 35 ++++++---- test/unit/descriptor/export.py | 147 ++++++++++++++++++++++++++++++++++------ 3 files changed, 176 insertions(+), 59 deletions(-) diff --git a/stem/descriptor/export.py b/stem/descriptor/export.py index dd7f324..af62be5 100644 --- a/stem/descriptor/export.py +++ b/stem/descriptor/export.py @@ -1,7 +1,7 @@ import os, csv, sets, cStringIO -def export_csv(descriptor, include_fields=[], exclude_fields=[]): +def export_csv(descriptor, include_fields=(), exclude_fields=()): """ Takes a single descriptor object, puts it in a list, and passes it to descriptors_csv_exp to build a csv. @@ -12,13 +12,15 @@ def export_csv(descriptor, include_fields=[], exclude_fields=[]): :returns: single csv line as a string with one descriptor attribute per cell. """ - descr = [descriptor] + + descr = (descriptor,) return export_csvs(descr, include_fields=include_fields, exclude_fields=exclude_fields) def export_csvs(descriptors, include_fields=[], exclude_fields=[], header=False): """ - Returns a string that is in csv format, ready to be placed in a .csv file. + Takes an iterable of descriptors, returns a string with one line per descriptor + where each line is a comma separated list of descriptor attributes. :param list descrs: List of descriptor objects whose attributes will be written. :param list include_fields: list of attribute fields to include in the csv string. @@ -27,64 +29,67 @@ def export_csvs(descriptors, include_fields=[], exclude_fields=[], header=False) needed outside of csv_file_exp's call of this function. :returns: csv string with one descriptor per line and one attribute per cell. + :raises: ValueError if more than one descriptor type (e.g. server_descriptor, + extrainfo_descriptor) is provided in the iterable. """ - _temp_file = cStringIO.StringIO() + # Need a file object to write to with DictWriter. + temp_file = cStringIO.StringIO() first = True for desc in descriptors: + import sys attr = vars(desc) # Defining incl_fields and the dwriter object requires having access # to a descriptor object. if first: # All descriptor objects should be of the same type - # (i.e. server_descriptor.RelayDescriptor) + # (i.e. server_descriptor.RelayDesrciptor) desc_type = type(desc) # define incl_fields, 4 cases where final case is incl_fields already # defined and excl_fields left blank, so no action is necessary. if not include_fields and exclude_fields: - _incl = sets.Set(attr.keys()) - include_fields = list(_incl.difference(exclude_fields)) + incl = set(attr.keys()) + include_fields = list(incl.difference(exclude_fields)) elif not include_fields and not exclude_fields: include_fields = attr.keys() elif include_fields and exclude_fields: - _incl = sets.Set(include_fields) - include_fields = list(_incl.difference(exclude_fields)) + incl = set(include_fields) + include_fields = list(incl.difference(exclude_fields)) - dwriter = csv.DictWriter(_temp_file, include_fields, extrasaction='ignore') - first = False + dwriter = csv.DictWriter(temp_file, include_fields, extrasaction='ignore') if header: dwriter.writeheader() + first = False if desc_type == type(desc): dwriter.writerow(attr) else: - raise ValueError('More than one type of descriptor was provided.') + raise ValueError('More than one descriptor type provided. Started with a %s, and just got a %s' % (desc_type, type(desc))) - return _temp_file.getvalue() + return temp_file.getvalue() # cStringIO files are closed automatically when the current scope is exited. -def export_csv_file(descriptors, document_location, header=True, include_fields=[], exclude_fields=[]): +def export_csv_file(descriptors, document, include_fields=[], exclude_fields=[], header=True): """ Writes descriptor attributes to a csv file on disk. Calls get_csv_lines with the given argument, then writes the returned string to a file location specified by document_location. - :param list descrs: descriptor objects with attributes to export as csv file. - :param str doc_loc: location and file name for csv file to be written to. - This overwrites existing files. - :param bool header: defaults to true, determines if doc will have a header row. - :param list incl_f: list of attribute fields to include in the csv line. - :param list excl_f: list of attribute fields to exclude from csv line. + :param list descriptors: descriptor objects with attributes to export as csv file. + :param object document: File object to be written to. + :param bool header: defaults to true, determines if document will have a header row. + :param list include_fields: list of attribute fields to include in the csv line. + :param list exclude_fields: list of attribute fields to exclude from csv line. """ - doc = open(document_location, 'w') - - for line in export_csvs(descriptors, include_fields=include_fields, exclude_fields=exclude_fields, head=header): - doc.write(line) + try: + document.write(export_csvs(descriptors, include_fields=include_fields, exclude_fields=exclude_fields, header=header)) + except AttributeError: + print "A valid document object was not provided; could not write" diff --git a/test/mocking.py b/test/mocking.py index 198bf70..b2d9737 100644 --- a/test/mocking.py +++ b/test/mocking.py @@ -58,27 +58,36 @@ def return_true(): return return_value(True) def return_false(): return return_value(False) def return_none(): return return_value(None) -def return_for_args(args_to_return_value, default = None): +def return_for_args(args_to_return_value, kwarg_type=None, default=None): """ Returns a value if the arguments to it match something in a given - 'argument => return value' mapping. Otherwise, a default function - is called with the arguments. + 'argument => return value' mapping. In the case of keyword arguments, + a type must be specified so _return_value can check these arguments as + well. Otherwise, a default function is called with the arguments. :param dict args_to_return_value: mapping of arguments to the value we should provide + :param object kwarg_type: type of kwarg mapping to be used in unwraping these arguments. :param functor default: returns the value of this function if the args don't match something that we have, we raise a ValueError by default """ - - def _return_value(*args): - if args in args_to_return_value: - return args_to_return_value[args] - elif default is None: - arg_label = ", ".join([str(v) for v in args]) - raise ValueError("Unrecognized argument sent for return_for_args(): %s" % arg_label) + + def _return_value(*args, **kwargs): + # First handle the case in which we aren't expecting keyword args. + if kwarg_type == None: + argument = args else: - return default(args) + argument = args + (kwarg_type(**kwargs),) + + try: + return args_to_return_value[argument] + except KeyError: + if default is None: + arg_label = ", ".join(map(str, argument)) + raise ValueError("Unrecognized argument sent for return_for_args(): %s" % arg_label) + else: + return default(args) return _return_value - + def raise_exception(exception): def _raise(*args): raise exception return _raise @@ -121,7 +130,7 @@ def mock(target, mock_call, target_module=None): target_function = target.__name__ MOCK_STATE[mocking_id] = (target_module, target_function, target) - mock_wrapper = lambda *args: mock_call(*args) + mock_wrapper = lambda *args, **kwargs: mock_call(*args, **kwargs) mock_wrapper.__dict__["mock_id"] = mocking_id # mocks the function with this wrapper diff --git a/test/unit/descriptor/export.py b/test/unit/descriptor/export.py index b1b78e3..a785957 100644 --- a/test/unit/descriptor/export.py +++ b/test/unit/descriptor/export.py @@ -5,9 +5,40 @@ import unittest import stem.descriptor.export as export import test.mocking as mocking -SINGLE_DESCR_DICT = {'average_bandwidth': 5242880, 'onion_key': 'RSA PUB = JAIK', 'address': '79.139.135.90', '_digest': None, 'exit_policy': ['reject *:*'], 'fingerprint': '0045EB8B820DC410197B'} +from collections import namedtuple +import stem.descriptor as desc +import cStringIO +# Create descriptor objects. +DESCR_DICT = {'average_bandwidth': 5242880, 'onion_key': 'RSA PUB = JAIK', 'address': '79.139.135.90', '_digest': None, 'exit_policy': ['reject *:*'], 'fingerprint': 'AAAAAAAAAAAAAAAAAAA'} +DESCR2_DICT = {'average_bandwidth': 5555555, 'onion_key': 'RSA PUB = GOUS', 'address': '100.1.1.1', '_digest': None, 'exit_policy': ['reject *:*'], 'fingerprint': 'BBBBBBBBBBBBBBBBBBBBB'} +DESCR3_DICT = {'bandwidth':12345,'average_bandwidth': 6666666, 'address': '101.0.0.1','extra_info':None} +RAW = 'router TORsinn3r 46.17.96.217 9001 0 0 platform Tor 0.2.3.19-rc on Linux bandwidth 4 5 6 ...andonandon' +descriptor = desc.Descriptor(RAW) +descriptor.__dict__.update(DESCR_DICT) + +descriptor2 = desc.Descriptor(RAW) +descriptor2.__dict__.update(DESCR2_DICT) + +descriptor3 = desc.server_descriptor.RelayDescriptor(RAW, validate=False) +descriptor3.__dict__.update(DESCR3_DICT) + +# Expected return csv strings. +SINGLE_ALL = '5242880,RSA PUB = JAIK,AAAAAAAAAAAAAAAAAAA,,router TORsinn3r 46.17.96.217 9001 0 0 platform Tor 0.2.3.19-rc on Linux bandwidth 4 5 6 ...andonandon,[\'reject *:*\'],79.139.135.90,' +SINGLE_PART = '79.139.135.90,[\'reject *:*\']' +SINGLE_PART2 = '5242880,,router TORsinn3r 46.17.96.217 9001 0 0 platform Tor 0.2.3.19-rc on Linux bandwidth 4 5 6 ...andonandon,[\'reject *:*\'],79.139.135.90,' +SINGLE_PART3 = '79.139.135.90,AAAAAAAAAAAAAAAAAAA' + +DOUBLE_ALL = '5242880,RSA PUB = JAIK,AAAAAAAAAAAAAAAAAAA,,router TORsinn3r 46.17.96.217 9001 0 0 platform Tor 0.2.3.19-rc on Linux bandwidth 4 5 6 ...andonandon,[\'reject *:*\'],79.139.135.90,\r\n5555555,RSA PUB = GOUS,BBBBBBBBBBBBBBBBBBBBB,,router TORsinn3r 46.17.96.217 9001 0 0 platform Tor 0.2.3.19-rc on Linux bandwidth 4 5 6 ...andonandon,[\'reject *:*\'],100.1.1.1,\r\n' +DOUBLE_PART = '79.139.135.90,[\'reject *:*\']\r\n100.1.1.1,[\'reject *:*\']\r\n' +DOUBLE_PART2 = '5242880,,router TORsinn3r 46.17.96.217 9001 0 0 platform Tor 0.2.3.19-rc on Linux bandwidth 4 5 6 ...andonandon,[\'reject *:*\'],79.139.135.90,\r\n5555555,,router TORsinn3r 46.17.96.217 9001 0 0 platform Tor 0.2.3.19-rc on Linux bandwidth 4 5 6 ...andonandon,[\'reject *:*\'],100.1.1.1,\r\n' + +SINGLE_ALL_HEAD = 'average_bandwidth,onion_key,fingerprint,_digest,_raw_contents,exit_policy,address,_path\r\n' + SINGLE_ALL + '\r\n' +SINGLE_PART3_HEAD = 'address,fingerprint\r\n' + SINGLE_PART3 +DOUBLE_ALL_HEAD = 'average_bandwidth,onion_key,fingerprint,_digest,_raw_contents,exit_policy,address,_path\r\n' + DOUBLE_ALL +DOUBLE_PART_HEAD = 'address,exit_policy\r\n' + DOUBLE_PART +DOUBLE_PART2_HEAD = 'average_bandwidth,_digest,_raw_contents,exit_policy,address,_path\r\n' + DOUBLE_PART2 class TestExport(unittest.TestCase): def tearDown(self): @@ -17,36 +48,108 @@ class TestExport(unittest.TestCase): """ Tests the export_csv function which takes a single descriptor object. """ + Fields = namedtuple('Fields', 'include_fields exclude_fields') - # TODO we should be passing descriptor objects not just dicts. - csv_string = '5242880, RSA PUB = JAIK, 79.139.135.90,,[\'reject *:*\'], 0045EB8B820DC410197B' - mocking.mock(export.export_csvs, mocking.return_value(csv_string)) - self.assertEqual(csv_string, export.export_csv(SINGLE_DESCR_DICT)) + # Descriptors must be an iterable + # named tuples replace dictionaries as dict keys must immutable. + ret_vals = {((descriptor,), Fields(include_fields=(), exclude_fields=())):SINGLE_ALL, + ((descriptor,), Fields(include_fields=('address', 'exit_policy'), + exclude_fields=())):SINGLE_PART, + ((descriptor,), Fields(include_fields=(), + exclude_fields=('onion_key', 'fingerprint'))):SINGLE_PART2, + ((descriptor,), Fields(include_fields=('address', 'exit_policy', 'fingerprint'), + exclude_fields=('fingerprint',))):SINGLE_PART, + ((descriptor,), Fields(include_fields=('address', 'fingerprint'), + exclude_fields=('_digest',))):SINGLE_PART3 + } + mocking.mock(export.export_csvs, mocking.return_for_args(ret_vals, kwarg_type=Fields)) - csv_string = '79.139.135.90,,[\'reject *:*\'], 0045EB8B820DC410197B' - mocking.mock(export.export_csvs, mocking.return_value(csv_string)) - self.assertEqual(csv_string, export.export_csv(SINGLE_DESCR_DICT, exclude_fields=['average_bandwidth', 'onion_key'])) + # Used tuples for incl/exclude_fields for parameter matching with ret_vals dict. + self.assertEqual(SINGLE_ALL, export.export_csv(descriptor)) + self.assertEqual(SINGLE_PART, export.export_csv(descriptor, + include_fields=('address', 'exit_policy'))) + self.assertEqual(SINGLE_PART2, export.export_csv(descriptor, + exclude_fields=('onion_key', 'fingerprint'))) + self.assertEqual(SINGLE_PART, export.export_csv(descriptor, + include_fields=('address', 'exit_policy', 'fingerprint'), exclude_fields=('fingerprint',))) + self.assertEqual(SINGLE_PART3, export.export_csv(descriptor, + include_fields=('address', 'fingerprint'), exclude_fields=('_digest',))) + + + def test_export_csvs(self): + """ + Test the export_csvs function which takes a list of descriptor objects. + """ - csv_string = 'RSA PUB = JAIK, 79.139.135.90,' - mocking.mock(export.export_csvs, mocking.return_value(csv_string)) - self.assertEqual(csv_string, export.export_csv(SINGLE_DESCR_DICT, include_fields=['onion_key', 'address'])) + # Single descriptor + #print "Type descriptor: %s" % type(descriptor) + #print "Type SINGLE_ALL: %s" % type(SINGLE_ALL) + #print SINGLE_ALL + #print "Type fn Call: %s" % type(export.export_csvs([descriptor])) + self.assertEquals(SINGLE_ALL + "\r\n", export.export_csvs([descriptor])) + self.assertEqual(SINGLE_PART + "\r\n", export.export_csvs([descriptor], + include_fields=['address', 'exit_policy'])) + self.assertEqual(SINGLE_PART2 + "\r\n", export.export_csvs([descriptor], + exclude_fields=['onion_key', 'fingerprint'])) + self.assertEqual(SINGLE_PART + "\r\n", export.export_csvs([descriptor], + include_fields=['address', 'exit_policy', 'fingerprint'], exclude_fields=['fingerprint'])) - # TODO 1 or two more cases to handle (subcases of overlap/no overlap - # incl & excl.) + # Multiple descriptors + #print "Sample Call: \n %s \n\n" % export.export_csvs([descriptor, descriptor2], header=True) + self.assertEqual(DOUBLE_ALL, export.export_csvs([descriptor, descriptor2])) + self.assertEqual(DOUBLE_PART, export.export_csvs([descriptor, descriptor2], + include_fields=['address', 'exit_policy'])) + #print export.export_csvs([descriptor, descriptor2], exclude_fields=['onion_key', 'fingerprint']) + #print "\n %s" % descriptor2.__dict__ - # TODO Make sure to undo mocking here or we won't be testing the next function. + self.assertEqual(DOUBLE_PART2, export.export_csvs([descriptor, descriptor2], + exclude_fields=['onion_key', 'fingerprint'])) + self.assertEqual(DOUBLE_PART, export.export_csvs([descriptor, descriptor2], + include_fields=['address', 'exit_policy', 'fingerprint'], exclude_fields=['fingerprint'])) - def test_export_csvs(self): - """ - Test the export_csvs function which takes a list of descriptor objects. - """ - pass + # Tests with headers + self.assertEqual(SINGLE_ALL_HEAD, export.export_csvs([descriptor], header=True)) + self.assertEqual(SINGLE_PART3_HEAD + "\r\n", export.export_csvs([descriptor], + include_fields=['address', 'fingerprint'], exclude_fields=['_digest'], header=True)) + self.assertEqual(DOUBLE_ALL_HEAD, export.export_csvs([descriptor, descriptor2], header=True)) + self.assertEqual(DOUBLE_PART_HEAD, export.export_csvs([descriptor, descriptor2], + include_fields=['address', 'exit_policy'], header=True)) + self.assertEqual(DOUBLE_PART2_HEAD, export.export_csvs([descriptor, descriptor2], + exclude_fields=['onion_key', 'fingerprint'], header=True)) + + # Other tests + self.assertRaises(ValueError, export.export_csvs, [descriptor, descriptor3]) + self.assertRaises(ValueError, export.export_csvs, [descriptor, descriptor3], + include_fields=['onion_key', 'address', 'fingerprint'], exclude_fields=['onion_key']) + + def test_export_csv_file(self): """ Tests the export_csv_file function. """ - pass - # mocking.mock(open, mocking.return_for_args(##)) - # mocking.mock(export.export_csvs, ##) + sample_csv_string = 'This, is, a, sample, string.\r\nline, two.\r\n' + sample_csv_string2 = 'Another, sample\r\n,, second,\r\n' + sample_file = cStringIO.StringIO() + + # Must use named tuples again for ret_vals dictionary. + Fields = namedtuple('Fields', 'include_fields exclude_fields header') + + ret_vals = {(descriptor, sample_file):sample_csv_string, + (descriptor, sample_file, Fields(include_fields=('address', 'onion_key'), exclude_fields=('address',), header=False)):sample_csv_string2} + # TODO Ask Danner: mock it once then do both tests (not including assertRaises), or do separate mockings. + # the latter requires that we still include empty incl_fields and excl_fields parameters instead of + # letting them default to []. Same for header. + mocking.mock(export.export_csvs, mocking.return_for_args(ret_vals, kwarg_type=Fields)) + + export.export_csv_file(descriptor, sample_file) + self.assertEqual(sample_csv_string, sample_file.getvalue()) + + sample_file = cStringIO.StringIO + + export.export_csv_file(descriptor, sample_file, include_fields=('address', 'onion_key'), exclude_fields=('address',), header=False) + self.assertEqual(sample_csv_string2, sample_file.getvalue()) + + # Make sure error is Raised when necessary. + self.assertRaises(export.export_csv_file, (descriptor, sample_csv_string))

1 0

[stem/master] First complete build of export.py tests.
by atagar＠torproject.org 04 Aug '12

04 Aug '12

commit aafb7b429e7220c938da7a591d17ca1b2463aa09 Author: Erik <eislo(a)wesleyan.edu> Date: Tue Jul 31 12:35:09 2012 -0400 First complete build of export.py tests. Everything should pass, and testing export_csv_file() promted some changes, which include the change from specifying a file for writing to providing a document object. This code is now ready for review. --- stem/descriptor/export.py | 9 +++++---- test/mocking.py | 2 +- test/unit/descriptor/export.py | 14 +++++++------- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/stem/descriptor/export.py b/stem/descriptor/export.py index af62be5..1f94734 100644 --- a/stem/descriptor/export.py +++ b/stem/descriptor/export.py @@ -76,7 +76,7 @@ def export_csvs(descriptors, include_fields=[], exclude_fields=[], header=False) return temp_file.getvalue() # cStringIO files are closed automatically when the current scope is exited. -def export_csv_file(descriptors, document, include_fields=[], exclude_fields=[], header=True): +def export_csv_file(descriptors, document, include_fields=(), exclude_fields=(), header=True): """ Writes descriptor attributes to a csv file on disk. @@ -89,7 +89,8 @@ def export_csv_file(descriptors, document, include_fields=[], exclude_fields=[], :param list include_fields: list of attribute fields to include in the csv line. :param list exclude_fields: list of attribute fields to exclude from csv line. """ - try: + if not hasattr(document, 'write'): + raise AttributeError("Provided %r object does not have a write method." % document) + else: document.write(export_csvs(descriptors, include_fields=include_fields, exclude_fields=exclude_fields, header=header)) - except AttributeError: - print "A valid document object was not provided; could not write" + diff --git a/test/mocking.py b/test/mocking.py index b2d9737..071514d 100644 --- a/test/mocking.py +++ b/test/mocking.py @@ -69,7 +69,7 @@ def return_for_args(args_to_return_value, kwarg_type=None, default=None): :param object kwarg_type: type of kwarg mapping to be used in unwraping these arguments. :param functor default: returns the value of this function if the args don't match something that we have, we raise a ValueError by default """ - + def _return_value(*args, **kwargs): # First handle the case in which we aren't expecting keyword args. if kwarg_type == None: diff --git a/test/unit/descriptor/export.py b/test/unit/descriptor/export.py index a785957..d006e85 100644 --- a/test/unit/descriptor/export.py +++ b/test/unit/descriptor/export.py @@ -136,20 +136,20 @@ class TestExport(unittest.TestCase): # Must use named tuples again for ret_vals dictionary. Fields = namedtuple('Fields', 'include_fields exclude_fields header') - ret_vals = {(descriptor, sample_file):sample_csv_string, - (descriptor, sample_file, Fields(include_fields=('address', 'onion_key'), exclude_fields=('address',), header=False)):sample_csv_string2} + ret_vals = {((descriptor,), Fields(include_fields=(), exclude_fields=(), header=True)):sample_csv_string, + ((descriptor,), Fields(include_fields=('address', 'onion_key'), exclude_fields=('address',), header=False)):sample_csv_string2} # TODO Ask Danner: mock it once then do both tests (not including assertRaises), or do separate mockings. # the latter requires that we still include empty incl_fields and excl_fields parameters instead of # letting them default to []. Same for header. mocking.mock(export.export_csvs, mocking.return_for_args(ret_vals, kwarg_type=Fields)) - export.export_csv_file(descriptor, sample_file) + export.export_csv_file((descriptor,), sample_file) self.assertEqual(sample_csv_string, sample_file.getvalue()) - sample_file = cStringIO.StringIO + sample_file = cStringIO.StringIO() - export.export_csv_file(descriptor, sample_file, include_fields=('address', 'onion_key'), exclude_fields=('address',), header=False) + export.export_csv_file((descriptor,), sample_file, include_fields=('address', 'onion_key'), exclude_fields=('address',), header=False) self.assertEqual(sample_csv_string2, sample_file.getvalue()) - + # Make sure error is Raised when necessary. - self.assertRaises(export.export_csv_file, (descriptor, sample_csv_string)) + self.assertRaises(AttributeError, export.export_csv_file, (descriptor,), sample_csv_string)

1 0

[stem/master] First draft of torexport, 3 functions defined to handle different use cases.
by atagar＠torproject.org 04 Aug '12

04 Aug '12

commit 508373f1a7824b1ff9b86a597a718918d4250e24 Author: Erik <eislo(a)wesleyan.edu> Date: Thu Jul 12 14:13:59 2012 -0400 First draft of torexport, 3 functions defined to handle different use cases. First draft of torexport, 3 functions defined to handle different use cases. csv_file_exp() takes a list of descriptors and exports their attributes as a csv file. descriptors_csv_exp() takes a list of descriptors and returns a generator. Each iteration, this function returns a line of csv representing the attributes of a single descriptor object. descriptor_csv_exp() returns a single line of csv based on the attributes of a sole descriptor object. --- stem/descriptor/export.py | 89 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 89 insertions(+), 0 deletions(-) diff --git a/stem/descriptor/export.py b/stem/descriptor/export.py new file mode 100644 index 0000000..2859fee --- /dev/null +++ b/stem/descriptor/export.py @@ -0,0 +1,89 @@ +import os, csv, sets, cStringIO + + +def descriptor_csv_exp(descr, incl_fields=[], excl_fields=[]): + """ + Takes a single descriptor object, puts it in a list, and passes it to + descriptors_csv_exp to build a csv. + + :param object descr: single descriptor object to export as csv. + """ + descriptor = [descr] + for desc in descriptors_csv_exp(descriptor, incl_fields, excl_fields): + return desc + + +def descriptors_csv_exp(descrs, incl_fields=[], excl_fields=[], head=False): + """ + Builds a csv file based on attributes of descriptors. + + :param list descrs: List of descriptor objects to export as a csv line. + :param list incl_fields: list of attribute fields to include in the csv line. + :param list excl_fields: list of attribute fields to exclude from csv line. + :param bool head: whether or not a header is requested; shouldn't be needed + outside of csv_file_exp's call of this function. + + :returns: generator for csv strings, one line per descr object. + """ + + _temp_file = cStringIO.StringIO() + + first = True + + for desc in descrs: + attr = vars(desc) + + # Defining incl_fields and the dwriter object requires having access + # to a descriptor object. + if first: + # define incl_fields, 4 cases where final case is incl_fields already + # defined and excl_fields left blank, so no action is necessary. + if not incl_fields and excl_fields: + _incl = sets.Set(attr.keys()) + incl_fields = list(_incl.difference(excl_fields)) + + elif not incl_fields and not excl_fields: + incl_fields = attr.keys() + + elif incl_fields and excl_fields: + _incl = sets.Set(incl_fields) + incl_fields = list(_incl.difference(excl_fields)) + + dwriter = csv.DictWriter(_temp_file, incl_fields) + first = False + + if head: + dwriter.writeheader() + + # Need to remove fields that aren't wanted for dwriter. + final = {} + for at in attr: + if at in incl_fields: + final[at] = attr[at] + + dwriter.writerow(final) + yield _temp_file.getvalue() + + # Clear cString wrapper for new descriptor. + _temp_file.reset() + _temp_file.truncate() + + + _temp_file.close() + +def csv_file_exp(descrs, doc_loc, header=True, incl_f=[], excl_f=[]): + """ + Writes descriptor attributes to a csv file on disk. + + :param list descrs: descriptor objects with attributes to export as csv file. + :param str doc_loc: location and file name for csv file to be written to. + This overwrites existing files. + :param bool header: defaults to true, determines if doc will have a header row. + :param list incl_f: list of attribute fields to include in the csv line. + :param list excl_f: list of attribute fields to exclude from csv line. + """ + doc = open(doc_loc, 'w') + + for line in descriptors_csv_exp(descrs, incl_fields=incl_f, excl_fields=excl_f, head=header): + doc.write(line) +

1 0