Merge pull request #571 from stakx/forward-non-intercepted-methods-to…

…-target

Forward non-intercepted methods on class proxies to target

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 Enhancements:
 - .NET Standard 2.0 and 2.1 support (@lg2de, #485)
+- Non-intercepted methods on a class proxy with target are now forwarded to the target (@stakx, #571)
 
 Bugfixes:
 - Proxying certain `[Serializable]` classes produces proxy types that fail PEVerify test (@stakx, #367)

src/Castle.Core.Tests/DynamicProxy.Tests/BugsReported/GitHubIssue536.cs

@@ -0,0 +1,63 @@
+// Copyright 2004-2021 Castle Project - http://www.castleproject.org/
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+namespace Castle.DynamicProxy.Tests.BugsReported
+{
+	using System;
+	using System.Reflection;
+	using System.Threading.Tasks;
+
+	using NUnit.Framework;
+
+	[TestFixture]
+	public class GitHubIssue536 : BasePEVerifyTestCase
+	{
+		[Test]
+		public void DynamicProxy_NonIntercepted_Property_Leaked()
+		{
+			var instance = new TestClassForCache();
+			var toProxy = instance.GetType();
+
+			var proxyGenerationOptions = new ProxyGenerationOptions(new TestCacheProxyGenerationHook());
+
+			var generator = new ProxyGenerator();
+			var proxy = generator.CreateClassProxyWithTarget(toProxy,
+				instance,
+				proxyGenerationOptions);
+
+			var accessor = (ITestCacheInterface)proxy;
+			accessor.InstanceProperty = 1;
+
+			Assert.AreEqual(accessor.InstanceProperty, instance.InstanceProperty);
+		}
+
+		public class TestCacheProxyGenerationHook : AllMethodsHook
+		{
+			public override bool ShouldInterceptMethod(Type type, MethodInfo methodInfo)
+			{
+				return false;
+			}
+		}
+
+		public interface ITestCacheInterface
+		{
+			int InstanceProperty { get; set; }
+		}
+
+		public class TestClassForCache : ITestCacheInterface
+		{
+			public virtual int InstanceProperty { get; set; }
+		}
+	}
+}

src/Castle.Core.Tests/DynamicProxy.Tests/Classes/HasVirtualStringAutoProperty.cs

@@ -0,0 +1,21 @@
+// Copyright 2004-2019 Castle Project - http://www.castleproject.org/
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+namespace Castle.DynamicProxy.Tests.Classes
+{
+	public class HasVirtualStringAutoProperty
+	{
+		public virtual string Property { get; set; }
+	}
+}

src/Castle.Core.Tests/DynamicProxy.Tests/NonProxiedTargetMethodsTestCase.cs

@@ -16,6 +16,7 @@ namespace Castle.DynamicProxy.Tests
 {
 	using Castle.DynamicProxy.Tests.Classes;
 	using Castle.DynamicProxy.Tests.Explicit;
+	using Castle.DynamicProxy.Tests.Interceptors;
 	using Castle.DynamicProxy.Tests.InterClasses;
 	using Castle.DynamicProxy.Tests.Interfaces;
 
@@ -119,5 +120,48 @@ public void Target_method_out_ref_parameters_WithTargetInterface()
 			Assert.DoesNotThrow(() => proxy.Did(ref result));
 			Assert.AreEqual(5, result);
 		}
+
+		[Test]
+		public void Unproxied_methods_should_pass_through_to_target()
+		{
+			var target = new HasVirtualStringAutoProperty();
+
+			var options = new ProxyGenerationOptions(
+				hook: new ProxySomeMethodsHook(
+					shouldInterceptMethod: (_, method) => method.Name == "set_" + nameof(HasVirtualStringAutoProperty.Property)));
+
+			var convertToLowerThenProceed = new WithCallbackInterceptor(invocation =>
+			{
+				string value = (string)invocation.GetArgumentValue(0);
+				string lowerCase = value?.ToLowerInvariant();
+				invocation.SetArgumentValue(0, lowerCase);
+				invocation.Proceed();
+			});
+
+			var proxy = generator.CreateClassProxyWithTarget(target, options, convertToLowerThenProceed);
+
+			proxy.Property = "HELLO WORLD";
+
+			Assert.AreEqual("hello world", target.Property);
+			Assert.AreEqual("hello world", proxy.Property);
+		}
+
+		[Test]
+		public void Unproxied_public_method_should_not_invoke_interceptor()
+		{
+			var target = new VirtualClassWithMethod();
+			var options = new ProxyGenerationOptions(new ProxyNothingHook());
+			var proxy = generator.CreateClassProxyWithTarget(target, options, new ThrowingInterceptor());
+			proxy.Method();  // the hook says "don't proxy anything", so this should not call the throwing interceptor
+		}
+
+		[Test]
+		public void Unproxied_non_public_method_should_not_invoke_interceptor()
+		{
+			var target = new ClassWithProtectedMethod();
+			var options = new ProxyGenerationOptions(new ProxyNothingHook());
+			var proxy = generator.CreateClassProxyWithTarget(target, options, new ThrowingInterceptor());
+			proxy.PublicMethod();
+		}
 	}
 }

src/Castle.Core.Tests/DynamicProxy.Tests/ProxySomeMethodsHook.cs

@@ -0,0 +1,41 @@
+// Copyright 2004-2019 Castle Project - http://www.castleproject.org/
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+namespace Castle.DynamicProxy.Tests
+{
+	using System;
+	using System.Reflection;
+
+#if FEATURE_SERIALIZATION
+	[Serializable]
+#endif
+	public class ProxySomeMethodsHook : IProxyGenerationHook
+	{
+		private readonly Func<Type, MethodInfo, bool> shouldInterceptMethod;
+
+		public ProxySomeMethodsHook(Func<Type, MethodInfo, bool> shouldInterceptMethod)
+		{
+			this.shouldInterceptMethod = shouldInterceptMethod;
+		}
+
+		public bool ShouldInterceptMethod(Type type, MethodInfo methodInfo)
+		{
+			return shouldInterceptMethod(type, methodInfo);
+		}
+
+		void IProxyGenerationHook.MethodsInspected() { }
+
+		void IProxyGenerationHook.NonProxyableMemberNotification(Type type, MemberInfo memberInfo) { }
+	}
+}

src/Castle.Core/DynamicProxy/Contributors/ClassProxyWithTargetTargetContributor.cs

@@ -56,13 +56,21 @@ protected override MethodGenerator GetMethodGenerator(MetaMethod method, ClassEm
 				return null;
 			}
 
+			var methodIsDirectlyAccessible = IsDirectlyAccessible(method);
+
 			if (!method.Proxyable)
 			{
-				return new MinimialisticMethodGenerator(method,
-				                                        overrideMethod);
+				if (methodIsDirectlyAccessible)
+				{
+					return new ForwardingMethodGenerator(method, overrideMethod, (c, m) => c.GetField("__target"));
+				}
+				else
+				{
+					return IndirectlyCalledMethodGenerator(method, @class, overrideMethod, skipInterceptors: true);
+				}
 			}
 
-			if (IsDirectlyAccessible(method) == false)
+			if (!methodIsDirectlyAccessible)
 			{
 				return IndirectlyCalledMethodGenerator(method, @class, overrideMethod);
 			}
@@ -137,15 +145,16 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter @class)
 		}
 
 		private MethodGenerator IndirectlyCalledMethodGenerator(MetaMethod method, ClassEmitter proxy,
-		                                                        OverrideMethodDelegate overrideMethod)
+		                                                        OverrideMethodDelegate overrideMethod,
+		                                                        bool skipInterceptors = false)
 		{
 			var @delegate = GetDelegateType(method, proxy);
 			var contributor = GetContributor(@delegate, method);
 			var invocation = new CompositionInvocationTypeGenerator(targetType, method, null, false, contributor)
 				.Generate(proxy, namingScope)
 				.BuildType();
 			return new MethodWithInvocationGenerator(method,
-			                                         proxy.GetField("__interceptors"),
+			                                         skipInterceptors ? NullExpression.Instance : proxy.GetField("__interceptors"),
 			                                         invocation,
 			                                         (c, m) => c.GetField("__target"),
 			                                         overrideMethod,

src/Castle.Core/DynamicProxy/Contributors/MembersCollector.cs

@@ -163,6 +163,18 @@ MetaMethod AddMethod(MethodInfo method, bool isStandalone)
 		///   to select methods.
 		/// </summary>
 		protected bool AcceptMethod(MethodInfo method, bool onlyVirtuals, IProxyGenerationHook hook)
+		{
+			return AcceptMethodPreScreen(method, onlyVirtuals, hook) && hook.ShouldInterceptMethod(type, method);
+		}
+
+		/// <summary>
+		///   Performs some basic screening to filter out non-interceptable methods.
+		/// </summary>
+		/// <remarks>
+		///   The <paramref name="hook"/> will get invoked for non-interceptable method notification only;
+		///   it does not get asked whether or not to intercept the <paramref name="method"/>.
+		/// </remarks>
+		protected bool AcceptMethodPreScreen(MethodInfo method, bool onlyVirtuals, IProxyGenerationHook hook)
 		{
 			if (IsInternalAndNotVisibleToDynamicProxy(method))
 			{
@@ -207,7 +219,7 @@ protected bool AcceptMethod(MethodInfo method, bool onlyVirtuals, IProxyGenerati
 				return false;
 			}
 
-			return hook.ShouldInterceptMethod(type, method);
+			return true;
 		}
 
 		private static bool IsInternalAndNotVisibleToDynamicProxy(MethodInfo method)

src/Castle.Core/DynamicProxy/Contributors/WrappedClassMembersCollector.cs

@@ -42,13 +42,15 @@ protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGener
 				return null;
 			}
 
-			var accepted = AcceptMethod(method, true, hook);
-			if (!accepted && !method.IsAbstract)
+			var interceptable = AcceptMethodPreScreen(method, true, hook);
+			if (!interceptable)
 			{
 				//we don't need to do anything...
 				return null;
 			}
 
+			var accepted = hook.ShouldInterceptMethod(type, method);
+
 			return new MetaMethod(method, method, isStandalone, accepted, hasTarget: true);
 		}
 

src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs

@@ -34,17 +34,17 @@ internal class MethodWithInvocationGenerator : MethodGenerator
 		private readonly IInvocationCreationContributor contributor;
 		private readonly GetTargetExpressionDelegate getTargetExpression;
 		private readonly GetTargetExpressionDelegate getTargetTypeExpression;
-		private readonly Reference interceptors;
+		private readonly IExpression interceptors;
 		private readonly Type invocation;
 
-		public MethodWithInvocationGenerator(MetaMethod method, Reference interceptors, Type invocation,
+		public MethodWithInvocationGenerator(MetaMethod method, IExpression interceptors, Type invocation,
 		                                     GetTargetExpressionDelegate getTargetExpression,
 		                                     OverrideMethodDelegate createMethod, IInvocationCreationContributor contributor)
 			: this(method, interceptors, invocation, getTargetExpression, null, createMethod, contributor)
 		{
 		}
 
-		public MethodWithInvocationGenerator(MetaMethod method, Reference interceptors, Type invocation,
+		public MethodWithInvocationGenerator(MetaMethod method, IExpression interceptors, Type invocation,
 		                                     GetTargetExpressionDelegate getTargetExpression,
 		                                     GetTargetExpressionDelegate getTargetTypeExpression,
 		                                     OverrideMethodDelegate createMethod, IInvocationCreationContributor contributor)