Merge pull request #1370 from sillsdev/robust-file-utilities

Fixed problems in GetSafeDirectories and GetDirectoryDistributedWithApplication

Author: tombogle

CHANGELOG.md

@@ -94,6 +94,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [SIL.Core.Desktop] Implemented GetDefaultProgramForFileType (as trenamed) in a way that works on Windows 11, Mono (probably) and MacOS (untested).
 - [SIL.Media] MediaInfo.HaveNecessaryComponents properly returns true if FFprobe is on the system path.
 - [SIL.Media] Made MediaInfo.FFprobeFolder look for and return the folder when first accessed, even if no prior call to the setter or other action had caused it t be found.
+- [SIL.Core] Made GetSafeDirectories not crash and simply not return any subdirectory the user does not have permission to access.
+- [SIL.Core] In GetDirectoryDistributedWithApplication, prevented a failure in accessing one of the specified subfolders from allowing it to try the others.
 
 ### Removed
 

SIL.Core/IO/DirectoryHelper.cs

@@ -1,10 +1,13 @@
 // Copyright (c) 2024 SIL Global
 // This software is licensed under the MIT License (http://opensource.org/licenses/MIT)
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Security;
 using JetBrains.Annotations;
 using SIL.PlatformUtilities;
+using SIL.Reporting;
 using static System.Environment;
 using static System.Environment.SpecialFolder;
 using static System.IO.Path;
@@ -25,7 +28,7 @@ public static void Copy(string sourcePath, string destinationPath, bool overwrit
 				File.Copy(filepath, Combine(destinationPath, filename), overwrite);
 			}
 
