Fix assembly load when file path contains URI reserved chars (#76617)

Resolves microsoft/vscode-dotnettools#1686

When vscode is installed in (or an extension assembly is loaded from) a
path that contains a URI reserved character, the server crashes. This is
because VS-MEF (for compatibility with clr behavior) sets the code base
to a file URI with an unencoded path.

When parsing the URI, some reserved characters cause the path component
to be incorrectly split into query or fragment components if unescaped.
Then we get an invalid file path from the URI and cannot load the
extension dll.

Author: dibarbet

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/ExportProviderBuilderTests.cs

@@ -2,6 +2,11 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using Basic.Reference.Assemblies;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.Test.Utilities;
+using Microsoft.CodeAnalysis.Text;
+using Roslyn.Test.Utilities;
 using Xunit.Abstractions;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests
@@ -49,6 +54,98 @@ public async Task MultipleMefCompositionsAreCached()
             AssertCachedCompositionCountEquals(expectedCount: 2);
         }
 
+        [Theory, WorkItem("https://github.com/microsoft/vscode-dotnettools/issues/1686")]
+        // gen-delims
+        [InlineData("#"),
+        InlineData(":"),
+        InlineData("?"),
+        InlineData("["),
+        InlineData("]"),
+        InlineData("@"),
+        // sub-delims
+        InlineData("!"),
+        InlineData("$"),
+        InlineData("&"),
+        InlineData("'"),
+        InlineData("("),
+        InlineData(")"),
+        InlineData("*"),
+        InlineData("+"),
+        InlineData(","),
+        InlineData(";"),
+        InlineData("=")]
+        public async Task CanFindCodeBaseWhenReservedCharactersInPath(string reservedCharacter)
+        {
+            // Test that given an unescaped code base URI (as vs-mef gives us), we can resolve the file path even if it contains reserved characters.
+
+            // Certain characters aren't valid file paths on different file systems and so can't be in the path.
+            if (ExecutionConditionUtil.IsWindows && reservedCharacter is "*" or ":" or "?")
+            {
+                return;
+            }
+
+            var dllPath = GenerateDll(reservedCharacter, out var assemblyName);
+
+            await using var testServer = await TestLspServer.CreateAsync(new Roslyn.LanguageServer.Protocol.ClientCapabilities(), TestOutputLogger, MefCacheDirectory.Path, includeDevKitComponents: true, [dllPath]);
+
+            var assemblies = AppDomain.CurrentDomain.GetAssemblies();
+            var assembly = Assert.Single(assemblies, a => a.GetName().Name == assemblyName);
+            var type = Assert.Single(assembly.GetTypes(), t => t.FullName?.Contains("ExportedType") == true);
+            var values = testServer.ExportProvider.GetExportedValues(type, contractName: null);
+            Assert.Single(values);
+        }
+
+        private string GenerateDll(string reservedCharacter, out string assemblyName)
+        {
+            var directory = TempRoot.CreateDirectory();
+            CSharpCompilationOptions options = new(OutputKind.DynamicallyLinkedLibrary);
+
+            // Create a dll that exports and imports a mef type to ensure that MEF attempts to load and create a MEF graph
+            // using this assembly.
+            var source = """
+                namespace MyTestExportNamespace
+                {
+                    [System.ComponentModel.Composition.Export(typeof(ExportedType))]
+                    public class ExportedType { }
+
+                    public class ImportType
+                    {
+                        [System.ComponentModel.Composition.ImportingConstructorAttribute]
+                        public ImportType(ExportedType t) { }
+                    }
+                }
+                """;
+
+            // Generate an assembly name associated with the character we're testing - this ensures
+            // that if multiple of these tests run in the same process we're making sure that the correct expected assembly is loaded.
+            assemblyName = "MyAssembly" + reservedCharacter.GetHashCode();
+#pragma warning disable RS0030 // Do not use banned APIs - intentionally using System.ComponentModel.Composition to verify mef construction.
+            var compilation = CSharpCompilation.Create(
+                assemblyName,
+                syntaxTrees: [SyntaxFactory.ParseSyntaxTree(SourceText.From(source, encoding: null, SourceHashAlgorithms.Default))],
+                references:
+                [
+                    NetStandard20.References.mscorlib,
+                    NetStandard20.References.netstandard,
+                    NetStandard20.References.SystemRuntime,
+                    MetadataReference.CreateFromFile(typeof(System.ComponentModel.Composition.ExportAttribute).Assembly.Location)
+                ],
+                options: options);
+#pragma warning restore RS0030 // Do not use banned APIs
+
+            // Write a dll to a subdir with a reserved character in the name.
+            var tempSubDir = directory.CreateDirectory(reservedCharacter);
+            var tempFile = tempSubDir.CreateFile($"{assemblyName}.dll");
+            var dllData = compilation.EmitToStream();
+            tempFile.WriteAllBytes(dllData.ToArray());
+
+            // Mark the file as read only to prevent mutations.
+            var fileInfo = new FileInfo(tempFile.Path);
+            fileInfo.IsReadOnly = true;
+
+            return tempFile.Path;
+        }
+
         private async Task AssertCacheWriteWasAttemptedAsync()
         {
             var cacheWriteTask = ExportProviderBuilder.TestAccessor.GetCacheWriteTask();

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/TelemetryReporterTests.cs

@@ -18,6 +18,7 @@ private async Task<ITelemetryReporter> CreateReporterAsync()
             TestOutputLogger.Factory,
             includeDevKitComponents: true,
             MefCacheDirectory.Path,
+            [],
             out var _,
             out var _);
 

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/AbstractLanguageServerHostTests.cs

