Skip to content

Commit 0073708

Browse files
committed
Fix infinite resource lookup recursion
1 parent d1a5098 commit 0073708

File tree

2 files changed

+77
-51
lines changed

2 files changed

+77
-51
lines changed

src/Adapter/MSTestAdapter.PlatformServices/AssemblyResolver.cs

+76-50
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ class AssemblyResolver :
8686
/// </summary>
8787
private readonly object _syncLock = new();
8888

89+
private Stack<string>? _currentlyResolvingResources;
8990
private bool _disposed;
9091

9192
/// <summary>
@@ -335,7 +336,7 @@ protected virtual
335336
if (EqtTrace.IsInfoEnabled)
336337
{
337338
EqtTrace.Info(
338-
"AssemblyResolver: {0}: Failed to create assemblyName. Reason:{1} ",
339+
"MSTest.AssemblyResolver.OnResolve: Failed to create assemblyName '{0}'. Reason: {1} ",
339340
name,
340341
ex);
341342
}
@@ -344,7 +345,7 @@ protected virtual
344345
return null;
345346
}
346347

347-
DebugEx.Assert(requestedName != null && !StringEx.IsNullOrEmpty(requestedName.Name), "AssemblyResolver.OnResolve: requested is null or name is empty!");
348+
DebugEx.Assert(requestedName != null && !StringEx.IsNullOrEmpty(requestedName.Name), "MSTest.AssemblyResolver.OnResolve: requested is null or name is empty!");
348349

349350
foreach (string dir in searchDirectorypaths)
350351
{
@@ -359,15 +360,41 @@ protected virtual
359360
{
360361
if (EqtTrace.IsVerboseEnabled)
361362
{
362-
EqtTrace.Verbose("AssemblyResolver: Searching assembly: {0} in the directory: {1}", requestedName.Name, dir);
363+
EqtTrace.Verbose("MSTest.AssemblyResolver.OnResolve: Searching assembly '{0}' in the directory '{1}'", requestedName.Name, dir);
363364
}
364365
});
365366

366367
foreach (string extension in new string[] { ".dll", ".exe" })
367368
{
368369
string assemblyPath = Path.Combine(dir, requestedName.Name + extension);
369370

371+
bool isPushed = false;
372+
bool isResource = requestedName.Name.EndsWith(".resources", StringComparison.InvariantCulture);
373+
if (isResource)
374+
{
375+
// Check for recursive resource lookup.
376+
// This can happen when we are on non-english locale, and we try to load mscorlib.resources
377+
// (or potentially some other resources). This will trigger a new Resolve and call the method
378+
// we are currently in. If then some code in this Resolve method (like File.Exists) will again
379+
// try to access mscorlib.resources it will end up recursing forever.
380+
if (_currentlyResolvingResources != null && _currentlyResolvingResources.Count > 0 && _currentlyResolvingResources.Contains(assemblyPath))
381+
{
382+
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Assembly '{0}' is searching for itself recursively '{1}', returning as not found.", name, assemblyPath);
383+
_resolvedAssemblies[name] = null;
384+
return null;
385+
}
386+
387+
_currentlyResolvingResources ??= new Stack<string>(4);
388+
_currentlyResolvingResources.Push(assemblyPath);
389+
isPushed = true;
390+
}
391+
370392
Assembly? assembly = SearchAndLoadAssembly(assemblyPath, name, requestedName, isReflectionOnly);
393+
if (isResource && isPushed)
394+
{
395+
_currentlyResolvingResources?.Pop();
396+
}
397+
371398
if (assembly != null)
372399
{
373400
return assembly;
@@ -447,7 +474,7 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
447474
{
448475
if (StringEx.IsNullOrEmpty(args.Name))
449476
{
450-
Debug.Fail("AssemblyResolver.OnResolve: args.Name is null or empty.");
477+
Debug.Fail("MSTest.AssemblyResolver.OnResolve: args.Name is null or empty.");
451478
return null;
452479
}
453480

@@ -457,7 +484,7 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
457484
{
458485
if (EqtTrace.IsInfoEnabled)
459486
{
460-
EqtTrace.Info("AssemblyResolver: Resolving assembly: {0}.", args.Name);
487+
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Resolving assembly '{0}'", args.Name);
461488
}
462489
});
463490

@@ -469,7 +496,7 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
469496
{
470497
if (EqtTrace.IsInfoEnabled)
471498
{
472-
EqtTrace.Info("AssemblyResolver: Resolving assembly after applying policy: {0}.", assemblyNameToLoad);
499+
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Resolving assembly after applying policy '{0}'", assemblyNameToLoad);
473500
}
474501
});
475502

@@ -488,56 +515,55 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
488515
return assembly;
489516
}
490517

