diff --git a/CHANGELOG.md b/CHANGELOG.md index a34e7bd401..94d0a7ab15 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/BugsReported/GitHubIssue536.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/BugsReported/GitHubIssue536.cs new file mode 100644 index 0000000000..e1948cc310 --- /dev/null +++ b/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; } + } + } +} diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/Classes/HasVirtualStringAutoProperty.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/Classes/HasVirtualStringAutoProperty.cs new file mode 100644 index 0000000000..25bee82769 --- /dev/null +++ b/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; } + } +} diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/NonProxiedTargetMethodsTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/NonProxiedTargetMethodsTestCase.cs index b43026a43b..e402796201 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/NonProxiedTargetMethodsTestCase.cs +++ b/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(); + } } } \ No newline at end of file diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/ProxySomeMethodsHook.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/ProxySomeMethodsHook.cs new file mode 100644 index 0000000000..63c5fe77a0 --- /dev/null +++ b/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 shouldInterceptMethod; + + public ProxySomeMethodsHook(Func 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) { } + } +} diff --git a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyWithTargetTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyWithTargetTargetContributor.cs index bd38cd5fa5..e9ab21fc32 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyWithTargetTargetContributor.cs +++ b/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,7 +145,8 @@ 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); @@ -145,7 +154,7 @@ private MethodGenerator IndirectlyCalledMethodGenerator(MetaMethod method, Class .Generate(proxy, namingScope) .BuildType(); return new MethodWithInvocationGenerator(method, - proxy.GetField("__interceptors"), + skipInterceptors ? NullExpression.Instance : proxy.GetField("__interceptors"), invocation, (c, m) => c.GetField("__target"), overrideMethod, diff --git a/src/Castle.Core/DynamicProxy/Contributors/MembersCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/MembersCollector.cs index 36c7c237ff..487a8903f3 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/MembersCollector.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/MembersCollector.cs @@ -163,6 +163,18 @@ MetaMethod AddMethod(MethodInfo method, bool isStandalone) /// to select methods. /// protected bool AcceptMethod(MethodInfo method, bool onlyVirtuals, IProxyGenerationHook hook) + { + return AcceptMethodPreScreen(method, onlyVirtuals, hook) && hook.ShouldInterceptMethod(type, method); + } + + /// + /// Performs some basic screening to filter out non-interceptable methods. + /// + /// + /// The will get invoked for non-interceptable method notification only; + /// it does not get asked whether or not to intercept the . + /// + 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) diff --git a/src/Castle.Core/DynamicProxy/Contributors/WrappedClassMembersCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/WrappedClassMembersCollector.cs index 765f0b689f..eca221461b 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/WrappedClassMembersCollector.cs +++ b/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); } diff --git a/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs index 8f0d955821..9815661106 100644 --- a/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs +++ b/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)