Merge pull request #714 from stakx/bugfix/method-invocation-target

stakx · web-flow · commit 3cd489d3cb05 · 2025-12-23T01:40:49.000+01:00
Fix `ArgumentNullException` thrown by `invocation.MethodInvocationTarget` for default interface methods
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ Enhancements:
 
 Bugfixes:
 - `InvalidProgramException` when proxying `MemoryStream` with .NET 7 (@stakx, #651)
+- `invocation.MethodInvocationTarget` throws `ArgumentNullException` for default interface method (@stakx, #684)
 
 Deprecations:
 - .NET Core 2.1, .NET Core 3.1, .NET 6, and mono tests
diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/DefaultInterfaceMembersTestCase.cs
@@ -100,6 +100,28 @@ public void Can_proceed_to_method_default_implementation_in_proxied_interface()
 			Assert.AreEqual(expected, actual);
 		}
 
+		[Test]
+		public void Invocation_InvocationTarget_is_proxy_for_intercepted_method_with_default_implementation()
+		{
+			var interceptor = new KeepDataInterceptor();
+			var proxy = generator.CreateInterfaceProxyWithoutTarget<IHaveMethodWithDefaultImplementation>(interceptor);
+			proxy.MethodWithDefaultImplementation();
+			Assert.AreSame(
+				expected: proxy,
+				actual: interceptor.Invocation.InvocationTarget);
+		}
+
+		[Test]
+		public void Invocation_MethodInvocationTarget_is_intercepted_method_with_default_implementation()
+		{
+			var interceptor = new KeepDataInterceptor();
+			var proxy = generator.CreateInterfaceProxyWithoutTarget<IHaveMethodWithDefaultImplementation>(interceptor);
+			proxy.MethodWithDefaultImplementation();
+			Assert.AreSame(
+				expected: typeof(IHaveMethodWithDefaultImplementation).GetMethod(nameof(IHaveMethodWithDefaultImplementation.MethodWithDefaultImplementation)),
+				actual: interceptor.Invocation.MethodInvocationTarget);
+		}
+
 		#endregion
 
 		#region Generic methods with default implementation
@@ -296,6 +318,28 @@ public void Can_proceed_to_method_default_implementation_from_additional_interfa
 			Assert.AreEqual(expected, actual);
 		}
 
