diff --git a/netwerk/protocol/http/PackagedAppService.cpp b/netwerk/protocol/http/PackagedAppService.cpp index f5bd81c3eacc..a9205370954a 100644 --- a/netwerk/protocol/http/PackagedAppService.cpp +++ b/netwerk/protocol/http/PackagedAppService.cpp @@ -585,7 +585,12 @@ PackagedAppService::PackagedAppDownloader::FinalizeDownload(nsresult aStatusCode } ClearCallbacks(aStatusCode); - mVerifier = nullptr; + // Explicity remove the downloader from the verifier. The downloader + // will die after exiting this function but the verifier may still be + // alive for a while since some resources maybe being verified. + if (mVerifier) { + mVerifier->ClearListener(); + } } nsCString @@ -634,7 +639,15 @@ PackagedAppService::PackagedAppDownloader::OnStopRequest(nsIRequest *aRequest, // Chances to get here: // 1) Very likely the package has been cached or // 2) Less likely the package is malformed. - FinalizeDownload(aStatusCode); + if (!mVerifier) { + FinalizeDownload(aStatusCode); + } else { + // We've got a broken last part and some resources might be still + // in the verification queue. Send a dummy ResourceCacheInfo to the + // verifier so this broken resource will be processed in the correct + // order. + mVerifier->SetHasBrokenLastPart(aStatusCode); + } } return NS_OK; } @@ -863,6 +876,12 @@ PackagedAppService::PackagedAppDownloader::OnVerified(bool aIsManifest, bool aIsLastPart, bool aVerificationSuccess) { + if (!aUri) { + NS_WARNING("We've got a broken last part."); + FinalizeDownload(aStatusCode); + return NS_OK; + } + RefPtr info = new ResourceCacheInfo(aUri, aCacheEntry, aStatusCode, aIsLastPart); @@ -884,6 +903,12 @@ PackagedAppService::PackagedAppDownloader::OnManifestVerified(const ResourceCach // TODO: If we disallow the request for the manifest file, do NOT callback here. CallCallbacks(aInfo->mURI, aInfo->mCacheEntry, aInfo->mStatusCode); + if (aInfo->mIsLastPart) { + NS_WARNING("This package has manifest only."); + FinalizeDownload(aInfo->mStatusCode); + return; + } + bool isPackagedSigned; mVerifier->GetIsPackageSigned(&isPackagedSigned); if (!isPackagedSigned) { diff --git a/netwerk/protocol/http/PackagedAppVerifier.cpp b/netwerk/protocol/http/PackagedAppVerifier.cpp index 5a8239a9aca1..ab94245272ba 100644 --- a/netwerk/protocol/http/PackagedAppVerifier.cpp +++ b/netwerk/protocol/http/PackagedAppVerifier.cpp @@ -48,6 +48,16 @@ PackagedAppVerifier::PackagedAppVerifier(nsIPackagedAppVerifierListener* aListen Init(aListener, aPackageOrigin, aSignature, aPackageCacheEntry); } +PackagedAppVerifier::~PackagedAppVerifier() +{ + MOZ_RELEASE_ASSERT(NS_IsMainThread(), "mPendingResourceCacheInfoList is not thread safe."); + while (auto i = mPendingResourceCacheInfoList.popFirst()) { + // This seems to be the only way that we can manually delete a + // nsISupports instance with no warning. + RefPtr deleter(i); + } +} + NS_IMETHODIMP PackagedAppVerifier::Init(nsIPackagedAppVerifierListener* aListener, const nsACString& aPackageOrigin, const nsACString& aSignature, @@ -118,15 +128,26 @@ PackagedAppVerifier::OnStopRequest(nsIRequest* aRequest, nsISupports* aContext, nsresult aStatusCode) { + MOZ_RELEASE_ASSERT(NS_IsMainThread(), "mHashingResourceURI is not thread safe."); + NS_ENSURE_TRUE(mHasher, NS_ERROR_FAILURE); - nsresult rv = mHasher->Finish(true, mLastComputedResourceHash); + nsAutoCString hash; + nsresult rv = mHasher->Finish(true, hash); NS_ENSURE_SUCCESS(rv, rv); - LOG(("Hash of %s is %s", mHashingResourceURI.get(), - mLastComputedResourceHash.get())); + LOG(("Hash of %s is %s", mHashingResourceURI.get(), hash.get())); - ProcessResourceCache(static_cast(aContext)); + // Store the computated hash associated with the resource URI. + mResourceHashStore.Put(mHashingResourceURI, new nsCString(hash)); + mHashingResourceURI = EmptyCString(); + + // Get a internal copy and take over the life cycle handling + // since the linked list we use only supports pointer-based element. + ResourceCacheInfo* info + = new ResourceCacheInfo(*(static_cast(aContext))); + + ProcessResourceCache(info); return NS_OK; } @@ -136,18 +157,27 @@ PackagedAppVerifier::ProcessResourceCache(const ResourceCacheInfo* aInfo) { MOZ_RELEASE_ASSERT(NS_IsMainThread(), "ProcessResourceCache must be on main thread"); + // Queue this info since we might process it asynchronously. + mPendingResourceCacheInfoList.insertBack(const_cast(aInfo)); + switch (mState) { case STATE_UNKNOWN: // The first resource has to be the manifest. VerifyManifest(aInfo); break; + case STATE_MANIFEST_VERIFYING: + // A resource is cached in the middle of manifest verification. + // Verify it until the manifest is verified. + break; + case STATE_MANIFEST_VERIFIED_OK: VerifyResource(aInfo); break; case STATE_MANIFEST_VERIFIED_FAILED: - OnResourceVerified(aInfo, false); + LOG(("Resource not verified because manifest verification failed.")); + FireVerifiedEvent(false, false); break; default: @@ -156,6 +186,24 @@ PackagedAppVerifier::ProcessResourceCache(const ResourceCacheInfo* aInfo) } } +void +PackagedAppVerifier::FireVerifiedEvent(bool aForManifest, bool aSuccess) +{ + nsCOMPtr r; + + if (aForManifest) { + r = NS_NewRunnableMethodWithArgs(this, + &PackagedAppVerifier::OnManifestVerified, + aSuccess); + } else { + r = NS_NewRunnableMethodWithArgs(this, + &PackagedAppVerifier::OnResourceVerified, + aSuccess); + } + + NS_DispatchToMainThread(r); +} + void PackagedAppVerifier::VerifyManifest(const ResourceCacheInfo* aInfo) { @@ -163,21 +211,29 @@ PackagedAppVerifier::VerifyManifest(const ResourceCacheInfo* aInfo) LOG(("Ready to verify manifest.")); + if (!aInfo->mURI) { // Broken last part. + FireVerifiedEvent(false, false); + mState = STATE_MANIFEST_VERIFIED_FAILED; + return; + } + + mState = STATE_MANIFEST_VERIFYING; + if (gDeveloperMode) { LOG(("Developer mode! Bypass verification.")); - OnManifestVerified(aInfo, true); + FireVerifiedEvent(true, true); return; } if (mSignature.IsEmpty()) { LOG(("No signature. No need to do verification.")); - OnManifestVerified(aInfo, true); + FireVerifiedEvent(true, true); return; } // TODO: Implement manifest verification. LOG(("Manifest verification not implemented yet. See Bug 1178518.")); - OnManifestVerified(aInfo, false); + FireVerifiedEvent(true, false); } void @@ -185,30 +241,51 @@ PackagedAppVerifier::VerifyResource(const ResourceCacheInfo* aInfo) { MOZ_RELEASE_ASSERT(NS_IsMainThread(), "Resource verification must be on main thread"); - LOG(("Checking the resource integrity. '%s'", mLastComputedResourceHash.get())); + if (!aInfo->mURI) { // Broken last part. + FireVerifiedEvent(false, false); + return; + } + + // Look up the resource hash that we computed and stored to + // mResourceHashStore before. + nsAutoCString uriAsAscii; + aInfo->mURI->GetAsciiSpec(uriAsAscii); + nsCString* resourceHash = mResourceHashStore.Get(uriAsAscii); + + if (!resourceHash) { + LOG(("Hash value for %s is not computed. ERROR!", uriAsAscii.get())); + MOZ_CRASH(); + } if (gDeveloperMode) { LOG(("Developer mode! Bypass integrity check.")); - OnResourceVerified(aInfo, true); + FireVerifiedEvent(false, true); return; } if (mSignature.IsEmpty()) { LOG(("No signature. No need to do resource integrity check.")); - OnResourceVerified(aInfo, true); + FireVerifiedEvent(false, true); return; } // TODO: Implement resource integrity check. LOG(("Resource integrity check not implemented yet. See Bug 1178518.")); - OnResourceVerified(aInfo, false); + FireVerifiedEvent(false, false); } void -PackagedAppVerifier::OnManifestVerified(const ResourceCacheInfo* aInfo, bool aSuccess) +PackagedAppVerifier::OnManifestVerified(bool aSuccess) { + MOZ_RELEASE_ASSERT(NS_IsMainThread(), "OnManifestVerified must be on main thread."); + LOG(("PackagedAppVerifier::OnManifestVerified: %d", aSuccess)); + // The listener could have been removed before we verify the resource. + if (!mListener) { + return; + } + // Only when the manifest verified and package has signature would we // regard this package is signed. mIsPackageSigned = aSuccess && !mSignature.IsEmpty(); @@ -228,30 +305,56 @@ PackagedAppVerifier::OnManifestVerified(const ResourceCacheInfo* aInfo, bool aSu } } + RefPtr info(mPendingResourceCacheInfoList.popFirst()); + MOZ_ASSERT(info); + mListener->OnVerified(true, // aIsManifest. - aInfo->mURI, - aInfo->mCacheEntry, - aInfo->mStatusCode, - aInfo->mIsLastPart, + info->mURI, + info->mCacheEntry, + info->mStatusCode, + info->mIsLastPart, aSuccess); - LOG(("PackagedAppVerifier::OnManifestVerified done")); + LOG(("Ready to verify resources that were cached during verification")); + // Verify the resources which were cached during verification accordingly. + for (auto i = mPendingResourceCacheInfoList.getFirst(); i; i = i->getNext()) { + VerifyResource(i); + } } void -PackagedAppVerifier::OnResourceVerified(const ResourceCacheInfo* aInfo, bool aSuccess) +PackagedAppVerifier::OnResourceVerified(bool aSuccess) { MOZ_RELEASE_ASSERT(NS_IsMainThread(), "PackagedAppVerifier::OnResourceVerified must be on main thread"); + // The listener could have been removed before we verify the resource. + if (!mListener) { + return; + } + + RefPtr info(mPendingResourceCacheInfoList.popFirst()); + MOZ_ASSERT(info); + mListener->OnVerified(false, // aIsManifest. - aInfo->mURI, - aInfo->mCacheEntry, - aInfo->mStatusCode, - aInfo->mIsLastPart, + info->mURI, + info->mCacheEntry, + info->mStatusCode, + info->mIsLastPart, aSuccess); } +void +PackagedAppVerifier::SetHasBrokenLastPart(nsresult aStatusCode) +{ + // Append a record with null URI as a broken last part. + + ResourceCacheInfo* info + = new ResourceCacheInfo(nullptr, nullptr, aStatusCode, true); + + mPendingResourceCacheInfoList.insertBack(info); +} + //--------------------------------------------------------------- // nsIPackagedAppVerifier. //--------------------------------------------------------------- diff --git a/netwerk/protocol/http/PackagedAppVerifier.h b/netwerk/protocol/http/PackagedAppVerifier.h index 99d64201b30c..7049dbcac35c 100644 --- a/netwerk/protocol/http/PackagedAppVerifier.h +++ b/netwerk/protocol/http/PackagedAppVerifier.h @@ -13,6 +13,7 @@ #include "nsHashKeys.h" #include "nsICryptoHash.h" #include "nsIPackagedAppVerifier.h" +#include "mozilla/LinkedList.h" namespace mozilla { namespace net { @@ -31,6 +32,11 @@ public: // The initial state. STATE_UNKNOWN, + // When we are notified to process the first resource, we will start to + // verify the manifest and go to this state no matter the package has + // signature or not. + STATE_MANIFEST_VERIFYING, + // Either the package has no signature or the manifest is verified // successfully will we be in this state. STATE_MANIFEST_VERIFIED_OK, @@ -45,6 +51,7 @@ public: // The only reason to inherit from nsISupports is it needs to be // passed as the context to PackagedAppVerifier::OnStopRequest. class ResourceCacheInfo : public nsISupports + , public mozilla::LinkedListElement { public: NS_DECL_ISUPPORTS @@ -60,6 +67,16 @@ public: { } + ResourceCacheInfo(const ResourceCacheInfo& aCopyFrom) + : mURI(aCopyFrom.mURI) + , mCacheEntry(aCopyFrom.mCacheEntry) + , mStatusCode(aCopyFrom.mStatusCode) + , mIsLastPart(aCopyFrom.mIsLastPart) + { + } + + // A ResourceCacheInfo must have a URI. If mURI is null, this + // resource is broken. nsCOMPtr mURI; nsCOMPtr mCacheEntry; nsresult mStatusCode; @@ -77,10 +94,17 @@ public: const nsACString& aSignature, nsICacheEntry* aPackageCacheEntry); + // A internal used function to let the verifier know there's a broken + // last part. + void SetHasBrokenLastPart(nsresult aStatusCode); + + // Used to explicitly clear the listener to avoid circula reference. + void ClearListener() { mListener = nullptr; } + static const char* kSignedPakOriginMetadataKey; private: - virtual ~PackagedAppVerifier() { } + virtual ~PackagedAppVerifier(); // Called when a resource is already fully written in the cache. This resource // will be processed and is guaranteed to be called back in either: @@ -100,8 +124,11 @@ private: void VerifyManifest(const ResourceCacheInfo* aInfo); void VerifyResource(const ResourceCacheInfo* aInfo); - void OnManifestVerified(const ResourceCacheInfo* aInfo, bool aSuccess); - void OnResourceVerified(const ResourceCacheInfo* aInfo, bool aSuccess); + void OnManifestVerified(bool aSuccess); + void OnResourceVerified(bool aSuccess); + + // Fire a async event to notify the verification result. + void FireVerifiedEvent(bool aForManifest, bool aSuccess); // To notify that either manifest or resource check is done. nsCOMPtr mListener; @@ -131,6 +158,12 @@ private: // The last computed hash value for a resource. It will be set on every // |EndResourceHash| call. nsCString mLastComputedResourceHash; + + // A list of pending resource that is downloaded but not verified yet. + mozilla::LinkedList mPendingResourceCacheInfoList; + + // A place to store the computed hashes of each resource. + nsClassHashtable mResourceHashStore; }; // class PackagedAppVerifier } // namespace net diff --git a/netwerk/test/unit/test_packaged_app_service.js b/netwerk/test/unit/test_packaged_app_service.js index 2e8198e8a57b..18c836c76ed1 100644 --- a/netwerk/test/unit/test_packaged_app_service.js +++ b/netwerk/test/unit/test_packaged_app_service.js @@ -30,6 +30,11 @@ // test_bad_package_404 // - tests that a request for a missing subresource doesn't hang if // if the last file in the package is missing some headers +// +// test_worse_package +// - tests that a request for a missing/existing resource doesn't +// break the service and the async verifier. +// Cu.import('resource://gre/modules/LoadContextInfo.jsm'); Cu.import("resource://testing-common/httpd.js"); @@ -124,6 +129,13 @@ function run_test() httpserver.registerPathHandler("/package", packagedAppContentHandler); httpserver.registerPathHandler("/304Package", packagedAppContentHandler); httpserver.registerPathHandler("/badPackage", packagedAppBadContentHandler); + + let worsePackageNum = 6; + for (let i = 0; i < worsePackageNum; i++) { + httpserver.registerPathHandler("/worsePackage_" + i, + packagedAppWorseContentHandler.bind(null, i)); + } + httpserver.start(-1); paservice = Cc["@mozilla.org/network/packaged-app-service;1"] @@ -152,6 +164,13 @@ function run_test() add_test(test_channel_no_loadinfo); } + add_test(test_worse_package_0); + add_test(test_worse_package_1); + add_test(test_worse_package_2); + add_test(test_worse_package_3); + add_test(test_worse_package_4); + add_test(test_worse_package_5); + // run tests run_next_test(); } @@ -329,3 +348,129 @@ function test_channel_no_loadinfo() { channel.loadInfo = null; paservice.getResource(channel, cacheListener); } + +// ---------------------------------------------------------------------------- + +// Worse package testing to ensure the async PackagedAppVerifier working good. + +function getData(aTestingData) { + var str = ""; + for (var i in aTestingData.content) { + str += "--" + aTestingData.token + "\r\n"; + for (var j in aTestingData.content[i].headers) { + str += aTestingData.content[i].headers[j] + "\r\n"; + } + str += "\r\n"; + str += aTestingData.content[i].data + "\r\n"; + } + + str += "--" + aTestingData.token + "--"; + return str; +} + +var worseTestData = [ + // 0. Only one broken resource. + { content: [ + { headers: ["Content-Type: text/javascript"], data: "module Math from '/scripts/helpers/math.js';\r\n...\r\n", type: "text/javascript" }, + ], + token : "gc0pJq0M:08jU534c0p", + }, + + // 1. Only one valid resource. + { content: [ + { headers: ["Content-Location: /index.html", "Content-Type: text/html"], data: "\r\n \r\n \r\n ...\r\n \r\n ...\r\n\r\n", type: "text/html" }, + ], + token : "gc0pJq0M:08jU534c0p", + }, + + // 2. All broken resources. + { content: [ + { headers: ["Content-Type: text/javascript"], data: "module Math from '/scripts/helpers/math.js';\r\n...\r\n", type: "text/javascript" }, + { headers: ["Content-Type: text/javascript"], data: "\r\n \r\n \r\n ...\r\n \r\n ...\r\n\r\n", type: "text/html" }, + { headers: ["Content-Type: text/javascript"], data: "export function sum(nums) { ... }\r\n...\r\n", type: "text/javascript" } + ], + token : "gc0pJq0M:08jU534c0p", + }, + + // 3. All broken resources except the first one. + { content: [ + { headers: ["Content-Location: /index.html", "Content-Type: text/html"], data: "\r\n \r\n \r\n ...\r\n \r\n ...\r\n\r\n", type: "text/html" }, + { headers: ["Content-Type: text/javascript"], data: "module Math from '/scripts/helpers/math.js';\r\n...\r\n", type: "text/javascript" }, + { headers: ["Content-Type: text/javascript"], data: "\r\n \r\n \r\n ...\r\n \r\n ...\r\n\r\n", type: "text/html" }, + { headers: ["Content-Type: text/javascript"], data: "export function sum(nums) { ... }\r\n...\r\n", type: "text/javascript" } + ], + token : "gc0pJq0M:08jU534c0p", + }, + + // 4. All broken resources except the last one. + { content: [ + { headers: ["Content-Type: text/javascript"], data: "module Math from '/scripts/helpers/math.js';\r\n...\r\n", type: "text/javascript" }, + { headers: ["Content-Type: text/javascript"], data: "\r\n \r\n \r\n ...\r\n \r\n ...\r\n\r\n", type: "text/html" }, + { headers: ["Content-Type: text/javascript"], data: "export function sum(nums) { ... }\r\n...\r\n", type: "text/javascript" }, + { headers: ["Content-Location: /index.html", "Content-Type: text/html"], data: "\r\n \r\n \r\n ...\r\n \r\n ...\r\n\r\n", type: "text/html" }, + ], + token : "gc0pJq0M:08jU534c0p", + }, + + // 5. All broken resources except the first and the last one. + { content: [ + { headers: ["Content-Location: /whatever.html", "Content-Type: text/html"], data: "\r\n \r\n \r\n ...\r\n \r\n ...\r\n\r\n", type: "text/html" }, + { headers: ["Content-Type: text/javascript"], data: "module Math from '/scripts/helpers/math.js';\r\n...\r\n", type: "text/javascript" }, + { headers: ["Content-Type: text/javascript"], data: "\r\n \r\n \r\n ...\r\n \r\n ...\r\n\r\n", type: "text/html" }, + { headers: ["Content-Type: text/javascript"], data: "export function sum(nums) { ... }\r\n...\r\n", type: "text/javascript" }, + { headers: ["Content-Location: /index.html", "Content-Type: text/html"], data: "\r\n \r\n \r\n ...\r\n \r\n ...\r\n\r\n", type: "text/html" }, + ], + token : "gc0pJq0M:08jU534c0p", + }, +]; + +function packagedAppWorseContentHandler(index, metadata, response) +{ + response.setHeader("Content-Type", 'application/package'); + var body = getData(worseTestData[index]); + response.bodyOutputStream.write(body, body.length); +} + +function test_worse_package(index, success) { + packagePath = "/worsePackage_" + index; + let url = uri + packagePath + "!//index.html"; + let channel = getChannelForURL(url); + paservice.getResource(channel, { + QueryInterface: function (iid) { + if (iid.equals(Ci.nsICacheEntryOpenCallback) || + iid.equals(Ci.nsISupports)) + return this; + throw Cr.NS_ERROR_NO_INTERFACE; + }, + onCacheEntryCheck: function() { return Ci.nsICacheEntryOpenCallback.ENTRY_WANTED; }, + onCacheEntryAvailable: function (entry, isnew, appcache, status) { + let cacheSuccess = (status === Cr.NS_OK); + equal(success, status === Cr.NS_OK, "Check status"); + run_next_test(); + } + }); +} + +function test_worse_package_0() { + test_worse_package(0, false); +} + +function test_worse_package_1() { + test_worse_package(1, true); +} + +function test_worse_package_2() { + test_worse_package(2, false); +} + +function test_worse_package_3() { + test_worse_package(3, true); +} + +function test_worse_package_4() { + test_worse_package(4, true); +} + +function test_worse_package_5() { + test_worse_package(5, true); +}