From 3608d6b7834411e8ab7a7ca9e6c19f987447d429 Mon Sep 17 00:00:00 2001 From: Ralf Date: Mon, 31 Jan 2022 23:08:16 +0100 Subject: [PATCH] more strict check for aspect method signatures --- .../DuplicateEntryMethodsAspectTests.cs | 40 ++++++++++++++ .../Aspects/DuplicateEntryMethodsAspect.cs | 55 +++++++++++++++++++ .../DuplicateEntryMethodsAspectMethods.cs | 19 +++++++ .../AspectDataOnAsyncMethod.cs | 2 +- .../AspectMethodCriteria.cs | 36 ++++++++++++ .../InstructionBlockChainCreator.cs | 20 ++++--- .../MethodWeaverFactory.cs | 8 +-- 7 files changed, 167 insertions(+), 13 deletions(-) create mode 100644 src/MethodBoundaryAspect.Fody.UnitTests.NetFramework/DuplicateEntryMethodsAspectTests.cs create mode 100644 src/MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework/Aspects/DuplicateEntryMethodsAspect.cs create mode 100644 src/MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework/DuplicateEntryMethodsAspectMethods.cs create mode 100644 src/MethodBoundaryAspect.Fody/AspectMethodCriteria.cs diff --git a/src/MethodBoundaryAspect.Fody.UnitTests.NetFramework/DuplicateEntryMethodsAspectTests.cs b/src/MethodBoundaryAspect.Fody.UnitTests.NetFramework/DuplicateEntryMethodsAspectTests.cs new file mode 100644 index 0000000..6e9e0a1 --- /dev/null +++ b/src/MethodBoundaryAspect.Fody.UnitTests.NetFramework/DuplicateEntryMethodsAspectTests.cs @@ -0,0 +1,40 @@ +using System; +using FluentAssertions; +using MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework; +using Xunit; + +namespace MethodBoundaryAspect.Fody.UnitTests.NetFramework +{ + public class DuplicateEntryMethodsAspectTests : MethodBoundaryAspectNetFrameworkTestBase + { + private static readonly Type TestMethodsType = typeof(DuplicateEntryMethodsAspectMethods); + + [Fact] + public void IfStaticMethodIsCalled_ThenTheOnMethodBoundaryAspectShouldBeCalled() + { + // Arrange + const string testMethodName = "StaticMethodCall"; + WeaveAssemblyMethodAndLoad(TestMethodsType, testMethodName); + + // Act + var result = AssemblyLoader.InvokeMethod(TestMethodsType.TypeInfo(), testMethodName); + + // Assert + result.Should().Be("-OnEntry()-OnExit()"); + } + + [Fact] + public void IfInstanceMethodIsCalled_ThenTheOnMethodBoundaryAspectShouldBeCalled() + { + // Arrange + const string testMethodName = "InstanceMethodCall"; + WeaveAssemblyMethodAndLoad(TestMethodsType, testMethodName); + + // Act + var result = AssemblyLoader.InvokeMethod(TestMethodsType.TypeInfo(), testMethodName); + + // Assert + result.Should().Be("-OnEntry()-OnExit()"); + } + } +} \ No newline at end of file diff --git a/src/MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework/Aspects/DuplicateEntryMethodsAspect.cs b/src/MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework/Aspects/DuplicateEntryMethodsAspect.cs new file mode 100644 index 0000000..aa1323e --- /dev/null +++ b/src/MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework/Aspects/DuplicateEntryMethodsAspect.cs @@ -0,0 +1,55 @@ +using MethodBoundaryAspect.Fody.Attributes; + +namespace MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework.Aspects +{ + /// + /// remark: weaved methods with parameter MethodExecutionArgs should be ordered last + /// + public class DuplicateEntryMethodsAspect : OnMethodBoundaryAspect + { + public virtual void OnEntry(MethodExecutionArgs arg, bool b) + { + DuplicateEntryMethodsAspectMethods.Result += "-OnEntry(bool)"; + } + + public override void OnEntry(MethodExecutionArgs arg) + { + DuplicateEntryMethodsAspectMethods.Result += "-OnEntry()"; + } + + public virtual void OnEntry(MethodExecutionArgs arg) + { + DuplicateEntryMethodsAspectMethods.Result += "-OnEntry"; + } + + public virtual void OnExit(MethodExecutionArgs arg, bool b) + { + DuplicateEntryMethodsAspectMethods.Result += "-OnExit(bool)"; + } + + public virtual void OnExit(MethodExecutionArgs arg) + { + DuplicateEntryMethodsAspectMethods.Result += "-OnExit"; + } + + public override void OnExit(MethodExecutionArgs arg) + { + DuplicateEntryMethodsAspectMethods.Result += "-OnExit()"; + } + + public virtual void OnException(MethodExecutionArgs arg, bool b) + { + DuplicateEntryMethodsAspectMethods.Result += "-OnException(bool)"; + } + + public virtual void OnException(MethodExecutionArgs arg) + { + DuplicateEntryMethodsAspectMethods.Result += "-OnException"; + } + + public override void OnException(MethodExecutionArgs arg) + { + DuplicateEntryMethodsAspectMethods.Result += "-OnException()"; + } + } +} \ No newline at end of file diff --git a/src/MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework/DuplicateEntryMethodsAspectMethods.cs b/src/MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework/DuplicateEntryMethodsAspectMethods.cs new file mode 100644 index 0000000..ab6f6ad --- /dev/null +++ b/src/MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework/DuplicateEntryMethodsAspectMethods.cs @@ -0,0 +1,19 @@ +using MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework.Aspects; + +namespace MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework +{ + public class DuplicateEntryMethodsAspectMethods + { + public static object Result { get; set; } + + [DuplicateEntryMethodsAspect] + public static void StaticMethodCall() + { + } + + [DuplicateEntryMethodsAspect] + public void InstanceMethodCall() + { + } + } +} \ No newline at end of file diff --git a/src/MethodBoundaryAspect.Fody/AspectDataOnAsyncMethod.cs b/src/MethodBoundaryAspect.Fody/AspectDataOnAsyncMethod.cs index b8df025..22adc0f 100644 --- a/src/MethodBoundaryAspect.Fody/AspectDataOnAsyncMethod.cs +++ b/src/MethodBoundaryAspect.Fody/AspectDataOnAsyncMethod.cs @@ -83,7 +83,7 @@ public InstructionBlockChain LoadTagInMoveNext(IPersistable executionArgs) public InstructionBlockChain CallOnExceptionInMoveNext(IPersistable executionArgs, VariableDefinition exceptionLocal) { var onExceptionMethodRef = _referenceFinder.GetMethodReference(Info.AspectAttribute.AttributeType, - md => md.Name == "OnException"); + AspectMethodCriteria.IsOnExceptionMethod); var setMethod = _referenceFinder.GetMethodReference(executionArgs.PersistedType, md => md.Name == "set_Exception"); diff --git a/src/MethodBoundaryAspect.Fody/AspectMethodCriteria.cs b/src/MethodBoundaryAspect.Fody/AspectMethodCriteria.cs new file mode 100644 index 0000000..9bc1308 --- /dev/null +++ b/src/MethodBoundaryAspect.Fody/AspectMethodCriteria.cs @@ -0,0 +1,36 @@ +using System.Linq; +using Mono.Cecil; + +namespace MethodBoundaryAspect.Fody +{ + public static class AspectMethodCriteria + { + public static readonly string OnEntryMethodName = "OnEntry"; + public static readonly string OnExitMethodName = "OnExit"; + public static readonly string OnExceptionMethodName = "OnException"; + + public static bool MatchesSignature(MethodDefinition method) + { + return method.IsVirtual + && !method.HasGenericParameters + && method.Parameters.Count == 1 + && method.Parameters.Single().ParameterType.FullName == + "MethodBoundaryAspect.Fody.Attributes.MethodExecutionArgs"; + } + + public static bool IsOnEntryMethod(MethodDefinition method) + { + return method.Name == OnEntryMethodName && MatchesSignature(method); + } + + public static bool IsOnExitMethod(MethodDefinition method) + { + return method.Name == OnExitMethodName && MatchesSignature(method); + } + + public static bool IsOnExceptionMethod(MethodDefinition method) + { + return method.Name == OnExceptionMethodName && MatchesSignature(method); + } + } +} \ No newline at end of file diff --git a/src/MethodBoundaryAspect.Fody/InstructionBlockChainCreator.cs b/src/MethodBoundaryAspect.Fody/InstructionBlockChainCreator.cs index 31e4fa3..1e56eae 100644 --- a/src/MethodBoundaryAspect.Fody/InstructionBlockChainCreator.cs +++ b/src/MethodBoundaryAspect.Fody/InstructionBlockChainCreator.cs @@ -114,7 +114,7 @@ public NamedInstructionBlockChain CreateMethodExecutionArgsInstance( // MethodExecutionArgs instance var onEntryMethodTypeRef = - anyAspectTypeDefinition.Resolve().BaseType.Resolve().Methods.Single(x => x.Name == "OnEntry"); + anyAspectTypeDefinition.Resolve().BaseType.Resolve().Methods.Single(AspectMethodCriteria.IsOnEntryMethod); var firstParameterType = onEntryMethodTypeRef.Parameters.Single().ParameterType; var methodExecutionArgsTypeRef = _moduleDefinition.ImportReference(firstParameterType); @@ -272,11 +272,13 @@ public NamedInstructionBlockChain SaveThrownException() return block; } - public InstructionBlockChain CallAspectOnEntry(AspectData aspectInstance, + public InstructionBlockChain CallAspectOnEntry( + AspectData aspectInstance, IPersistable executionArgs) { - var onEntryMethodRef = _referenceFinder.GetMethodReference(aspectInstance.Info.AspectAttribute.AttributeType, - md => md.Name == "OnEntry"); + var onEntryMethodRef = _referenceFinder.GetMethodReference( + aspectInstance.Info.AspectAttribute.AttributeType, + AspectMethodCriteria.IsOnEntryMethod); var callOnEntryBlock = _creator.CallVoidInstanceMethod(onEntryMethodRef, aspectInstance.AspectPersistable, executionArgs); @@ -288,8 +290,9 @@ public InstructionBlockChain CallAspectOnEntry(AspectData aspectInstance, public InstructionBlockChain CallAspectOnExit(AspectData aspectData, IPersistable executionArgs) { - var onExitMethodRef = _referenceFinder.GetMethodReference(aspectData.Info.AspectAttribute.AttributeType, - md => md.Name == "OnExit"); + var onExitMethodRef = _referenceFinder.GetMethodReference( + aspectData.Info.AspectAttribute.AttributeType, + AspectMethodCriteria.IsOnExitMethod); var callOnExitBlock = _creator.CallVoidInstanceMethod(onExitMethodRef, aspectData.AspectPersistable, executionArgs); @@ -302,8 +305,9 @@ public InstructionBlockChain CallAspectOnException( AspectData aspectData, IPersistable executionArgs) { - var onExceptionMethodRef = _referenceFinder.GetMethodReference(aspectData.Info.AspectAttribute.AttributeType, - md => md.Name == "OnException"); + var onExceptionMethodRef = _referenceFinder.GetMethodReference( + aspectData.Info.AspectAttribute.AttributeType, + AspectMethodCriteria.IsOnExceptionMethod); var callOnExceptionBlock = _creator.CallVoidInstanceMethod(onExceptionMethodRef, aspectData.AspectPersistable, executionArgs); diff --git a/src/MethodBoundaryAspect.Fody/MethodWeaverFactory.cs b/src/MethodBoundaryAspect.Fody/MethodWeaverFactory.cs index 2b560a7..d9c14e4 100644 --- a/src/MethodBoundaryAspect.Fody/MethodWeaverFactory.cs +++ b/src/MethodBoundaryAspect.Fody/MethodWeaverFactory.cs @@ -44,7 +44,7 @@ static AspectMethods GetUsedAspectMethods(TypeReference aspectTypeDefinition) { var typeDefinition = currentType.Resolve(); var methods = typeDefinition.Methods - .Where(x => x.IsVirtual) + .Where(AspectMethodCriteria.MatchesSignature) .ToList(); foreach (var method in methods) { @@ -58,11 +58,11 @@ static AspectMethods GetUsedAspectMethods(TypeReference aspectTypeDefinition) } while (currentType.FullName != AttributeFullNames.OnMethodBoundaryAspect); var aspectMethods = AspectMethods.None; - if (overloadedMethods.ContainsKey("OnEntry")) + if (overloadedMethods.ContainsKey(AspectMethodCriteria.OnEntryMethodName)) aspectMethods |= AspectMethods.OnEntry; - if (overloadedMethods.ContainsKey("OnExit")) + if (overloadedMethods.ContainsKey(AspectMethodCriteria.OnExitMethodName)) aspectMethods |= AspectMethods.OnExit; - if (overloadedMethods.ContainsKey("OnException")) + if (overloadedMethods.ContainsKey(AspectMethodCriteria.OnExceptionMethodName)) aspectMethods |= AspectMethods.OnException; return aspectMethods; }