merge

3 files changed, 81 insertions, 39 deletions

diff --git a/indra/viewer_components/updater/llupdatedownloader.cpp b/indra/viewer_components/updater/llupdatedownloader.cpp
index c28ad76c77..c42112af80 100755
--- a/indra/viewer_components/updater/llupdatedownloader.cpp
+++ b/indra/viewer_components/updater/llupdatedownloader.cpp
@@ -77,7 +77,8 @@ private:
 	void run(void);
 	void startDownloading(LLURI const & uri, std::string const & hash);
 	void throwOnCurlError(CURLcode code);
-	bool validateDownload(void);
+	bool validateDownload(const std::string& filePath);
+	bool validateOrRemove(const std::string& filePath);
 
 	LOG_CLASS(LLUpdateDownloader::Implementation);
 };
@@ -295,9 +296,8 @@ void LLUpdateDownloader::Implementation::resume(void)
 			{
 				resumeDownloading(fileStatus.st_size);
 			}
-			else if(!validateDownload())
+			else if(!validateOrRemove(filePath))
 			{
-				LLFile::remove(filePath);
 				download(LLURI(mDownloadData["url"].asString()),
 						 mDownloadData["hash"].asString(),
 						 mDownloadData["update_channel"].asString(),
@@ -421,19 +421,13 @@ void LLUpdateDownloader::Implementation::run(void)
 	if(code == CURLE_OK)
 	{
 		LLFile::remove(mDownloadRecordPath);
-		if(validateDownload())
+		if(validateOrRemove(mDownloadData["path"]))
 		{
 			LL_INFOS("UpdaterService") << "download successful" << LL_ENDL;
 			mClient.downloadComplete(mDownloadData);
 		}
 		else
 		{
-			LL_INFOS("UpdaterService") << "download failed hash check" << LL_ENDL;
-			std::string filePath = mDownloadData["path"].asString();
-			if(filePath.size() != 0)
-			{
-				LLFile::remove(filePath);
-			}
 			mClient.downloadError("failed hash check");
 		}
 	}
@@ -449,7 +443,9 @@ void LLUpdateDownloader::Implementation::run(void)
 		LLFile::remove(mDownloadRecordPath);
 		if(mDownloadData.has("path"))
 		{
-			LLFile::remove(mDownloadData["path"].asString());
+			std::string filePath = mDownloadData["path"].asString();
+			LL_INFOS("UpdaterService") << "removing " << filePath << LL_ENDL;
+			LLFile::remove(filePath);
 		}
 		mClient.downloadError("curl error");
 	}
@@ -561,31 +557,49 @@ void LLUpdateDownloader::Implementation::throwOnCurlError(CURLcode code)
 	}
 }
 
+bool LLUpdateDownloader::Implementation::validateOrRemove(const std::string& filePath)
+{
+	bool valid = validateDownload(filePath);
+	if (! valid)
+	{
+		LL_INFOS("UpdaterService") << "removing " << filePath << LL_ENDL;
+		LLFile::remove(filePath);
+	}
+	return valid;
+}
 
-bool LLUpdateDownloader::Implementation::validateDownload(void)
+bool LLUpdateDownloader::Implementation::validateDownload(const std::string& filePath)
 {
-	std::string filePath = mDownloadData["path"].asString();
 	llifstream fileStream(filePath, std::ios_base::in | std::ios_base::binary);
 	if(!fileStream)
 	{
+		LL_INFOS("UpdaterService") << "can't open " << filePath << ", invalid" << LL_ENDL;
 		return false;
 	}
 
 	std::string hash = mDownloadData["hash"].asString();
-	if(hash.size() != 0)
+	if (! hash.empty())
 	{
-		LL_INFOS("UpdaterService") << "checking hash..." << LL_ENDL;
 		char digest[33];
 		LLMD5(fileStream).hex_digest(digest);
-		if(hash != digest)
+		if (hash == digest)
+		{
+			LL_INFOS("UpdaterService") << "verified hash " << hash
+									   << " for downloaded " << filePath << LL_ENDL;
+			return true;
+		}
+		else
 		{
-			LL_WARNS("UpdaterService") << "download hash mismatch; expected " << hash <<
-				" but download is " << digest << LL_ENDL;
+			LL_WARNS("UpdaterService") << "download hash mismatch for "
+									   << filePath << ": expected " << hash
+									   << " but computed " << digest << LL_ENDL;
+			return false;
 		}
-		return hash == digest;
 	}
 	else
 	{
+		LL_INFOS("UpdaterService") << "no hash specified for " << filePath
+								   << ", unverified" << LL_ENDL;
 		return true; // No hash check provided.
 	}
 }
diff --git a/indra/viewer_components/updater/llupdaterservice.cpp b/indra/viewer_components/updater/llupdaterservice.cpp
index 16950e1d62..cb3be5bbdc 100755
--- a/indra/viewer_components/updater/llupdaterservice.cpp
+++ b/indra/viewer_components/updater/llupdaterservice.cpp
@@ -296,37 +296,49 @@ bool LLUpdaterServiceImpl::checkForInstall(bool launchInstaller)
 		update_marker.close();
 
 		// Get the path to the installer file.
