[Xamarin.Android.Tools.AndroidSdk] Fix quotes in %PATH% or %PATHEXT% (#…

…112)

Fixes: https://developercommunity.visualstudio.com/t/illegal-character-exception-in-xamarinandroid-afte/1363149

In 16.9 we are getting some reports of:

	(_ResolveSdks target) ->
	    error XARSD7004: System.ArgumentException: Illegal characters in path.
	    error XARSD7004:    at System.IO.Path.CheckInvalidPathChars(String path, Boolean checkAdditional)
	    error XARSD7004:    at System.IO.Path.Combine(String path1, String path2)
	    error XARSD7004:    at Xamarin.Android.Tools.ProcessUtils.<FindExecutablesInDirectory>d__9.MoveNext()
	    error XARSD7004:    at Xamarin.Android.Tools.ProcessUtils.<FindExecutablesInPath>d__8.MoveNext()
	    error XARSD7004:    at Xamarin.Android.Tools.AndroidSdkBase.<GetAllAvailableAndroidNdks>d__73.MoveNext()
	    error XARSD7004:    at Xamarin.Android.Tools.AndroidSdkWindows.<GetAllAvailableAndroidNdks>d__43.MoveNext()
	    error XARSD7004:    at Xamarin.Android.Tools.AndroidSdkBase.GetValidNdkPath(String ctorParam)
	    error XARSD7004:    at Xamarin.Android.Tools.AndroidSdkBase.Initialize(String androidSdkPath, String androidNdkPath, String javaSdkPath)
	    error XARSD7004:    at Xamarin.Android.Tools.AndroidSdkWindows.Initialize(String androidSdkPath, String androidNdkPath, String javaSdkPath)
	    error XARSD7004:    at Xamarin.Android.Tools.AndroidSdkInfo..ctor(Action`2 logger, String androidSdkPath, String androidNdkPath, String javaSdkPath)
	    error XARSD7004:    at Xamarin.Android.Tasks.MonoAndroidHelper.RefreshAndroidSdk(String sdkPath, String ndkPath, String javaPath, TaskLoggingHelper logHelper)
	    error XARSD7004:    at Xamarin.Android.Tasks.ResolveSdks.RunTask()
	    error XARSD7004:    at Xamarin.Android.Tasks.AndroidTask.Execute()

It appears this is simply a call like this failing:

	Path.Combine ("foo", "\"")
	Path.Combine ("\"", "foo")

The most likely reason for certain customers to get this exception is
because an environment variable contains quotes, e.g.

	rem Windows CMD.EXE
	set PATH='…'

On Windows+CMD.EXE, the `'` (or `"`) is *not* needed, even if the
value has spaces:

	rem Can cause Pain and Suffering™
	set PATH="C:\This Directory Has Spaces;%PATH%"

	rem Good and Proper™
	set PATH=C:\This Directory Has Spaces;%PATH%

I could reproduce this in a test by setting `%PATH%` or `%PATHEXT%`
to invalid names.

For `%PATH%`, I could simply add a `Directory.Exists()` check in the
place that makes sense.  However, I think a `try`/`catch` of
`ArgumentException` is the only way to handle `%PATHEXT%`?  I had to
put this in two places where the new test found an issue.

src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs

@@ -173,8 +173,15 @@ internal static IEnumerable<string> FindExecutablesInPath (string executable)
 
 		internal static IEnumerable<string> FindExecutablesInDirectory (string dir, string executable)
 		{
+			if (!Directory.Exists (dir))
+				yield break;
 			foreach (var exe in ExecutableFiles (executable)) {
-				var exePath = Path.Combine (dir, exe);
+				string exePath;
+				try {
+					exePath = Path.Combine (dir, exe);
+				} catch (ArgumentException) {
+					continue;
+				}
 				if (File.Exists (exePath))
 					yield return exePath;
 			}

src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkBase.cs

@@ -292,9 +292,14 @@ static string GetExecutablePath (string? dir, string exe)
 			if (string.IsNullOrEmpty (dir))
 				return exe;
 
-			foreach (var e in ProcessUtils.ExecutableFiles (exe))
-				if (File.Exists (Path.Combine (dir, e)))
-					return e;
+			foreach (var e in ProcessUtils.ExecutableFiles (exe)) {
+				try {
+					if (File.Exists (Path.Combine (dir, e)))
+						return e;
+				} catch (ArgumentException) {
+					continue;
+				}
+			}
 			return exe;
 		}
 	}

tests/Xamarin.Android.Tools.AndroidSdk-Tests/AndroidSdkInfoTests.cs

@@ -146,6 +146,50 @@ public void Ndk_PathInSdk()
 			}
 		}
 
+		[Test]
+		public void Ndk_Path_InvalidChars ()
+		{
+			CreateSdks (out string root, out string jdk, out string ndk, out string sdk);
+
+			Action<TraceLevel, string> logger = (level, message) => {
+				Console.WriteLine ($"[{level}] {message}");
+				if (level == TraceLevel.Error)
+					Assert.Fail (message);
+			};
+
+			var oldPath = Environment.GetEnvironmentVariable ("PATH");
+			try {
+				Environment.SetEnvironmentVariable ("PATH", "\"C:\\IHAVEQUOTES\\\"");
+				// Check that this doesn't throw
+				new AndroidSdkInfo (logger, androidSdkPath: sdk, androidNdkPath: null, javaSdkPath: jdk);
+			} finally {
+				Environment.SetEnvironmentVariable ("PATH", oldPath);
+				Directory.Delete (root, recursive: true);
+			}
+		}
+
+		[Test]
+		public void Ndk_PathExt_InvalidChars ()
+		{
+			CreateSdks (out string root, out string jdk, out string ndk, out string sdk);
+
+			Action<TraceLevel, string> logger = (level, message) => {
+				Console.WriteLine ($"[{level}] {message}");
+				if (level == TraceLevel.Error)
+					Assert.Fail (message);
+			};
+
+			var oldPathExt = Environment.GetEnvironmentVariable ("PATHEXT");
+			try {
+				Environment.SetEnvironmentVariable ("PATHEXT", string.Join (Path.PathSeparator.ToString (), "\"", ".EXE", ".BAT"));
+				// Check that this doesn't throw
+				new AndroidSdkInfo (logger, androidSdkPath: sdk, androidNdkPath: null, javaSdkPath: jdk);
+			} finally {
+				Environment.SetEnvironmentVariable ("PATHEXT", oldPathExt);
+				Directory.Delete (root, recursive: true);
+			}
+		}
+
 		[Test]
 		public void Ndk_AndroidSdkDoesNotExist ()
 		{