fixed FileLoadException when trying to do clr.AddReference('/full/lib/path.dll')

lostmsu · lostmsu · commit fc989d5bb339 · 2021-09-27T17:10:03.000-07:00
Before trying to load an assembly by its full path we were trying to call `Assembly.Load` on it. `Assembly.Load` interprets its argument as a valid `AssemblyName`. However full paths are not valid assembly names, so that call would throw `FileLoadException`, which we did not handle. reported in 9d5f579#commitcomment-57061082 Related: #1514
diff --git a/src/embed_tests/pyimport.cs b/src/embed_tests/pyimport.cs
@@ -1,5 +1,7 @@
 using System;
 using System.IO;
+using System.Runtime.InteropServices;
+
 using NUnit.Framework;
 using Python.Runtime;
 
@@ -83,10 +85,18 @@ public void TestCastGlobalVar()
         public void BadAssembly()
         {
             string path;
-            if (Python.Runtime.Runtime.IsWindows)
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
             {
                 path = @"C:\Windows\System32\kernel32.dll";
             }
+            else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
+            {
+                path = "/usr/lib/libc.dylib";
+            }
+            else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
+            {
+                path = "/usr/lib/locale/locale-archive";
+            }
             else
             {
                 Assert.Pass("TODO: add bad assembly location for other platforms");
@@ -98,7 +108,7 @@ import clr
 clr.AddReference('{path}')
 ";
 
-            Assert.Throws<FileLoadException>(() => PythonEngine.Exec(code));
+            Assert.Throws<BadImageFormatException>(() => PythonEngine.Exec(code));
         }
     }
 }
diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs
@@ -121,6 +121,18 @@ private static Assembly ResolveHandler(object ob, ResolveEventArgs args)
             return LoadAssemblyPath(name.Name);
         }
 
+        internal static AssemblyName? TryParseAssemblyName(string name)
+        {
+            try
+            {
+                return new AssemblyName(name);
+            }
+            catch (FileLoadException)
+            {
+                return null;
+            }
+        }
+
 
         /// <summary>
         /// We __really__ want to avoid using Python objects or APIs when
@@ -208,18 +220,11 @@ static IEnumerable<string> FindAssemblyCandidates(string name)
 
         /// <summary>
         /// Loads an assembly from the application directory or the GAC
-        /// given a simple assembly name. Returns the assembly if loaded.
+        /// given its name. Returns the assembly if loaded.
         /// </summary>
-        public static Assembly LoadAssembly(string name)
+        public static Assembly LoadAssembly(AssemblyName name)
         {
-            try
-            {
-                return Assembly.Load(name);
-            }
-            catch (FileNotFoundException)
-            {
-                return null;
-            }
+            return Assembly.Load(name);
         }
 
 
diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs
@@ -516,9 +516,9 @@ public static Assembly AddReference(string name)
             {
                 assembly = AssemblyManager.LoadAssemblyPath(name);
             }
-            if (assembly == null)
+            if (assembly == null && AssemblyManager.TryParseAssemblyName(name) is { } parsedName)
             {
-                assembly = AssemblyManager.LoadAssembly(name);
+                assembly = AssemblyManager.LoadAssembly(parsedName);
             }
             if (assembly == null)
             {

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,7 @@`
`1`	`1`	`using System;`
`2`	`2`	`using System.IO;`
	`3`	`+using System.Runtime.InteropServices;`
	`4`	`+`
`3`	`5`	`using NUnit.Framework;`
`4`	`6`	`using Python.Runtime;`
`5`	`7`
`@@ -83,10 +85,18 @@ public void TestCastGlobalVar()`
`83`	`85`	`public void BadAssembly()`
`84`	`86`	`{`
`85`	`87`	`string path;`
`86`		`- if (Python.Runtime.Runtime.IsWindows)`
	`88`	`+ if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))`
`87`	`89`	`{`
`88`	`90`	`path = @"C:\Windows\System32\kernel32.dll";`
`89`	`91`	`}`
	`92`	`+ else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))`
	`93`	`+ {`
	`94`	`+ path = "/usr/lib/libc.dylib";`
	`95`	`+ }`
	`96`	`+ else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))`
	`97`	`+ {`
	`98`	`+ path = "/usr/lib/locale/locale-archive";`
	`99`	`+ }`
`90`	`100`	`else`
`91`	`101`	`{`
`92`	`102`	`Assert.Pass("TODO: add bad assembly location for other platforms");`
`@@ -98,7 +108,7 @@ import clr`
`98`	`108`	`clr.AddReference('{path}')`
`99`	`109`	`";`
`100`	`110`
`101`		`- Assert.Throws<FileLoadException>(() => PythonEngine.Exec(code));`
	`111`	`+ Assert.Throws<BadImageFormatException>(() => PythonEngine.Exec(code));`
`102`	`112`	`}`
`103`	`113`	`}`
`104`	`114`	`}`
Original file line number	Diff line number	Diff line change
`@@ -516,9 +516,9 @@ public static Assembly AddReference(string name)`
`516`	`516`	`{`
`517`	`517`	`assembly = AssemblyManager.LoadAssemblyPath(name);`
`518`	`518`	`}`
`519`		`- if (assembly == null)`
	`519`	`+ if (assembly == null && AssemblyManager.TryParseAssemblyName(name) is { } parsedName)`
`520`	`520`	`{`
`521`		`- assembly = AssemblyManager.LoadAssembly(name);`
	`521`	`+ assembly = AssemblyManager.LoadAssembly(parsedName);`
`522`	`522`	`}`
`523`	`523`	`if (assembly == null)`
`524`	`524`	`{`