-		LLSD path = update_info.get("path");
-		if(update_info["current_version"].asString() != ll_get_version())
+		std::string path(update_info.get("path"));
+		std::string downloader_version(update_info["current_version"]);
+		if (downloader_version != ll_get_version())
 		{
 			// This viewer is not the same version as the one that downloaded
-			// the update.  Do not install this update.
-			if(!path.asString().empty())
+			// the update. Do not install this update.
+			LL_INFOS("UpdaterService") << "ignoring update downloaded by "
+									   << "different viewer version "
+									   << downloader_version << LL_ENDL;
+			if (! path.empty())
 			{
-				LL_INFOS("UpdaterService") << "ignoring update dowloaded by different client version" << LL_ENDL;;
-				LLFile::remove(path.asString());
+				LL_INFOS("UpdaterService") << "removing " << path << LL_ENDL;
+				LLFile::remove(path);
 				LLFile::remove(update_marker_path());
 			}
-			else
-			{
-				; // Nothing to clean up.
-			}
-			
+
 			foundInstall = false;
 		} 
-		else if(path.isDefined() && !path.asString().empty())
+		else if (path.empty())
+		{
+			LL_WARNS("UpdaterService") << "Marker file " << update_marker_path()
+									   << " 'path' entry empty, ignoring" << LL_ENDL;
+			foundInstall = false;
+		}
+		else if (! LLFile::isfile(path))
+		{
+			LL_WARNS("UpdaterService") << "Nonexistent installer " << path
+									   << ", ignoring" << LL_ENDL;
+			foundInstall = false;
+		}
+		else
 		{
 			if(launchInstaller)
 			{
 				setState(LLUpdaterService::INSTALLING);
-				
+
 				LLFile::remove(update_marker_path());
 
 				int result = ll_install_update(install_script_path(),
-											   update_info["path"].asString(),
+											   path,
 											   update_info["required"].asBoolean(),
 											   install_script_mode());	
-				
+
 				if((result == 0) && mAppExitCallback)
 				{
 					mAppExitCallback();
@@ -360,7 +372,8 @@ bool LLUpdaterServiceImpl::checkForResume()
 			LLSD download_info;
 			LLSDSerialize::fromXMLDocument(download_info, download_marker_stream);
 			download_marker_stream.close();
-			if(download_info["current_version"].asString() == ll_get_version())
+			std::string downloader_version(download_info["current_version"]);
+			if (downloader_version == ll_get_version())
 			{
 				mIsDownloading = true;
 				mNewVersion = download_info["update_version"].asString();
@@ -371,10 +384,13 @@ bool LLUpdaterServiceImpl::checkForResume()
 			else 
 			{
 				// The viewer that started this download is not the same as this viewer; ignore.
-				LL_INFOS("UpdaterService") << "ignoring partial download from different viewer version" << LL_ENDL;;
+				LL_INFOS("UpdaterService") << "ignoring partial download "
+										   << "from different viewer version "
+										   << downloader_version << LL_ENDL;
 				std::string path = download_info["path"].asString();
 				if(!path.empty())
 				{
+					LL_INFOS("UpdaterService") << "removing " << path << LL_ENDL;
 					LLFile::remove(path);
 				}
 				LLFile::remove(download_marker_path);
@@ -539,7 +555,7 @@ bool LLUpdaterServiceImpl::onMainLoop(LLSD const & event)
 		// Check for failed install.
 		if(LLFile::isfile(ll_install_failed_marker_path()))
 		{
-			LL_DEBUGS("UpdaterService") << "found marker " << ll_install_failed_marker_path() << LL_ENDL;;
+			LL_DEBUGS("UpdaterService") << "found marker " << ll_install_failed_marker_path() << LL_ENDL;
 			int requiredValue = 0; 
 			{
 				llifstream stream(ll_install_failed_marker_path());
@@ -552,12 +568,12 @@ bool LLUpdaterServiceImpl::onMainLoop(LLSD const & event)
 			// TODO: notify the user.
 			LL_WARNS("UpdaterService") << "last install attempt failed" << LL_ENDL;;
 			LLFile::remove(ll_install_failed_marker_path());
-			
+
 			LLSD event;
 			event["type"] = LLSD(LLUpdaterService::INSTALL_ERROR);
 			event["required"] = LLSD(requiredValue);
 			LLEventPumps::instance().obtain(LLUpdaterService::pumpName()).post(event);
-			
+
 			setState(LLUpdaterService::TERMINAL);
 		}
 		else
diff --git a/indra/viewer_components/updater/scripts/darwin/update_install.py b/indra/viewer_components/updater/scripts/darwin/update_install.py
index 10d507c9ef..08f4f0ebb9 100755
--- a/indra/viewer_components/updater/scripts/darwin/update_install.py
+++ b/indra/viewer_components/updater/scripts/darwin/update_install.py
@@ -199,6 +199,11 @@ def main(dmgfile, markerfile, markertext):
         # prepare for other cleanup
         with Janitor(LOGF) as janitor:
 
+            # Under some circumstances, this script seems to be invoked with a
+            # nonexistent pathname. Check for that.
+            if not os.path.isfile(dmgfile):
+                fail(dmgfile + " has been deleted")
+
             # Try to derive the name of the running viewer app bundle from our
             # own pathname. (Hopefully the old viewer won't copy this script
             # to a temp dir before running!)
@@ -376,6 +381,13 @@ def main(dmgfile, markerfile, markertext):
             log(' '.join(command))
             subprocess.check_call(command, stdout=LOGF, stderr=subprocess.STDOUT)
 
+        # If all the above succeeded, delete the .dmg file. We don't do this
+        # as a janitor.later() operation because we only want to do it if we
+        # get this far successfully. Note that this is out of the scope of the
+        # Janitor: we must detach the .dmg before removing it!
+        log("rm " + dmgfile)
+        os.remove(dmgfile)
+
     except Exception, err:
         # Because we carefully set sys.excepthook -- and even modify it to log
         # the problem once we have our log file open -- you might think we