From ac65a6c1a9d1b67d9a41c9ca46ec176bc643cfba Mon Sep 17 00:00:00 2001 From: stakx Date: Sat, 26 May 2018 22:36:05 +0200 Subject: [PATCH 1/3] Add IInterceptorSelector.SelectInterceptors tests These verify that the `type` argument passed to `SelectInterceptors` is equal to the type of the proxy's target. (There are also tests for `IInvocation.TargetType` in order to ensure consistency between `IInvocation` and `IInterceptorSelector`.) One of these tests will fail: When a proxy is created using `Create- ClassProxy`, `SelectInterceptors` will receive a `type` representing `System.RuntimeType` instead of the target's type. --- .../InterceptorSelectorTargetTypeTestCase.cs | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 src/Castle.Core.Tests/DynamicProxy.Tests/InterceptorSelectorTargetTypeTestCase.cs diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/InterceptorSelectorTargetTypeTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/InterceptorSelectorTargetTypeTestCase.cs new file mode 100644 index 0000000000..2ea9956218 --- /dev/null +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/InterceptorSelectorTargetTypeTestCase.cs @@ -0,0 +1,182 @@ +// Copyright 2004-2018 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; + + using Castle.DynamicProxy.Tests.Interceptors; + + using NUnit.Framework; + + [TestFixture] + public class InterceptorSelectorTargetTypeTestCase : BasePEVerifyTestCase + { + // NOTE: This fixture does not just contain tests targeting IInterceptorSelector.SelectInterceptors, + // but also tests targeting IInvocation.TargetType. These tests complement the former set of tests + // to ensure consistency between the two. + + [Test] + public void When_using_CreateClassProxy_SelectInterceptors_receives_type_equal_to_proxied_type() + { + var selector = new InterceptorSelector(); + + var proxy = generator.CreateClassProxy(new ProxyGenerationOptions { Selector = selector }, new DoNothingInterceptor()); + proxy.Method(); + + Assert.AreEqual(typeof(Foo), selector.ReceivedType); + } + + [Test] + public void When_using_CreateClassProxy_Invocation_TargetType_is_equal_to_proxied_type() + { + var interceptor = new Interceptor(); + + var proxy = generator.CreateClassProxy(interceptor); + proxy.Method(); + + Assert.AreEqual(typeof(Foo), interceptor.ReceivedTargetType); + } + + [Test] + public void When_using_CreateClassProxyWithTarget_SelectInterceptors_receives_type_equal_to_type_of_target() + { + var selector = new InterceptorSelector(); + + var proxy = generator.CreateClassProxyWithTarget(new FooTarget(), new ProxyGenerationOptions { Selector = selector }, new DoNothingInterceptor()); + proxy.Method(); + + Assert.AreEqual(typeof(FooTarget), selector.ReceivedType); + } + + [Test] + public void When_using_CreateClassProxyWithTarget_Invocation_TargetType_is_equal_to_type_of_target() + { + var interceptor = new Interceptor(); + + var proxy = generator.CreateClassProxyWithTarget(new FooTarget(), interceptor); + proxy.Method(); + + Assert.AreEqual(typeof(FooTarget), interceptor.ReceivedTargetType); + } + + [Test] + public void When_using_CreateInterfaceProxyWithoutTarget_SelectInterceptors_receives_type_equal_to_null() + { + var selector = new InterceptorSelector(); + + var proxy = generator.CreateInterfaceProxyWithoutTarget(new ProxyGenerationOptions { Selector = selector }, new DoNothingInterceptor()); + proxy.Method(); + + Assert.AreEqual(null, selector.ReceivedType); + } + + [Test] + public void When_using_CreateInterfaceProxyWithoutTarget_Invocation_TargetType_is_equal_to_null() + { + var interceptor = new Interceptor(); + + var proxy = generator.CreateInterfaceProxyWithoutTarget(interceptor); + proxy.Method(); + + Assert.AreEqual(null, interceptor.ReceivedTargetType); + } + + [Test] + public void When_using_CreateInterfaceProxyWithTarget_SelectInterceptors_receives_type_equal_to_type_of_target() + { + var selector = new InterceptorSelector(); + + var proxy = generator.CreateInterfaceProxyWithTarget(new FooTarget(), new ProxyGenerationOptions { Selector = selector }, new DoNothingInterceptor()); + proxy.Method(); + + Assert.AreEqual(typeof(FooTarget), selector.ReceivedType); + } + + [Test] + public void When_using_CreateInterfaceProxyWithTarget_Invocation_TargetType_is_equal_to_type_of_target() + { + var interceptor = new Interceptor(); + + var proxy = generator.CreateInterfaceProxyWithTarget(new FooTarget(), interceptor); + proxy.Method(); + + Assert.AreEqual(typeof(FooTarget), interceptor.ReceivedTargetType); + } + + [Test] + public void When_using_CreateInterfaceProxyWithTargetInterface_SelectInterceptors_receives_type_equal_to_type_of_target() + { + var selector = new InterceptorSelector(); + + var proxy = generator.CreateInterfaceProxyWithTargetInterface(new FooTarget(), new ProxyGenerationOptions { Selector = selector }, new DoNothingInterceptor()); + proxy.Method(); + + Assert.AreEqual(typeof(FooTarget), selector.ReceivedType); + } + + [Test] + public void When_using_CreateInterfaceProxyWithTargetInterface_Invocation_TargetType_is_equal_to_type_of_target() + { + var interceptor = new Interceptor(); + + var proxy = generator.CreateInterfaceProxyWithTargetInterface(new FooTarget(), interceptor); + proxy.Method(); + + Assert.AreEqual(typeof(FooTarget), interceptor.ReceivedTargetType); + } + + public interface IFoo + { + void Method(); + } + + public abstract class Foo : IFoo + { + public abstract void Method(); + } + + public sealed class FooTarget : Foo + { + public override void Method() + { + } + } + +#if FEATURE_SERIALIZATION + [Serializable] +#endif + public sealed class InterceptorSelector : IInterceptorSelector + { + public Type ReceivedType { get; private set; } + + public IInterceptor[] SelectInterceptors(Type type, MethodInfo method, IInterceptor[] interceptors) + { + this.ReceivedType = type; + return interceptors; + } + } + + public sealed class Interceptor : IInterceptor + { + public Type ReceivedTargetType { get; private set; } + + public void Intercept(IInvocation invocation) + { + ReceivedTargetType = invocation.TargetType; + } + } + } +} From 1a3b4a0d32c57f428989b7eafdb09bf66d442227 Mon Sep 17 00:00:00 2001 From: stakx Date: Sat, 26 May 2018 20:54:42 +0200 Subject: [PATCH 2/3] Fix wrong `type` passed to `SelectInterceptors` This is achieved by the following three measures: 1. Fix the incorrect XML documentation for the `type` parameter of `IInterceptorSelector.SelectInterceptors`. This parameter receives the target type, *not* the intercepted method's declaring type. 2. Add a new ctor to `MethodWithInvocationGenerator` that accepts an additional parameter `getTargetTypeExpression` (next to `get- TargetExpression`) to account for the fact that invocation classes sometimes require a target object (e.g. `CompositionInvocation`) while others require a target type (e.g. `InheritanceInvocation`). The new parameter is optional; if `getTargetTypeExpression` is not provided, the target type is derived from the target object. 3. Use the new constructor for class proxies. We do *not* want to have the target type derived from the target object, because that would make the dynamically generated proxy type visible. (For class proxies, the target object and the proxy object are one and the same). Instead we want the target type to be the proxied type. --- .../ClassProxyTargetContributor.cs | 5 +++- .../MethodWithInvocationGenerator.cs | 24 ++++++++++++++++--- .../DynamicProxy/IInterceptorSelector.cs | 2 +- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs index 3cba5a161c..bcb0e75382 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyTargetContributor.cs @@ -78,10 +78,13 @@ protected override MethodGenerator GetMethodGenerator(MetaMethod method, ClassEm var invocation = GetInvocationType(method, @class, options); + GetTargetExpressionDelegate getTargetTypeExpression = (c, m) => new TypeTokenExpression(targetType); + return new MethodWithInvocationGenerator(method, @class.GetField("__interceptors"), invocation, - (c, m) => new TypeTokenExpression(targetType), + getTargetTypeExpression, + getTargetTypeExpression, overrideMethod, null); } diff --git a/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs index 8bdcbf47d3..8414d28f59 100644 --- a/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs +++ b/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs @@ -32,16 +32,26 @@ public class MethodWithInvocationGenerator : MethodGenerator { private readonly IInvocationCreationContributor contributor; private readonly GetTargetExpressionDelegate getTargetExpression; + private readonly GetTargetExpressionDelegate getTargetTypeExpression; private readonly Reference interceptors; private readonly Type invocation; public MethodWithInvocationGenerator(MetaMethod method, Reference 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, + GetTargetExpressionDelegate getTargetExpression, + GetTargetExpressionDelegate getTargetTypeExpression, + OverrideMethodDelegate createMethod, IInvocationCreationContributor contributor) : base(method, createMethod) { this.invocation = invocation; this.getTargetExpression = getTargetExpression; + this.getTargetTypeExpression = getTargetTypeExpression; this.interceptors = interceptors; this.contributor = contributor; } @@ -147,11 +157,19 @@ private Expression SetMethodInterceptors(ClassEmitter @class, INamingScope namin var methodInterceptorsField = BuildMethodInterceptorsField(@class, MethodToOverride, namingScope); + Expression targetTypeExpression; + if (getTargetTypeExpression != null) + { + targetTypeExpression = getTargetTypeExpression(@class, MethodToOverride); + } + else + { + targetTypeExpression = new MethodInvocationExpression(null, TypeUtilMethods.GetTypeOrNull, getTargetExpression(@class, MethodToOverride)); + } + var emptyInterceptors = new NewArrayExpression(0, typeof(IInterceptor)); var selectInterceptors = new MethodInvocationExpression(selector, InterceptorSelectorMethods.SelectInterceptors, - new MethodInvocationExpression(null, - TypeUtilMethods.GetTypeOrNull, - getTargetExpression(@class, MethodToOverride)), + targetTypeExpression, proxiedMethodTokenExpression, interceptors.ToExpression()) { VirtualCall = true }; diff --git a/src/Castle.Core/DynamicProxy/IInterceptorSelector.cs b/src/Castle.Core/DynamicProxy/IInterceptorSelector.cs index fa497e7da4..92faf54c52 100644 --- a/src/Castle.Core/DynamicProxy/IInterceptorSelector.cs +++ b/src/Castle.Core/DynamicProxy/IInterceptorSelector.cs @@ -26,7 +26,7 @@ public interface IInterceptorSelector /// /// Selects the interceptors that should intercept calls to the given . /// - /// The type declaring the method to intercept. + /// The type of the target object. /// The method that will be intercepted. /// All interceptors registered with the proxy. /// An array of interceptors to invoke upon calling the . From e9c58c8ecee42adcee950f0136a73e0fe680b535 Mon Sep 17 00:00:00 2001 From: stakx Date: Sat, 26 May 2018 23:29:27 +0200 Subject: [PATCH 3/3] Update the changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10048ba5e2..7185af0565 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Bugfixes: - Fix Castle.DynamicProxy.Generators.AttributesToAvoidReplicating not being thread safe (InvalidOperationException "Collection was modified; enumeration operation may not execute.") (@BrunoJuchli, #334) - Fix TraceLoggerFactory to allow specifying the default logger level (@acjh, #342) - Ensure that DynamicProxy doesn't create invalid dynamic assemblies when proxying types from non-strong-named assemblies (@stakx, #327) +- Fix interceptor selectors being passed `System.RuntimeType` for class proxies instead of the target type (@stakx, #359) ## 4.2.1 (2017-10-11)