+		[Test]
+		public void Invocation_InvocationTarget_is_proxy_for_intercepted_method_with_default_implementation_from_additional_interface()
+		{
+			var interceptor = new KeepDataInterceptor();
+			var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IEmpty), new[] { typeof(IHaveMethodWithDefaultImplementation) }, interceptor);
+			((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation();
+			Assert.AreSame(
+				expected: proxy,
+				actual: interceptor.Invocation.InvocationTarget);
+		}
+
+		[Test]
+		public void Invocation_MethodInvocationTarget_is_intercepted_method_with_default_implementation_from_additional_interface()
+		{
+			var interceptor = new KeepDataInterceptor();
+			var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IEmpty), new[] { typeof(IHaveMethodWithDefaultImplementation) }, interceptor);
+			((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation();
+			Assert.AreSame(
+				expected: typeof(IHaveMethodWithDefaultImplementation).GetMethod(nameof(IHaveMethodWithDefaultImplementation.MethodWithDefaultImplementation)),
+				actual: interceptor.Invocation.MethodInvocationTarget);
+		}
+
 		#endregion
 
 		#region Non-public methods with default implementation
@@ -457,6 +501,34 @@ public void Can_proceed_to_method_default_implementation_from_mixin_in_proxied_i
 			var actual = ((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation();
 			Assert.AreEqual(expected, actual);
 		}
+
+		[Test]
+		public void Invocation_InvocationTarget_is_mixin_for_intercepted_method_with_default_implementation_from_mixin()
+		{
+			var options = new ProxyGenerationOptions();
+			var mixin = new InheritsMethodWithDefaultImplementation();
+			options.AddMixinInstance(mixin);
+			var interceptor = new KeepDataInterceptor();
+			var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IEmpty), options, interceptor);
+			((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation();
+			Assert.AreSame(
+				expected: mixin,
+				actual: interceptor.Invocation.InvocationTarget);
+		}
+
+		[Test]
+		public void Invocation_MethodInvocationTarget_is_intercepted_method_with_default_implementation_from_mixin()
+		{
+			var options = new ProxyGenerationOptions();
+			options.AddMixinInstance(new InheritsMethodWithDefaultImplementation());
+			var interceptor = new KeepDataInterceptor();
+			var proxy = generator.CreateInterfaceProxyWithoutTarget(typeof(IEmpty), options, interceptor);
+			((IHaveMethodWithDefaultImplementation)proxy).MethodWithDefaultImplementation();
+			Assert.AreSame(
+				expected: typeof(IHaveMethodWithDefaultImplementation).GetMethod(nameof(IHaveMethodWithDefaultImplementation.MethodWithDefaultImplementation)),
+				actual: interceptor.Invocation.MethodInvocationTarget);
+		}
+
 		#endregion
 
 		public interface IHaveGenericMethodWithDefaultImplementation
diff --git a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs
@@ -76,10 +76,16 @@ protected override MethodGenerator GetMethodGenerator(MetaMethod method, ClassEm
 				return ExplicitlyImplementedInterfaceMethodGenerator(method, @class, overrideMethod);
 			}
 
+			// since this contributor is used for class proxies without target,
+			// the invocation type will be derived from `InheritanceInvocation`:
 			var invocation = GetInvocationType(method, @class);
 
 			GetTargetExpressionDelegate getTargetTypeExpression = (c, m) => new TypeTokenExpression(targetType);
 
+			// `MethodWithInvocationGenerator` uses its `getTargetExpression` argument
+			// to determine the first argument to be passed to the received invocation type;
+			// and since `InheritanceInvocation`s' first ctor param is `targetType`,
+			// we pass `getTargetTypeExpression` here:
 			return new MethodWithInvocationGenerator(method,
 			                                         @class.GetField("__interceptors"),
 			                                         invocation,
diff --git a/src/Castle.Core/DynamicProxy/Generators/BaseClassProxyGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/BaseClassProxyGenerator.cs
@@ -153,7 +153,11 @@ private IEnumerable<Type> GetTypeImplementerMapping(out IEnumerable<ITypeContrib
 			// 3. then additional interfaces
 			if (interfaces.Length > 0)
 			{
-				var additionalInterfacesContributor = new InterfaceProxyWithoutTargetContributor(namingScope, (c, m) => NullExpression.Instance) { Logger = Logger };
+				// this is explained in `InterfaceProxyWithoutTargetGenerator.GetProxyTargetContributor`:
+				GetTargetExpressionDelegate getTargetType =
+					(c, m) => m.IsAbstract ? NullExpression.Instance
+										   : new TypeTokenExpression(m.DeclaringType);
+				var additionalInterfacesContributor = new InterfaceProxyWithoutTargetContributor(namingScope, getTargetType) { Logger = Logger };
 				contributorsList.Add(additionalInterfacesContributor);
 
 				foreach (var @interface in interfaces)
diff --git a/src/Castle.Core/DynamicProxy/Generators/BaseInterfaceProxyGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/BaseInterfaceProxyGenerator.cs
@@ -145,7 +145,11 @@ protected override Type GenerateType(string typeName, INamingScope namingScope)
 		protected virtual InterfaceProxyWithoutTargetContributor GetContributorForAdditionalInterfaces(
 			INamingScope namingScope)
 		{
-			return new InterfaceProxyWithoutTargetContributor(namingScope, (c, m) => NullExpression.Instance) { Logger = Logger };
+			// this is explained in `InterfaceProxyWithoutTargetGenerator.GetProxyTargetContributor`:
+			GetTargetExpressionDelegate getTargetType =
+				(c, m) => m.IsAbstract ? NullExpression.Instance
+				                       : new TypeTokenExpression(m.DeclaringType);
+			return new InterfaceProxyWithoutTargetContributor(namingScope, getTargetType) { Logger = Logger };
 		}
 
 		protected virtual IEnumerable<Type> GetTypeImplementerMapping(Type proxyTargetType,
diff --git a/src/Castle.Core/DynamicProxy/Generators/InterfaceProxyWithoutTargetGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/InterfaceProxyWithoutTargetGenerator.cs
@@ -35,7 +35,16 @@ public InterfaceProxyWithoutTargetGenerator(ModuleScope scope, Type targetType,
 
 		protected override CompositeTypeContributor GetProxyTargetContributor(Type proxyTargetType, INamingScope namingScope)
 		{
-			return new InterfaceProxyWithoutTargetContributor(namingScope, (c, m) => NullExpression.Instance) { Logger = Logger };
+			// The type of contributor instantiated below will use an inheritance-based invocation type.
+			// Those expect a target type, not a target instance (see first ctor param of `InheritanceInvocation`).
+			// For interface proxies without target, there typically isn't a target at all...
+			// except when an interface method has a default implementation (i.e. isn't abstract).
+			// Then the target type is the method's declaring interface itself.
+			// (Similar scenario: class proxies w/o target inherit method impls from the proxied class.)
+			GetTargetExpressionDelegate getTargetType =
+				(c, m) => m.IsAbstract ? NullExpression.Instance
+				                       : new TypeTokenExpression(m.DeclaringType);
+			return new InterfaceProxyWithoutTargetContributor(namingScope, getTargetType) { Logger = Logger };
 		}
 
 		protected override ProxyTargetAccessorContributor GetProxyTargetAccessorContributor()
diff --git a/src/Castle.Core/DynamicProxy/Internal/InvocationHelper.cs b/src/Castle.Core/DynamicProxy/Internal/InvocationHelper.cs
@@ -65,10 +65,17 @@ private static MethodInfo ObtainMethod(MethodInfo proxiedMethod, Type type)
 			MethodInfo methodOnTarget = null;
 			if (declaringType.IsInterface)
 			{
-				var mapping = type.GetInterfaceMap(declaringType);
-				var index = Array.IndexOf(mapping.InterfaceMethods, proxiedMethod);
-				Debug.Assert(index != -1);
-				methodOnTarget = mapping.TargetMethods[index];
+				if (proxiedMethod.IsAbstract)
+				{
+					var mapping = type.GetInterfaceMap(declaringType);
+					var index = Array.IndexOf(mapping.InterfaceMethods, proxiedMethod);
+					Debug.Assert(index != -1);
+					methodOnTarget = mapping.TargetMethods[index];
+				}
+				else
+				{
+					methodOnTarget = proxiedMethod;
+				}
 			}
 			else
 			{

Original file line number	Diff line number	Diff line change
`@@ -145,7 +145,11 @@ protected override Type GenerateType(string typeName, INamingScope namingScope)`
`145`	`145`	`protected virtual InterfaceProxyWithoutTargetContributor GetContributorForAdditionalInterfaces(`
`146`	`146`	`INamingScope namingScope)`
`147`	`147`	`{`
`148`		`- return new InterfaceProxyWithoutTargetContributor(namingScope, (c, m) => NullExpression.Instance) { Logger = Logger };`
	`148`	+ // this is explained in `InterfaceProxyWithoutTargetGenerator.GetProxyTargetContributor`:
	`149`	`+ GetTargetExpressionDelegate getTargetType =`
	`150`	`+ (c, m) => m.IsAbstract ? NullExpression.Instance`
	`151`	`+ : new TypeTokenExpression(m.DeclaringType);`
	`152`	`+ return new InterfaceProxyWithoutTargetContributor(namingScope, getTargetType) { Logger = Logger };`
`149`	`153`	`}`
`150`	`154`
`151`	`155`	`protected virtual IEnumerable<Type> GetTypeImplementerMapping(Type proxyTargetType,`