Recursion resource resolve regresssion

[nohwnd]

Describe the bug

Both vstest and mstest use custom assembly resolver. There is an edge case that vstest protects against which is not resolving the same dll recursively, which leads to stack overflow. This can happen when you are loading locale that you don't have, it will try to load the locale, cannot find the file, so it throws file not found, looks for the correct string to show the exception, which is in the same resource, so it again cannot find the file and throws.

In VSTest we explicitly test for this, and after upgrade from 2.2.10 to 3.3.1 we can see this assertion failure:

[Window Title]
testhost.exe - Assert Failure

[Main Instruction]
testhost.exe - Assert Failure

[Content]
Expression: [mscorlib recursive resource lookup bug]
Description: Infinite recursion during resource lookup within mscorlib.  This may be a bug in mscorlib, or potentially in certain extensibility points such as assembly resolve events or CultureInfo names.  Resource name: Security_Generic

[Expanded Information]
Stack Trace:
   at System.Environment.ResourceHelper.GetResourceStringCode(Object userDataIn)
   at System.Runtime.CompilerServices.RuntimeHelpers.ExecuteCodeWithGuaranteedCleanup(TryCode code, CleanupCode backoutCode, Object userData)
   at System.Environment.ResourceHelper.GetResourceString(String key, CultureInfo culture)
   at System.Environment.GetResourceStringLocal(String key)
   at System.Security.CodeAccessSecurityEngine.Check(Object demand, StackCrawlMark& stackMark, Boolean isPermSet)
   at System.Security.CodeAccessPermission.Demand()
   at System.Security.Permissions.FileIOPermission.QuickDemand(FileIOPermissionAccess access, String fullPath, Boolean checkForDuplicates, Boolean needFullPath)
   at System.IO.File.InternalExistsHelper(String path, Boolean checkHost)
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.AssemblyResolver.DoesFileExist(String filePath)
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.AssemblyResolver.SearchAndLoadAssembly(String assemblyPath, String assemblyName, AssemblyName requestedName, Boolean isReflectionOnly)
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.AssemblyResolver.SearchAssembly(List`1 searchDirectorypaths, String name, Boolean isReflectionOnly)
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.AssemblyResolver.OnResolveInternal(Object senderAppDomain, ResolveEventArgs args, Boolean isReflectionOnly)
   at System.AppDomain.OnAssemblyResolveEvent(RuntimeAssembly assembly, String assemblyFullName)
   at System.Reflection.RuntimeAssembly._nLoad(AssemblyName fileName, String codeBase, Evidence assemblySecurity, RuntimeAssembly locationHint, StackCrawlMark& stackMark, IntPtr pPrivHostBinder, Boolean throwOnFileNotFound, Boolean forIntrospection, Boolean suppressSecurityChecks)
   at System.Reflection.RuntimeAssembly.InternalGetSatelliteAssembly(String name, CultureInfo culture, Version version, Boolean throwOnFileNotFound, StackCrawlMark& stackMark)
   at System.Resources.ManifestBasedResourceGroveler.GetSatelliteAssembly(CultureInfo lookForCulture, StackCrawlMark& stackMark)
   at System.Resources.ManifestBasedResourceGroveler.GrovelForResourceSet(CultureInfo culture, Dictionary`2 localResourceSets, Boolean tryParents, Boolean createIfNotExists, StackCrawlMark& stackMark)
   at System.Resources.ResourceManager.InternalGetResourceSet(CultureInfo requestedCulture, Boolean createIfNotExists, Boolean tryParents, StackCrawlMark& stackMark)
   at System.Resources.ResourceManager.InternalGetResourceSet(CultureInfo culture, Boolean createIfNotExists, Boolean tryParents)
   at System.Resources.ResourceManager.GetString(String name, CultureInfo culture)
   at System.Environment.ResourceHelper.GetResourceStringCode(Object userDataIn)
   at System.Runtime.CompilerServices.RuntimeHelpers.ExecuteCodeWithGuaranteedCleanup(TryCode code, CleanupCode backoutCode, Object userData)
   at System.Environment.ResourceHelper.GetResourceString(String key, CultureInfo culture)
   at System.Environment.GetResourceStringLocal(String key)
   at System.Security.CodeAccessSecurityEngine.Check(Object demand, StackCrawlMark& stackMark, Boolean isPermSet)
   at System.Security.CodeAccessPermission.Demand()
   at System.Security.Permissions.FileIOPermission.QuickDemand(FileIOPermissionAccess access, String fullPath, Boolean checkForDuplicates, Boolean needFullPath)
   at System.IO.File.InternalExistsHelper(String path, Boolean checkHost)
   at Microsoft.VisualStudio.TestPlatform.Common.Utilities.AssemblyResolver.OnResolve(Object sender, AssemblyResolveEventArg

Expected behavior

There is no fail.

protection against this, such as https://github.com/microsoft/vstest/blob/main/src/Microsoft.TestPlatform.Common/Utilities/AssemblyResolver.cs#L144-L159 is in place.

Actual behavior

Additional context

Filing this bug so I don't lose the details. I did not investigate if there was change in MSTest, but there was none in VSTest so maybe some "simplicification" of code happened in mstest, or there is simply an interaction that was not there before.

[nohwnd]

This was found because in vstest repo this test is failing: RunsToCompletionWhenJapaneseResourcesAreLookedUpForMSCorLib , I've temporarily disabled it so we can update. Re-enable once solved.

[Evangelink]

I guess we need to duplicate the logic but this is so WTF to me 🤯🤯🤯

[nohwnd]

This should be .NET Framework only issue, because .NET does not localize exceptions.