MAINT-3489: Gracefully handle missing downloaded installer file.

If a marker file indicates that an installer has been downloaded and verified
and is ready to install, but that installer file does not exist, log it and
revert to checking for update.
Also add logging to every code path that can remove the installer explicitly,
in case the missing installer is actually being killed off by some weird
sequence of state-machine transitions.

2 files changed, 69 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