491-
if (_directoryList != null && _directoryList.Count != 0)
492-
{
493-
// required assembly is not present in searchDirectories??
494-
// see, if we can find it in user specified search directories.
495-
while (assembly == null && _directoryList.Count > 0)
496-
{
497-
// instead of loading whole search directory in one time, we are adding directory on the basis of need
498-
RecursiveDirectoryPath currentNode = _directoryList.Dequeue();
499-
500-
List<string> incrementalSearchDirectory = [];
518+
bool isResource = assemblyNameToLoad.EndsWith(".resources", StringComparison.InvariantCulture);
501519

502-
if (DoesDirectoryExist(currentNode.DirectoryPath))
503-
{
504-
incrementalSearchDirectory.Add(currentNode.DirectoryPath);
520+
// required assembly is not present in searchDirectories??
521+
// see, if we can find it in user specified search directories.
522+
while (assembly == null && _directoryList?.Count > 0)
523+
{
524+
// instead of loading whole search directory in one time, we are adding directory on the basis of need
525+
RecursiveDirectoryPath currentNode = _directoryList.Dequeue();
505526

506-
if (currentNode.IncludeSubDirectories)
507-
{
508-
// Add all its sub-directory in depth first search order.
509-
AddSubdirectories(currentNode.DirectoryPath, incrementalSearchDirectory);
510-
}
527+
List<string> incrementalSearchDirectory = [];
511528

512-
// Add this directory list in this.searchDirectories so that when we will try to resolve some other
513-
// assembly, then it will look in this whole directory first.
514-
_searchDirectories.AddRange(incrementalSearchDirectory);
529+
if (DoesDirectoryExist(currentNode.DirectoryPath))
530+
{
531+
incrementalSearchDirectory.Add(currentNode.DirectoryPath);
515532

516-
assembly = SearchAssembly(incrementalSearchDirectory, assemblyNameToLoad, isReflectionOnly);
517-
}
518-
else
533+
if (currentNode.IncludeSubDirectories)
519534
{
520-
// generate warning that path does not exist.
521-
SafeLog(
522-
assemblyNameToLoad,
523-
() =>
524-
{
525-
if (EqtTrace.IsWarningEnabled)
526-
{
527-
EqtTrace.Warning(
528-
"The Directory: {0}, does not exist",
529-
currentNode.DirectoryPath);
530-
}
531-
});
535+
// Add all its sub-directory in depth first search order.
536+
AddSubdirectories(currentNode.DirectoryPath, incrementalSearchDirectory);
532537
}
533-
}
534538

535-
if (assembly != null)
539+
// Add this directory list in this.searchDirectories so that when we will try to resolve some other
540+
// assembly, then it will look in this whole directory first.
541+
_searchDirectories.AddRange(incrementalSearchDirectory);
542+
543+
assembly = SearchAssembly(incrementalSearchDirectory, assemblyNameToLoad, isReflectionOnly);
544+
}
545+
else
536546
{
537-
return assembly;
547+
// generate warning that path does not exist.
548+
SafeLog(
549+
assemblyNameToLoad,
550+
() =>
551+
{
552+
if (EqtTrace.IsWarningEnabled)
553+
{
554+
EqtTrace.Warning(
555+
"MSTest.AssemblyResolver.OnResolve: the directory '{0}', does not exist",
556+
currentNode.DirectoryPath);
557+
}
558+
});
538559
}
539560
}
540561

562+
if (assembly != null)
563+
{
564+
return assembly;
565+
}
566+
541567
// Try for default load for System dlls that can't be found in search paths. Needs to loaded just by name.
542568
try
543569
{
@@ -580,7 +606,7 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
580606
{
581607
if (EqtTrace.IsInfoEnabled)
582608
{
583-
EqtTrace.Info("AssemblyResolver: {0}: Failed to load assembly. Reason: {1}", assemblyNameToLoad, ex);
609+
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Failed to load assembly '{0}'. Reason: {1}", assemblyNameToLoad, ex);
584610
}
585611
});
586612
}
@@ -609,7 +635,7 @@ private bool TryLoadFromCache(string assemblyName, bool isReflectionOnly, out As
609635
{
610636
if (EqtTrace.IsInfoEnabled)
611637
{
612-
EqtTrace.Info("AssemblyResolver: Resolved: {0}.", assemblyName);
638+
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Resolved '{0}'", assemblyName);
613639
}
614640
});
615641
return true;
@@ -685,7 +711,7 @@ private static void SafeLog(string? assemblyName, Action loggerAction)
685711
{
686712
if (EqtTrace.IsInfoEnabled)
687713
{
688-
EqtTrace.Info("AssemblyResolver: Resolved assembly: {0}. ", assemblyName);
714+
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Resolved assembly '{0}'", assemblyName);
689715
}
690716
});
691717

@@ -699,7 +725,7 @@ private static void SafeLog(string? assemblyName, Action loggerAction)
699725
{
700726
if (EqtTrace.IsInfoEnabled)
701727
{
702-
EqtTrace.Info("AssemblyResolver: Failed to load assembly: {0}. Reason:{1} ", assemblyName, ex);
728+
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Failed to load assembly '{0}'. Reason:{1} ", assemblyName, ex);
703729
}
704730
});
705731

@@ -717,7 +743,7 @@ private static void SafeLog(string? assemblyName, Action loggerAction)
717743
{
718744
if (EqtTrace.IsInfoEnabled)
719745
{
720-
EqtTrace.Info("AssemblyResolver: Failed to load assembly: {0}. Reason:{1} ", assemblyName, ex);
746+
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Failed to load assembly '{0}'. Reason:{1} ", assemblyName, ex);
721747
}
722748
});
723749
}

test/IntegrationTests/MSTest.Acceptance.IntegrationTests/AssemblyResolverTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ namespace MSTest.Acceptance.IntegrationTests;
1010
[TestGroup]
1111
public class AssemblyResolverTests : AcceptanceTestBase
1212
{
13-
private readonly TestAssetFixture _testAssetFixture;
1413
private const string AssetName = "AssemblyResolverCrash";
14+
private readonly TestAssetFixture _testAssetFixture;
1515

1616
// There's a bug in TAFX where we need to use it at least one time somewhere to use it inside the fixture self (AcceptanceFixture).
1717
public AssemblyResolverTests(ITestExecutionContext testExecutionContext, TestAssetFixture testAssetFixture,

0 commit comments

Comments
 (0)