diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Reliability/SqlConfigurableRetryLogicLoader.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Reliability/SqlConfigurableRetryLogicLoader.cs index 96eadb9d56..df556a49ba 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Reliability/SqlConfigurableRetryLogicLoader.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Reliability/SqlConfigurableRetryLogicLoader.cs @@ -50,14 +50,6 @@ public SqlConfigurableRetryLogicLoader( string cnnSectionName = SqlConfigurableRetryConnectionSection.Name, string cmdSectionName = SqlConfigurableRetryCommandSection.Name) { - #if NET - // Just only one subscription to this event is required. - // This class isn't supposed to be called more than one time; - // SqlConfigurableRetryLogicManager manages a single instance of this class. - System.Runtime.Loader.AssemblyLoadContext.Default.Resolving -= Default_Resolving; - System.Runtime.Loader.AssemblyLoadContext.Default.Resolving += Default_Resolving; - #endif - AssignProviders(connectionRetryConfigs == null ? null : CreateRetryLogicProvider(cnnSectionName, connectionRetryConfigs), commandRetryConfigs == null ? null : CreateRetryLogicProvider(cmdSectionName, commandRetryConfigs)); } @@ -119,44 +111,70 @@ private static SqlRetryLogicBaseProvider ResolveRetryLogicProvider(string config throw new ArgumentNullException(nameof(retryMethod), StringsHelper.GetString(Strings.SQLCR_RetryMethodNullOrEmpty)); } - Type type = null; - try - { - // Resolve a Type object from the given type name - // Different implementation in .NET Framework & .NET Core - type = LoadType(configurableRetryType); - } - catch (Exception e) - { - // Try to use 'SqlConfigurableRetryFactory' as a default type to discover retry methods - // if there is a problem, resolve using the 'configurableRetryType' type. - type = typeof(SqlConfigurableRetryFactory); - SqlClientEventSource.Log.TryTraceEvent(" Unable to load the '{2}' type; Trying to use the internal `{3}` type: {4}", - TypeName, methodName, configurableRetryType, type.FullName, e); - } + Type type; - // Run the function by using the resolved values to get the SqlRetryLogicBaseProvider object - try + // Whitespace is treated the same as unset: there is no type to resolve, so the + // resolving handler must not be installed for it. + bool customRetryTypeConfigured = !string.IsNullOrWhiteSpace(configurableRetryType); + + // Keep the handler subscribed across both type resolution and provider construction. + // Invoking the configured type's constructor and retry method can load that assembly's + // private dependencies after LoadType has returned. + using (AssemblyResolutionSubscription subscription = new(customRetryTypeConfigured)) { - // Create an instance from the discovered type by its default constructor - object result = CreateInstance(type, retryMethod, option); + if (!customRetryTypeConfigured) + { + // No custom retry logic type was configured, so there is nothing to resolve and + // the built-in factory is used to discover the requested retry method. + type = typeof(SqlConfigurableRetryFactory); + SqlClientEventSource.Log.TryTraceEvent(" No custom retry logic type is configured; Using the internal `{2}` type.", + TypeName, methodName, type.FullName); + } + else + { + try + { + // Resolve a Type object from the given type name + // Different implementation in .NET Framework & .NET Core + type = LoadType(configurableRetryType); + } + catch (Exception e) + { + // The custom type will not be constructed, so the built-in fallback no + // longer needs the custom assembly resolution handler. + subscription.Dispose(); + + // Try to use 'SqlConfigurableRetryFactory' as a default type to discover retry methods + // if there is a problem, resolve using the 'configurableRetryType' type. + type = typeof(SqlConfigurableRetryFactory); + SqlClientEventSource.Log.TryTraceEvent(" Unable to load the '{2}' type; Trying to use the internal `{3}` type: {4}", + TypeName, methodName, configurableRetryType, type.FullName, e); + } + } - if (result is SqlRetryLogicBaseProvider provider) + // Run the function by using the resolved values to get the SqlRetryLogicBaseProvider object + try { - SqlClientEventSource.Log.TryTraceEvent(" The created instace is a {2} type.", - TypeName, methodName, typeof(SqlRetryLogicBaseProvider).FullName); - provider.Retrying += OnRetryingEvent; - return provider; + // Create an instance from the discovered type by its default constructor + object result = CreateInstance(type, retryMethod, option); + + if (result is SqlRetryLogicBaseProvider provider) + { + SqlClientEventSource.Log.TryTraceEvent(" The created instace is a {2} type.", + TypeName, methodName, typeof(SqlRetryLogicBaseProvider).FullName); + provider.Retrying += OnRetryingEvent; + return provider; + } + } + catch (Exception e) + { + // In order to invoke a function dynamically, any type of exception can occur here; + // The main exception and its stack trace will be accessible through the inner exception. + // i.e: Opening a connection or executing a command while invoking a function + // runs the application to the `TargetInvocationException`. + // And using an isolated zone like a specific AppDomain results in an infinite loop. + throw new InvalidOperationException(StringsHelper.GetString(Strings.SQLCR_RetryMethodException, type.FullName, retryMethod), e); } - } - catch (Exception e) - { - // In order to invoke a function dynamically, any type of exception can occur here; - // The main exception and its stack trace will be accessible through the inner exception. - // i.e: Opening a connection or executing a command while invoking a function - // runs the application to the `TargetInvocationException`. - // And using an isolated zone like a specific AppDomain results in an infinite loop. - throw new InvalidOperationException(StringsHelper.GetString(Strings.SQLCR_RetryMethodException, type.FullName, retryMethod), e); } SqlClientEventSource.Log.TryTraceEvent(" Unable to resolve a valid provider; Returns `null`.", TypeName, methodName); @@ -334,13 +352,53 @@ private static ICollection SplitErrorNumberList(string list) } #region Type Resolution - + + internal sealed class AssemblyResolutionSubscription : IDisposable + { + #if NET + private bool _isSubscribed; + #endif + + internal AssemblyResolutionSubscription(bool subscribe) + { + #if NET + if (subscribe) + { + AssemblyLoadContext.Default.Resolving += Default_Resolving; + _isSubscribed = true; + } + #endif + } + + public void Dispose() + { + #if NET + if (_isSubscribed) + { + AssemblyLoadContext.Default.Resolving -= Default_Resolving; + _isSubscribed = false; + } + #endif + } + } + #if NET + /// + /// The directory that user-supplied configurable retry logic assemblies are probed from. + /// + /// + /// This is deliberately the application base directory rather than the current working + /// directory. The working directory is ambient process state that can be changed at any + /// time and is not necessarily related to where the application's binaries live, so + /// probing it can load assemblies from an unintended and untrusted location. + /// + private static string ProbingDirectory => AppContext.BaseDirectory; + private static Assembly AssemblyResolver(AssemblyName arg) { string methodName = nameof(AssemblyResolver); - string fullPath = MakeFullPath(Environment.CurrentDirectory, arg.Name); + string fullPath = MakeFullPath(ProbingDirectory, arg.Name); SqlClientEventSource.Log.TryTraceEvent(" Looking for '{2}' assembly by '{3}' full path." , TypeName, methodName, arg, fullPath); @@ -348,13 +406,19 @@ private static Assembly AssemblyResolver(AssemblyName arg) } /// - /// Load assemblies on request. + /// Load dependencies of a user-supplied configurable retry logic assembly on request. /// + /// + /// This handler is only subscribed while a configured retry logic provider is being + /// resolved and constructed, and only when a custom retry logic type has been configured. + /// It must not remain subscribed to after that, + /// because doing so changes assembly resolution behavior for the entire application. + /// private static Assembly Default_Resolving(AssemblyLoadContext arg1, AssemblyName arg2) { string methodName = nameof(Default_Resolving); - string target = MakeFullPath(Environment.CurrentDirectory, arg2.Name); + string target = MakeFullPath(ProbingDirectory, arg2.Name); SqlClientEventSource.Log.TryTraceEvent(" Looking for '{2}' assembly that is requested by '{3}' ALC from '{4}' path." , TypeName, methodName, arg2, arg1, target); @@ -371,7 +435,10 @@ private static Type LoadType(string fullyQualifiedName) string methodName = nameof(LoadType); SqlClientEventSource.Log.TryTraceEvent(" Entry point.", TypeName, methodName); - var result = Type.GetType(fullyQualifiedName, AssemblyResolver, TypeResolver); + Type result; + + result = Type.GetType(fullyQualifiedName, AssemblyResolver, TypeResolver); + if (result != null) { SqlClientEventSource.Log.TryTraceEvent(" The '{2}' type is resolved.", diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConfigurableRetryLogicTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConfigurableRetryLogicTest.cs index b04115b191..b24a32e804 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConfigurableRetryLogicTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConfigurableRetryLogicTest.cs @@ -3,6 +3,8 @@ // See the LICENSE file in the project root for more information. using System; +using System.IO; +using System.Reflection; using System.Threading.Tasks; using Xunit; @@ -80,5 +82,49 @@ public void ValidateRetryParameters() option.AuthorizedSqlCondition = null; SqlConfigurableRetryFactory.CreateIncrementalRetryProvider(option); } + +#if NET + /// + /// Regression test: triggering the configurable retry logic loader through its normal + /// entry points must not leave a process-wide + /// resolving handler + /// installed. Such a handler participates in resolution of every assembly the host + /// application fails to find, and serves them out of this component's probing directory, + /// which can load code from an unintended location. + /// + [Fact] + public void RetryLogicProviderDoesNotLeaveAssemblyProbingEnabled() + { + // Touch the default retry logic providers to force SqlConfigurableRetryLogicLoader + // construction via its normal code path. + Assert.NotNull(new SqlCommand().RetryLogicProvider); + Assert.NotNull(new SqlConnection().RetryLogicProvider); + + // A file that is not a valid assembly, planted in the loader's probing directory + // under a name no other component could be asking for. If a handler is still + // subscribed it finds this file and fails with BadImageFormatException. With correct + // behavior the runtime never looks here and reports the assembly as simply not found. + string assemblySimpleName = "MdsProbeAssembly_" + Guid.NewGuid().ToString("N"); + string plantedFile = Path.Combine(AppContext.BaseDirectory, assemblySimpleName + ".dll"); + + File.WriteAllText(plantedFile, "not an assembly"); + try + { + Assert.Throws( + () => Assembly.Load(new AssemblyName(assemblySimpleName))); + } + finally + { + try + { + File.Delete(plantedFile); + } + catch (IOException) + { + // Best effort cleanup. + } + } + } +#endif } } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConfigurableRetryLogicLoaderTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConfigurableRetryLogicLoaderTest.cs new file mode 100644 index 0000000000..d4ef506991 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConfigurableRetryLogicLoaderTest.cs @@ -0,0 +1,388 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#if NET + +using System; +using System.IO; +using System.Reflection; +using System.Runtime.Loader; +using Microsoft.Data.SqlClient.Tests.Common; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests; + +/// +/// Unit tests validating that never leaves a +/// process-wide assembly resolving handler attached to +/// . +/// +/// +/// A handler left attached there participates in resolution of every assembly the host +/// application fails to find, not just the retry logic assembly the loader was interested in. +/// That silently changes assembly loading behaviour for code that never opted in to this +/// feature, and can serve unrelated assemblies out of this component's probing directory. +/// +[Collection(AppContextSwitchTestCollection.Name)] +public class SqlConfigurableRetryLogicLoaderTest +{ + /// + /// The default code path: no configuration at all. The loader must not subscribe to the + /// default load context. + /// + [Fact] + public void Constructor_WithNoConfiguration_DoesNotLeaveAssemblyProbingEnabled() + { + _ = new SqlConfigurableRetryLogicLoader(null, null); + + AssertNoAssemblyProbingHandlerInstalled(); + } + + /// + /// A configuration that supplies only a retry method, which is the documented way to select + /// one of the built-in retry providers. No custom type is being requested, so no assembly + /// resolution is required and no handler may be subscribed - not even transiently. + /// + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Constructor_WithoutRetryLogicType_DoesNotLeaveAssemblyProbingEnabled(string? retryLogicType) + { + TestRetryConnectionSection section = CreateSection(retryLogicType); + + SqlConfigurableRetryLogicLoader loader = new(section, null); + + // The built-in factory still resolves the requested method. + Assert.NotNull(loader.ConnectionProvider); + AssertNoAssemblyProbingHandlerInstalled(); + } + + /// + /// A configuration that requests a custom retry logic type legitimately needs assembly + /// resolution, but the handler must be removed again once type resolution has finished. + /// + [Fact] + public void Constructor_WithUnresolvableRetryLogicType_DoesNotLeaveAssemblyProbingEnabled() + { + TestRetryConnectionSection section = + CreateSection("Some.Namespace.NoSuchType, Some.Assembly.That.Does.Not.Exist"); + + SqlConfigurableRetryLogicLoader loader = new(section, null); + + // Resolution fails and falls back to the built-in factory rather than throwing. + Assert.NotNull(loader.ConnectionProvider); + AssertNoAssemblyProbingHandlerInstalled(); + } + + /// + /// The path a custom retry logic type is actually resolved on. This is the only path that + /// legitimately installs the probing handler, and it must be removed again once resolution + /// has finished. + /// + /// + /// The retry logic type is resolved out of this test assembly, which is reached through the + /// loader's probing directory rather than through normal assembly resolution. The invocation + /// counter confirms the configured type really was resolved and used, so this is exercising + /// the successful branch of type resolution rather than silently falling back to the built-in + /// factory. + /// + [Fact] + public void Constructor_WithResolvableRetryLogicType_DoesNotLeaveAssemblyProbingEnabled() + { + RunWithProbedRetryLogicFactory(loader => + { + Assert.NotNull(loader.ConnectionProvider); + AssertNoAssemblyProbingHandlerInstalled(); + }); + } + + /// + /// The probing handler must stay subscribed until the configured provider has been fully + /// constructed, not just until the type has been resolved. + /// + /// + /// Instantiating the configured type and invoking its retry method can trigger loads of that + /// assembly's private dependencies, and those loads happen after type resolution has already + /// returned. Unsubscribing too early would break providers that depend on that behaviour. The + /// factory method records the state of the handler at the moment it runs, which is inside the + /// window that has to remain open. + /// + [Fact] + public void RetryLogicTypeResolution_KeepsAssemblyProbingEnabledWhileProviderIsConstructed() + { + string probeAssemblyName = NewProbeAssemblyName(); + string plantedFile = PlantProbeFile(probeAssemblyName); + + ProbedRetryLogicFactory.ProbeAssemblyName = probeAssemblyName; + ProbedRetryLogicFactory.ProbingHandlerInstalledDuringInvocation = null; + + try + { + RunWithProbedRetryLogicFactory(loader => + { + Assert.NotNull(loader.ConnectionProvider); + + Assert.True( + ProbedRetryLogicFactory.ProbingHandlerInstalledDuringInvocation, + "The assembly probing handler was not subscribed while the configured retry " + + "logic provider was being constructed."); + }); + + // ...and it must be gone again afterwards. + AssertNoAssemblyProbingHandlerInstalled(); + } + finally + { + ProbedRetryLogicFactory.ProbeAssemblyName = null; + ProbedRetryLogicFactory.ProbingHandlerInstalledDuringInvocation = null; + DeleteProbeFile(plantedFile); + } + } + + /// + /// Disposing an + /// removes its handler from the default assembly load context. + /// + [Fact] + public void AssemblyResolutionSubscription_DisposeRemovesAssemblyProbingHandler() + { + string subscribedProbeName = NewProbeAssemblyName(); + string subscribedProbeFile = PlantProbeFile(subscribedProbeName); + string disposedProbeName = NewProbeAssemblyName(); + string disposedProbeFile = PlantProbeFile(disposedProbeName); + + try + { + using SqlConfigurableRetryLogicLoader.AssemblyResolutionSubscription subscription = + new(subscribe: true); + + Assert.True(IsProbingHandlerInstalled(subscribedProbeName)); + + subscription.Dispose(); + + Assert.False(IsProbingHandlerInstalled(disposedProbeName)); + } + finally + { + DeleteProbeFile(subscribedProbeFile); + DeleteProbeFile(disposedProbeFile); + } + } + + /// + /// Builds a configuration that resolves out of this test + /// assembly through the loader's probing directory, constructs a loader from it, and hands the + /// loader to . + /// + private static void RunWithProbedRetryLogicFactory(Action assert) + { + Assembly testAssembly = typeof(SqlConfigurableRetryLogicLoaderTest).Assembly; + string assemblySimpleName = testAssembly.GetName().Name!; + + // The loader probes for '.dll'. The test assembly's file name does not + // necessarily match its simple name, so make a copy that does. + string probePath = Path.Combine(AppContext.BaseDirectory, assemblySimpleName + ".dll"); + bool copied = false; + if (!File.Exists(probePath)) + { + File.Copy(testAssembly.Location, probePath); + copied = true; + } + + try + { + TestRetryConnectionSection section = CreateSection( + $"{typeof(ProbedRetryLogicFactory).FullName}, {assemblySimpleName}"); + section.RetryMethod = nameof(ProbedRetryLogicFactory.CreateProbedRetryProvider); + + ProbedRetryLogicFactory.InvocationCount = 0; + + SqlConfigurableRetryLogicLoader loader = new(section, null); + + Assert.Equal(1, ProbedRetryLogicFactory.InvocationCount); + + assert(loader); + } + finally + { + if (copied) + { + DeleteProbeFile(probePath); + } + } + } + + private static TestRetryConnectionSection CreateSection(string? retryLogicType) => + new() + { + RetryLogicType = retryLogicType!, + RetryMethod = nameof(SqlConfigurableRetryFactory.CreateFixedRetryProvider), + NumberOfTries = 2, + DeltaTime = TimeSpan.FromSeconds(1), + MinTimeInterval = TimeSpan.Zero, + MaxTimeInterval = TimeSpan.FromSeconds(10), + }; + + /// + /// Asserts that a failed assembly load is not served out of the loader's probing directory, + /// which can only happen while a resolving handler installed by + /// is subscribed to + /// . + /// + /// + /// A file that is not a valid assembly is planted in the probing directory under a name no + /// other component could be asking for. If a handler is still subscribed it finds that file + /// and tries to load it, which surfaces as . With no + /// handler subscribed the runtime never looks there and reports the assembly as simply not + /// found. This asserts the behaviour that actually matters to a host application rather than + /// inspecting loader or runtime internals. + /// + private static void AssertNoAssemblyProbingHandlerInstalled() + { + string assemblySimpleName = NewProbeAssemblyName(); + string plantedFile = PlantProbeFile(assemblySimpleName); + + try + { + Assert.False( + IsProbingHandlerInstalled(assemblySimpleName), + "A resolving handler that probes the loader's probing directory is subscribed to " + + "the default assembly load context."); + } + finally + { + DeleteProbeFile(plantedFile); + } + } + + /// + /// Reports whether a resolving handler that serves assemblies out of the loader's probing + /// directory is currently subscribed to . + /// + /// + /// This distinguishes the two states using only public behaviour, which is what actually + /// matters to a host application. With such a handler subscribed the planted file is found + /// and an attempt is made to load it, which fails as + /// because it is not a valid assembly. With no such + /// handler subscribed the runtime never looks in that directory and reports the assembly as + /// simply not found. + /// + internal static bool IsProbingHandlerInstalled(string assemblySimpleName) + { + try + { + Assembly.Load(new AssemblyName(assemblySimpleName)); + + // Unreachable: the planted file is deliberately not a valid assembly, so a handler + // that found it cannot have loaded it successfully. + return true; + } + catch (BadImageFormatException) + { + return true; + } + catch (FileNotFoundException) + { + return false; + } + } + + private static string NewProbeAssemblyName() => + "MdsProbeAssembly_" + Guid.NewGuid().ToString("N"); + + /// + /// Plants a file that is not a valid assembly in the loader's probing directory, under a + /// name no other component could be asking for. + /// + private static string PlantProbeFile(string assemblySimpleName) + { + // The probing directory is the application base directory, which for a test run is the + // directory the test assembly was loaded from. + string plantedFile = Path.Combine(AppContext.BaseDirectory, assemblySimpleName + ".dll"); + + File.WriteAllText(plantedFile, "not an assembly"); + + return plantedFile; + } + + /// + /// Removes a planted probe file. Cleanup failures are ignored so that a test reports on + /// product behaviour rather than on the state of the file system. + /// + private static void DeleteProbeFile(string plantedFile) + { + try + { + File.Delete(plantedFile); + } + catch (IOException) + { + // The file is in use, most likely because it was successfully loaded as an assembly + // and is therefore locked for the lifetime of the process. + } + catch (UnauthorizedAccessException) + { + // The file is read only, or the caller lacks permission to delete it. + } + } + + private sealed class TestRetryConnectionSection : ISqlConfigurableRetryConnectionSection + { + public TimeSpan DeltaTime { get; set; } + + public TimeSpan MaxTimeInterval { get; set; } + + public TimeSpan MinTimeInterval { get; set; } + + public int NumberOfTries { get; set; } + + public string RetryLogicType { get; set; } = string.Empty; + + public string RetryMethod { get; set; } = string.Empty; + + public string TransientErrors { get; set; } = string.Empty; + } +} + +/// +/// A retry logic factory that is resolved through the loader's probing directory rather than +/// through normal assembly resolution, so tests can tell a successful custom type resolution +/// apart from a silent fallback to the built-in factory. +/// +/// +/// This has to be a public, non-nested type because the loader discovers candidates by +/// enumerating the resolved assembly's exported types. +/// +public static class ProbedRetryLogicFactory +{ + internal static int InvocationCount; + + /// + /// When set to the simple name of a planted probe assembly, the factory method records whether + /// the loader's probing handler was subscribed at the moment it ran. + /// + internal static string? ProbeAssemblyName; + + /// + /// The state of the loader's probing handler at the moment the factory method last ran, or null + /// if it was not recorded. + /// + internal static bool? ProbingHandlerInstalledDuringInvocation; + + public static SqlRetryLogicBaseProvider CreateProbedRetryProvider(SqlRetryLogicOption option) + { + InvocationCount++; + + if (ProbeAssemblyName is not null) + { + ProbingHandlerInstalledDuringInvocation = + SqlConfigurableRetryLogicLoaderTest.IsProbingHandlerInstalled(ProbeAssemblyName); + } + + return SqlConfigurableRetryFactory.CreateFixedRetryProvider(option); + } +} + +#endif