-			// Copy all the sub directories.
+			// Copy all the subdirectories.
 			foreach (var directoryPath in Directory.GetDirectories(sourcePath))
 			{
 				var directoryName = GetFileName(directoryPath);
@@ -106,21 +109,45 @@ public static bool IsEmpty(string path, bool onlyCheckForFiles = false)
 
 		/// <summary>
 		/// Return subdirectories of <paramref name="path"/> that are not system or hidden.
+		/// Subdirectories which the user does not have permission to access will also be skipped.
 		/// There are some cases where our call to Directory.GetDirectories() throws.
 		/// For example, when the access permissions on a folder are set so that it can't be read.
 		/// Another possible example may be Windows Backup files, which apparently look like directories.
 		/// </summary>
 		/// <param name="path">Directory path to look in.</param>
-		/// <returns>Zero or more directory names that are not system or hidden.</returns>
-		/// <exception cref="UnauthorizedAccessException">E.g. when the user does not have
-		/// read permission.</exception>
+		/// <returns>Zero or more directory names that are not system or hidden</returns>
+		/// <exception cref="UnauthorizedAccessException">The caller does not have the required
+		/// permission to access the subdirectories of <paramref name="path"/>.</exception>
+		/// <exception cref="ArgumentException">
+		/// <paramref name="path" /> is a zero-length string, contains only white space, or contains one or more invalid characters. You can query for invalid characters by using the <see cref="M:System.IO.Path.GetInvalidPathChars" /> method.</exception>
+		/// <exception cref="ArgumentNullException">
+		/// <paramref name="path" /> is <see langword="null" />.</exception>
+		/// <exception cref="PathTooLongException">The specified <paramref name="path" />, file name,
+		/// or both exceed the system-defined maximum length.</exception>
+		/// <exception cref="IOException"><paramref name="path" /> is a file name.</exception>
+		/// <exception cref="DirectoryNotFoundException">The specified <paramref name="path" /> is
+		/// invalid (for example, it is on an unmapped drive).</exception>
 		public static string[] GetSafeDirectories(string path)
 		{
-			return (from directoryName in Directory.GetDirectories(path)
-				let dirInfo = new DirectoryInfo(directoryName)
-				where (dirInfo.Attributes & FileAttributes.System) != FileAttributes.System
-				where (dirInfo.Attributes & FileAttributes.Hidden) != FileAttributes.Hidden
-				select directoryName).ToArray();
+			var list = new List<string>();
+			foreach (var directoryName in Directory.GetDirectories(path))
+			{
+				try
+				{
+					var dirInfo = new DirectoryInfo(directoryName);
+					if ((dirInfo.Attributes & FileAttributes.System) != FileAttributes.System)
+					{
+						if ((dirInfo.Attributes & FileAttributes.Hidden) != FileAttributes.Hidden)
+							list.Add(directoryName);
+					}
+				}
+				catch (SecurityException e)
+				{
+					Logger.WriteError(e);
+				}
+			}
+
+			return list.ToArray();
 		}
 
 		#region Utilities that are specific to the Windows OS.

SIL.Core/IO/FileLocationUtilities.cs

@@ -6,6 +6,7 @@
 using System.Text;
 using SIL.PlatformUtilities;
 using SIL.Reflection;
+using SIL.Reporting;
 
 namespace SIL.IO
 {
@@ -63,7 +64,7 @@ private static string LocateExecutableDistributedWithApplication(string[] partsO
 		/// [applicationFolder]/[distFileFolderName]/[subPath1]/[subPathN] or
 		/// [applicationFolder]/[distFileFolderName]/[platform]/[subPath1]/[subPathN] or
 		/// [applicationFolder]/[subPath1]/[subPathN]. If the executable can't be found we
-		/// search in the ProgramFiles folder ([programfiles]/[subPath1]/[subPathN]) on Windows,
+		/// search in the ProgramFiles folder ([ProgramFiles]/[subPath1]/[subPathN]) on Windows,
 		/// and in the folders included in the PATH environment variable on Linux.
 		/// When the executable has a prefix of ".exe" we're running on Linux we also
 		/// search for files without the prefix.
@@ -109,7 +110,7 @@ public static string LocateExecutable(params string[] partsOfTheSubPath)
 		}
 
 		private static string[] DirectoriesHoldingFiles => new[] {string.Empty, "DistFiles",
-			"common" /*for wesay*/, "src" /*for Bloom*/};
+			"common" /*for WeSay*/, "src" /*for Bloom*/};
 
 		/// <summary>
 		/// Find a file which, on a development machine, lives in [solution]/[distFileFolderName]/[subPath],
@@ -234,7 +235,7 @@ public static string GetDirectoryDistributedWithApplication(params string[] part
 		///
 		/// When subFoldersToSearch are not specified:
 		///		- If fallBackToDeepSearch is true, then the entire program files folder (and all
-		///		  (sub folders) is searched.
+		///		  sub folders) is searched.
 		///		- If fallBackToDeepSearch is false, then only the top-level program files
 		///		  folder is searched.
 		///
@@ -278,11 +279,11 @@ private static string LocateInProgramFilesFolder(string exeName, SearchOption sr
 			if (subFoldersToSearch.Length == 0)
 				subFoldersToSearch = new[] { string.Empty };
 
-			foreach (var progFolder in Enumerable.Where<string>(GetPossibleProgramFilesFolders(), Directory.Exists))
+			foreach (var progFolder in GetPossibleProgramFilesFolders().Where(Directory.Exists))
 			{
 				// calling Directory.GetFiles("C:\Program Files", exeName, SearchOption.AllDirectories) will fail
 				// if even one of the children of the Program Files doesn't allow you to search it.
-				// So instead we first gather up all the children directories, and then search those.
+				// So instead we first gather all the child directories, and then search those.
 				// Some will give us access denied, and that's fine, we skip them.
 				// But we don't want to look in child directories on Linux because GetPossibleProgramFilesFolders()
 				// gives us the individual elements of the PATH environment variable, and these specify exactly
@@ -291,11 +292,24 @@ private static string LocateInProgramFilesFolder(string exeName, SearchOption sr
 				{
 					if (Platform.IsWindows)
 					{
-						foreach (var subDir in DirectoryHelper.GetSafeDirectories(path))
+						string[] subDirectories = null;
+						try
 						{
-							var tgtPath = GetFiles(exeName, srcOption, subDir);
-							if (!string.IsNullOrEmpty(tgtPath))
-								return tgtPath;
+							subDirectories = DirectoryHelper.GetSafeDirectories(path);
+						}
+						catch (Exception e)
+						{
+							Logger.WriteError(e);
+						}
+
+						if (subDirectories != null)
+						{
+							foreach (var subDir in subDirectories)
+							{
+								var tgtPath = GetFiles(exeName, srcOption, subDir);
+								if (!string.IsNullOrEmpty(tgtPath))
+									return tgtPath;
+							}
 						}
 					}
 					else
@@ -336,14 +350,18 @@ private static IEnumerable<string> GetPossibleProgramFilesFolders()
 		{
 			if (!Platform.IsWindows)
 			{
-				foreach (var dir in Environment.GetEnvironmentVariable("PATH").Split(':'))
-					yield return dir;
+				var path = Environment.GetEnvironmentVariable("PATH");
+				if (!string.IsNullOrEmpty(path))
+				{
+					foreach (var dir in path.Split(':'))
+						yield return dir;
+				}
 				yield return "/opt"; // RAMP is installed in the /opt directory by default
 			}
 
-			var pf = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
-			yield return pf.Replace(" (x86)", string.Empty);
-			yield return pf.Replace(" (x86)", string.Empty) + " (x86)";
+			yield return Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
+			if (Environment.Is64BitProcess)
+				yield return Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86);
 		}
 	}
 }

SIL.Core/Reporting/Logger.cs

@@ -4,18 +4,20 @@
 using System.IO;
 using System.Linq;
 using System.Text;
+using JetBrains.Annotations;
 using SIL.IO;
 
 namespace SIL.Reporting
 {
-
 	public interface ILogger
 	{
 		/// <summary>
-		/// This is something that should be listed in the source control checkin
+		/// This is something that should be listed in the source control check-in
 		/// </summary>
 		void WriteConciseHistoricalEvent(string message, params object[] args);
 	}
+
+	[PublicAPI]
 	public class MultiLogger: ILogger
 	{
 		private readonly List<ILogger> _loggers= new List<ILogger>();
@@ -58,6 +60,8 @@ public void WriteConciseHistoricalEvent(string message, params object[] args)
 		{
 			_builder.Append(Logger.FormatMessage(message, args));
 		}
+
+		[PublicAPI]
 		public string GetLogText()
 		{
 			return _builder.ToString();
@@ -67,7 +71,7 @@ public string GetLogText()
 	/// ----------------------------------------------------------------------------------------
 	/// <summary>
 	/// Logs stuff to a file created in
-	/// c:\Documents and Settings\Username\Local Settings\Temp\Companyname\Productname\Log.txt
+	/// c:\Documents and Settings\Username\Local Settings\Temp\Companyname\ProductName\Log.txt
 	/// </summary>
 	/// ----------------------------------------------------------------------------------------
 	public class Logger: IDisposable, ILogger
@@ -206,7 +210,7 @@ private void RestartMinorEvents()
 		public void CheckDisposed()
 		{
 			if (IsDisposed)
-				throw new ObjectDisposedException(String.Format("'{0}' in use after being disposed.", GetType().Name));
+				throw new ObjectDisposedException($"'{GetType().Name}' in use after being disposed.");
 		}
 
 		/// <summary>
@@ -217,10 +221,7 @@ public void CheckDisposed()
 		/// <summary>
 		/// See if the object has been disposed.
 		/// </summary>
-		public bool IsDisposed
-		{
-			get { return m_isDisposed; }
-		}
+		public bool IsDisposed => m_isDisposed;
 
 		/// <summary>
 		/// Finalizer, in case client doesn't dispose it.
@@ -243,7 +244,7 @@ public void Dispose()
 		{
 			Dispose(true);
 			// This object will be cleaned up by the Dispose method.
-			// Therefore, you should call GC.SupressFinalize to
+			// Therefore, you should call GC.SuppressFinalize to
 			// take this object off the finalization queue
 			// and prevent finalization code for this object
 			// from executing a second time.
@@ -258,7 +259,7 @@ public void Dispose()
 		/// Both managed and unmanaged resources can be disposed.
 		///
 		/// 2. If disposing is false, the method has been called by the
-		/// runtime from inside the finalizer and you should not reference (access)
+		/// runtime from inside the finalizer, and you should not reference (access)
 		/// other managed objects, as they already have been garbage collected.
 		/// Only unmanaged resources can be disposed.
 		/// </summary>
@@ -295,7 +296,7 @@ protected virtual void Dispose(bool disposing)
 		#endregion IDisposable & Co. implementation
 
 		/// <summary>
-		/// This is for version-control checkin descriptions. E.g. "Deleted foobar".
+		/// This is for version-control check-in descriptions. E.g. "Deleted foobar".
 		/// </summary>
 		/// <param name="message"></param>
 		/// <param name="args"></param>
@@ -354,10 +355,7 @@ private string GetLogTextAndStartOver()
 		/// <summary>
 		/// added this for a case of a catastrophic error so bad I couldn't get the means of finding out what just happened
 		/// </summary>
-		public static string MinorEventsLog
-		{
-			get { return Singleton.m_minorEvents.ToString(); }
-		}
+		public static string MinorEventsLog => Singleton.m_minorEvents.ToString();
 
 		/// <summary>
 		/// the place on disk where we are storing the log
@@ -373,10 +371,7 @@ public static string LogPath
 				return _actualLogPath;
 			}
 		}
-		public static Logger Singleton
-		{
-			get { return _singleton; }
-		}
+		public static Logger Singleton => _singleton;
 
 		private static void SetActualLogPath(string filename)
 		{
@@ -407,7 +402,7 @@ private void WriteEventCore(string message, params object[] args)
 				{
 #endif
 				CheckDisposed();
-				if (m_out != null && m_out.BaseStream.CanWrite)
+				if (m_out is { BaseStream: { CanWrite: true } })
 				{
 					m_out.Write(DateTime.Now.ToLongTimeString() + "\t");
 					m_out.WriteLine(FormatMessage(message, args));
@@ -455,7 +450,7 @@ public static void WriteError(string msg, Exception e)
 		}
 
 		/// <summary>
-		/// only a limitted number of the most recent of these events will show up in the log
+		/// only a limited number of the most recent of these events will show up in the log
 		/// </summary>
 		public static void WriteMinorEvent(string message, params object[] args)
 		{
@@ -483,7 +478,7 @@ internal static string FormatMessage(string message, object[] args)
 				}
 				catch (Exception)
 				{
-					return string.Format("Error formatting message with {0} args: {1}", args.Length, message);
+					return $"Error formatting message with {args.Length} args: {message}";
 				}
 			}
 			return message;
@@ -520,16 +515,18 @@ private void WriteMinorEventCore(string message, params object[] args)
 			}
 		}
 
+		[PublicAPI]
 		public static void ShowUserTheLogFile()
 		{
 			Singleton.m_out.Flush();
 			Process.Start(LogPath);
 		}
 
 		/// <summary>
-		/// if you're working with unmanaged code and get a System.AccessViolationException, well you're toast, and anything
-		/// that requires UI is gonna freeze up.  So call this instead
+		/// If you're working with unmanaged code and get a System.AccessViolationException, you're
+		/// toast, and anything that requires UI is going to freeze up. So call this instead.
 		/// </summary>
+		[PublicAPI]
 		public static void ShowUserATextFileRelatedToCatastrophicError(Exception reallyBadException)
 		{
 			//did this special because we don't have an event loop to drive the error reporting dialog if Application.Run() dies