Validate addins file while reading it

src/NUnitEngine/nunit.engine.core.tests/Internal/AddinsFileTests.cs

@@ -48,6 +48,16 @@ public void Read_Stream()
             }
         }
 
+        [Test]
+        public void Read_InvalidEntry()
+        {
+            var content = "// This is not valid";
+            using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(content)))
+            {
+                Assert.That(() => AddinsFile.Read(stream), Throws.Exception);
+            }
+        }
+
         [Test]
         [Platform("win")]
         public void Read_Stream_TransformBackslash_Windows()

src/NUnitEngine/nunit.engine.core.tests/Internal/PathUtilTests.cs

@@ -63,9 +63,36 @@ public void IsFullyQualifiedWindowsPath_PathIsNull()
         {
             Assert.That(() => PathUtils.IsFullyQualifiedWindowsPath(null), Throws.ArgumentNullException);
         }
+
+        [TestCase("X")]
+        [TestCase("X/")]
+        [TestCase("/X/")]
+        [TestCase("\\X\\")]
+        [TestCase("X/Y/Z")]
+        [TestCase("X/Y/Z/")]
+        [TestCase("/X/Y/Z")]
+        [TestCase("/X/Y/Z/")]
+        [TestCase("\\X\\Y\\Z\\")]
+        [TestCase("C:X/Y/Z/")]
+        [TestCase("C:/X/Y/Z")]
+        [TestCase("C:/X/Y/Z/")]
+        [TestCase("C:\\X\\Y\\Z\\")]
+        public void IsValidPath(string path)
+        {
+            Assert.That(PathUtils.IsValidPath(path), Is.True);
+        }
+
+        [TestCase(":")]
+        [TestCase("?")]
+        [TestCase("*")]
+        [TestCase("// Spurious comment")]
+        public void IsValidPath_Fails(string path)
+        {
+            Assert.That(PathUtils.IsValidPath(path), Is.False);
+        }
     }
 
-	[TestFixture]
+    [TestFixture]
 	public class PathUtilDefaultsTests : PathUtils
 	{
 		[Test]

src/NUnitEngine/nunit.engine.core/Internal/AddinsFile.cs

@@ -20,7 +20,7 @@ public static AddinsFile Read(IFile file)
 
             using (var stream = new FileStream(file.FullName, FileMode.Open, FileAccess.Read, FileShare.Read))
             {
-                return Read(stream);
+                return Read(stream, file.FullName);
             }
         }
 
@@ -31,15 +31,24 @@ public static AddinsFile Read(IFile file)
         /// <returns>All entries contained in the file.</returns>
         /// <exception cref="System.IO.IOException"><paramref name="stream"/> cannot be read</exception>
         /// <remarks>If the executing system uses backslashes ('\') to separate directories, these will be substituted with slashes ('/').</remarks>