@@ -42,10 +42,10 @@ protected sealed class TestLspServer : IAsyncDisposable
 
         private ServerCapabilities? _serverCapabilities;
 
-        internal static async Task<TestLspServer> CreateAsync(ClientCapabilities clientCapabilities, TestOutputLogger logger, string cacheDirectory, bool includeDevKitComponents = true)
+        internal static async Task<TestLspServer> CreateAsync(ClientCapabilities clientCapabilities, TestOutputLogger logger, string cacheDirectory, bool includeDevKitComponents = true, string[]? extensionPaths = null)
         {
             var exportProvider = await LanguageServerTestComposition.CreateExportProviderAsync(
-                logger.Factory, includeDevKitComponents, cacheDirectory, out var _, out var assemblyLoader);
+                logger.Factory, includeDevKitComponents, cacheDirectory, extensionPaths, out var _, out var assemblyLoader);
             var testLspServer = new TestLspServer(exportProvider, logger, assemblyLoader);
             var initializeResponse = await testLspServer.ExecuteRequestAsync<InitializeParams, InitializeResult>(Methods.InitializeName, new InitializeParams { Capabilities = clientCapabilities }, CancellationToken.None);
             Assert.NotNull(initializeResponse?.Capabilities);

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs

@@ -24,6 +24,7 @@ public static Task<ExportProvider> CreateExportProviderAsync(
         ILoggerFactory loggerFactory,
         bool includeDevKitComponents,
         string cacheDirectory,
+        string[]? extensionPaths,
         out ServerConfiguration serverConfiguration,
         out IAssemblyLoader assemblyLoader)
     {
@@ -33,7 +34,7 @@ public static Task<ExportProvider> CreateExportProviderAsync(
             StarredCompletionsPath: null,
             TelemetryLevel: null,
             SessionId: null,
-            ExtensionAssemblyPaths: [],
+            ExtensionAssemblyPaths: extensionPaths ?? [],
             DevKitDependencyPath: devKitDependencyPath,
             RazorSourceGenerator: null,
             RazorDesignTimePath: null,

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/WorkspaceProjectFactoryServiceTests.cs

@@ -19,7 +19,7 @@ public async Task CreateProjectAndBatch()
     {
         var loggerFactory = new LoggerFactory();
         using var exportProvider = await LanguageServerTestComposition.CreateExportProviderAsync(
-            loggerFactory, includeDevKitComponents: false, MefCacheDirectory.Path, out var serverConfiguration, out var _);
+            loggerFactory, includeDevKitComponents: false, MefCacheDirectory.Path, [], out var serverConfiguration, out var _);
 
         exportProvider.GetExportedValue<ServerConfigurationFactory>()
             .InitializeConfiguration(serverConfiguration);

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/CustomExportAssemblyLoader.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Diagnostics.CodeAnalysis;
 using System.Reflection;
 using System.Runtime.Loader;
 using Microsoft.CodeAnalysis.LanguageServer.Services;
@@ -74,7 +75,13 @@ private Assembly LoadAssembly(AssemblyName assemblyName, string? codeBasePath)
 
     private Assembly LoadAssemblyFromCodeBase(AssemblyName assemblyName, string codeBaseUriStr)
     {
-        // CodeBase is spec'd as being a URL string.
+        // CodeBase is spec'd as being a URI string - however VS-MEF doesn't always give us a URI, nor do they always give us a valid URI representation of the code base (for compatibility with clr behavior).
+        // For example, when doing initial part discovery, we get a normal path as a string.  But when loading the assembly to create the graph, VS-MEF will pass us
+        // a file URI with an unescaped path part.  
+        // See https://github.com/microsoft/vs-mef/blob/21feb66e55cbef129801de3a6d572c087ee5b0b6/src/Microsoft.VisualStudio.Composition/Resolver.cs#L172
+        //
+        // This can cause issues during URI parsing, but we will handle that below.  First we try to parse the code base as a normal URI, which handles all the cases where we get
+        // a non-URI string as well as the majority of the cases where we get a file URI string.
         var codeBaseUri = ProtocolConversions.CreateAbsoluteUri(codeBaseUriStr);
         if (!codeBaseUri.IsFile)
         {
@@ -83,15 +90,52 @@ private Assembly LoadAssemblyFromCodeBase(AssemblyName assemblyName, string code
 
         var codeBasePath = codeBaseUri.LocalPath;
 
-        var assembly = extensionAssemblyManager.TryLoadAssemblyInExtensionContext(codeBasePath);
-        if (assembly is not null)
+        if (TryLoadAssemblyFromCodeBasePath(assemblyName, codeBasePath, out var assembly))
+        {
+            return assembly;
+        }
+
+        // As described above, we can get a code base URI that contains the unescaped code base file path.  This can cause issues when we parse it as a URI if the code base file path
+        // contains URI reserved characters (for example '#') which are left unescaped in the URI string.  While it is a well formed URI, when System.Uri parses the code base URI
+        // the path component can get mangled and longer accurately represent the actual file system path.
+        //
+        // A concrete example - given code base URI 'file:///c:/Learn C#/file.dll', the path component from System.Uri will be 'c:/learn c' and '#/file.dll' is parsed as part of the fragment.
+        // Of course we do not find a dll at 'c:/learn c' and crash.
+        //
+        // Unfortunately, solving this can be difficult - there is an EscapedCodeBase property on AssemblyName, but it does not escape reserved characters.  It uses
+        // the same implementation as Uri.EscapeUriString (which explicitly does not escape reserved characters as it cannot accurately do so).
+        //
+        // However - we do know that if we are given a file URI, the scheme and authority parts of the URI are correct (only the path can have unescaped reserved characters, which comes after both).
+        // We can attempt to reconstruct the real code base file path by combining all the URI parts following the authority (the path, query, and fragment).
+        // Note - System.URI returns the escaped versions of all these parts, so we unescape them first.
+        var possibleCodeBasePath = Uri.UnescapeDataString(codeBaseUri.PathAndQuery) + Uri.UnescapeDataString(codeBaseUri.Fragment);
+        if (TryLoadAssemblyFromCodeBasePath(assemblyName, possibleCodeBasePath, out assembly))
         {
-            _logger.LogTrace("{assemblyName} with code base {codeBase} found in extension context.", assemblyName, codeBasePath);
             return assembly;
         }
 
         // We were given an explicit code base path, but no extension context had the assembly.
         // This is unexpected, so we'll throw an exception.
         throw new FileNotFoundException($"Could not find assembly {assemblyName} with code base {codeBasePath} in any extension context.");
     }
+
+    private bool TryLoadAssemblyFromCodeBasePath(AssemblyName assemblyName, string codeBasePath, [NotNullWhen(true)] out Assembly? assembly)
+    {
+        assembly = null;
+        if (!File.Exists(codeBasePath))
+        {
+            _logger.LogTrace("Code base {codeBase} does not exist for {assemblyName}", codeBasePath, assemblyName);
+            return false;
+        }
+
+        assembly = extensionAssemblyManager.TryLoadAssemblyInExtensionContext(codeBasePath);
+        if (assembly is not null)
+        {
+            _logger.LogTrace("{assemblyName} with code base {codeBase} found in extension context.", assemblyName, codeBasePath);
+            return true;
+        }
+
+        _logger.LogTrace("Code base {codeBase} not found in any extension context for {assemblyName}", codeBasePath, assemblyName);
+        return false;
+    }
 }