-        internal static AddinsFile Read(Stream stream)
+        internal static AddinsFile Read(Stream stream, string fullName = null)
         {
             using (var reader = new StreamReader(stream))
             {
                 var addinsFile = new AddinsFile();
 
                 int lineNumber = 0;
                 while (!reader.EndOfStream)
-                    addinsFile.Add(new AddinsFileEntry(++lineNumber, reader.ReadLine()));
+                {
+                    var entry = new AddinsFileEntry(++lineNumber, reader.ReadLine());
+                    if (entry.Text != "" && !entry.IsValid)
+                    {
+                        string msg = $"Invalid Entry in {fullName ?? "addins file"}:\r\n  {entry}";
+                        throw new InvalidOperationException(msg);
+                    }
+
+                    addinsFile.Add(entry);
+                }
 
                 return addinsFile;
             }

src/NUnitEngine/nunit.engine.core/Internal/AddinsFileEntry.cs

@@ -13,6 +13,14 @@ internal class AddinsFileEntry
         public string RawText { get; }
         public string Text { get; }
 
+        public bool IsFullyQualified => PathUtils.IsFullyQualifiedPath(Text);
+        public bool IsDirectory => Text.EndsWith("/");
+        public bool IsPattern => Text.Contains("*");
+        public bool IsValid => PathUtils.IsValidPath(Text.Replace('*', 'X'));
+
+        public string DirectoryName => Path.GetDirectoryName(Text);
+        public string FileName => Path.GetFileName(Text);
+
         public AddinsFileEntry(int lineNumber, string rawText)
         {
             LineNumber = lineNumber;

src/NUnitEngine/nunit.engine.core/Internal/PathUtils.cs

@@ -144,10 +144,10 @@ public static bool SamePathOrUnder( string path1, string path2 )
 
             // if lengths are the same, check for equality
             if ( length1 == length2 )
-                return string.Compare( path1, path2, IsWindows() ) == 0;
+                return string.Compare( path1, path2, RunningOnWindows ) == 0;
 
             // path 2 is longer than path 1: see if initial parts match
-            if ( string.Compare( path1, path2.Substring( 0, length1 ), IsWindows() ) != 0 )
+            if ( string.Compare( path1, path2.Substring( 0, length1 ), RunningOnWindows ) != 0 )
                 return false;
 
             // must match through or up to a directory separator boundary
@@ -167,7 +167,20 @@ public static string Combine(string path1, params string[] morePaths)
         }
 
         /// <summary>
-        /// Returns a value that indicates whether the specified file path is absolute or not on Windows operating systems.
+        /// Returns a value that indicates whether the specified file path is fully qualified.
+        /// </summary>
+        /// <param name="path">Path to check</param>
+        /// <returns><see langword="true"/> if <paramref name="path"/> is an absolute path; otherwhise, false.</returns>
+        /// <exception cref="ArgumentNullException"><paramref name="path"/></exception>
+        public static bool IsFullyQualifiedPath(string path )
+        {
+            return RunningOnWindows
+                ? IsFullyQualifiedWindowsPath(path)
+                : IsFullyQualifiedUnixPath(path);
+        }
+
+        /// <summary>
+        /// Returns a value that indicates whether the specified file path is fully qualified or not on Windows operating systems.
         /// </summary>
         /// <param name="path">Path to check</param>
         /// <returns><see langword="true"/> if <paramref name="path"/> is an absolute or UNC path; otherwhise, false.</returns>
@@ -206,6 +219,27 @@ public static bool IsFullyQualifiedUnixPath(string path)
             return path.Length > 0 && path[0] == '/';
         }
 
+        public static bool IsValidPath(string path)
+        {
+            try
+            {
+                var info = GetFileSystemInfo(path);
+#if NETCOREAPP2_1_OR_GREATER
+                var creation = info.CreationTime;
+#endif
+                return true; // Whether it exists or not!
+            }
+            catch
+            {
+                return false;
+            }
+        }
+
+        private static FileSystemInfo GetFileSystemInfo(string path) => 
+            path.EndsWith("/") || path.EndsWith(@"\\") 
+                ? new DirectoryInfo(path) as FileSystemInfo 
+                : new FileInfo(path) as FileSystemInfo;
+
         private static bool IsWindowsDirectorySeparator(char c)
         {
             return c == '\\' || c == '/';
@@ -216,14 +250,11 @@ private static bool IsValidDriveSpecifier(char c)
             return ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z');
         }
 
-        private static bool IsWindows()
-        {
-            return PathUtils.DirectorySeparatorChar == '\\';
-        }
+        private static bool RunningOnWindows => DirectorySeparatorChar == '\\';
 
         private static string[] SplitPath(string path)
         {
-            char[] separators = new char[] { PathUtils.DirectorySeparatorChar, PathUtils.AltDirectorySeparatorChar };
+            char[] separators = new char[] { DirectorySeparatorChar, AltDirectorySeparatorChar };
 
             string[] trialSplit = path.Split(separators);
 
@@ -246,7 +277,7 @@ private static string[] SplitPath(string path)
 
         private static bool PathsEqual(string path1, string path2)
         {
-            if (PathUtils.IsWindows())
+            if (RunningOnWindows)
                 return path1.ToLower().Equals(path2.ToLower());
             else
                 return path1.Equals(path2);

src/NUnitEngine/nunit.engine.core/Services/ExtensionManager.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Reflection;
 using TestCentric.Metadata;
 using NUnit.Engine.Extensibility;
@@ -10,13 +11,6 @@
 using NUnit.Engine.Internal.FileSystemAccess.Default;
 using System.Linq;
 
-
-#if NET462 || NETSTANDARD2_0
-using Path = NUnit.Engine.Internal.Backports.Path;
-#else
-using Path = System.IO.Path;
-#endif
-
 namespace NUnit.Engine.Services
 {
     public class ExtensionManager : IExtensionManager
@@ -286,7 +280,7 @@ private int ProcessAddinsFiles(IDirectory startDir, bool fromWildCard)
 
             foreach (var file in addinsFiles)
             {
-                ProcessAddinsFile(startDir, file, fromWildCard);
+                ProcessAddinsFile(file, fromWildCard);
                 addinsFileCount++;
             }
 
@@ -299,56 +293,43 @@ private int ProcessAddinsFiles(IDirectory startDir, bool fromWildCard)
         /// path or a wildcard pattern used to find assemblies. Blank
         /// lines and comments started by # are ignored.
         /// </summary>
-        private void ProcessAddinsFile(IDirectory baseDir, IFile addinsFile, bool fromWildCard)
+        private void ProcessAddinsFile(IFile addinsFile, bool fromWildCard)
         {
             log.Info("Processing file " + addinsFile.FullName);
 
             foreach (var entry in AddinsFile.Read(addinsFile).Where(e => e.Text != string.Empty))
             {
-                bool isWild = fromWildCard || entry.Text.Contains("*");
-                var args = GetBaseDirAndPattern(baseDir, entry.Text);
-                // TODO: See if we can handle '/tools/*/' efficiently by examining every
-                // assembly in the directory. Otherwise try this approach:
-                // 1. Check entry for ending with '/tools/*/'
-                // 2. If so, examine the directory name to see if it matches a tfm.
-                // 3. If it does, check to see if the implied runtime would be loadable.
-                // 4. If so, process it, if not, skip it.
-                if (entry.Text.EndsWith("/"))
+                bool isWild = fromWildCard || entry.IsPattern;
+                IDirectory baseDir = addinsFile.Parent;
+                string entryDir = entry.DirectoryName;
+                string entryFile = entry.FileName;
+
+                if (entry.IsDirectory)
                 {
-                    foreach (var dir in _directoryFinder.GetDirectories(args.Item1, args.Item2))
+                    if (entry.IsFullyQualified)
                     {
-                        ProcessDirectory(dir, isWild);
+                        baseDir = _fileSystem.GetDirectory(entry.Text);
+                        foreach (var dir in _directoryFinder.GetDirectories(_fileSystem.GetDirectory(entryDir), ""))
+                            ProcessDirectory(dir, isWild);
                     }
+                    else
+                        foreach (var dir in _directoryFinder.GetDirectories(baseDir, entry.Text))
+                            ProcessDirectory(dir, isWild);
                 }
                 else
                 {
-                    foreach (var file in _directoryFinder.GetFiles(args.Item1, args.Item2))
+                    if (entry.IsFullyQualified)
                     {
-                        ProcessCandidateAssembly(file.FullName, isWild);
+                        foreach (var file in _directoryFinder.GetFiles(_fileSystem.GetDirectory(entryDir), entryFile))
+                            ProcessCandidateAssembly(file.FullName, isWild);
                     }
+                    else
+                        foreach (var file in _directoryFinder.GetFiles(baseDir, entry.Text))
+                            ProcessCandidateAssembly(file.FullName, isWild);
                 }
             }
         }
 
-        private Tuple<IDirectory, string> GetBaseDirAndPattern(IDirectory baseDir, string path)
-        {
-            if (Path.IsPathFullyQualified(path))
-            {
-                if (path.EndsWith("/"))
-                {
-                    return new Tuple<IDirectory, string>(_fileSystem.GetDirectory(path), string.Empty);
-                }
-                else
-                {
-                    return new Tuple<IDirectory, string>(_fileSystem.GetDirectory(System.IO.Path.GetDirectoryName(path)), System.IO.Path.GetFileName(path));
-                }
-            }
-            else
-            {
-                return new Tuple<IDirectory, string>(baseDir, path);
-            }
-        }
-
         private void ProcessCandidateAssembly(string filePath, bool fromWildCard)
         {
             if (WasVisited(filePath, fromWildCard))