castleproject / Core

Castle Core, including Castle DynamicProxy, Logging Services and DictionaryAdapter
http://www.castleproject.org/
Other
2.2k stars 467 forks source link

Failure proxying type that has a non-inheritable custom attribute type applied where `null` argument is given for array parameter #637

Closed stakx closed 1 year ago

stakx commented 1 year ago

@kimbirkelund made a bug report over at the Moq 4 repository that boils down to a problem with how DynamicProxy processes array parameters of custom attributes. Given the following test code:

namespace Castle.DynamicProxy.Tests;

using System;

using NUnit.Framework;

[TestFixture]
public class Tests : BasePEVerifyTestCase
{
    [TestCase(typeof(I1))]
    [TestCase(typeof(I2))]
    [TestCase(typeof(I3))]
    [TestCase(typeof(I4))]
    [TestCase(typeof(I5))]
    [TestCase(typeof(I6))]
    [TestCase(typeof(I7))]
    [TestCase(typeof(I8))]
    public void Can_proxy_type(Type interfaceTypeToProxy)
    {
        _ = generator.CreateInterfaceProxyWithoutTarget(interfaceTypeToProxy);
    }

    [NestedInheritedAttributeWithOnePositionalArrayParameter(null)]
    public interface I1 { }

    [NestedInheritedAttributeWithOnePositionalArrayParameter(new object[0])]
    public interface I2 { }

    [NestedNonInheritedAttributeWithOnePositionalArrayParameter(null)]
    public interface I3 { }

    [NestedNonInheritedAttributeWithOnePositionalArrayParameter(new object[0])]
    public interface I4 { }

    [NonNestedInheritedAttributeWithOnePositionalArrayParameter(null)]
    public interface I5 { }

    [NonNestedInheritedAttributeWithOnePositionalArrayParameter(new object[0])]
    public interface I6 { }

    [NonNestedNonInheritedAttributeWithOnePositionalArrayParameter(null)]
    public interface I7 { }

    [NonNestedNonInheritedAttributeWithOnePositionalArrayParameter(new object[0])]
    public interface I8 { }

    [AttributeUsage(AttributeTargets.Interface, Inherited = true)]
    public sealed class NestedInheritedAttributeWithOnePositionalArrayParameterAttribute : Attribute
    {
        public NestedInheritedAttributeWithOnePositionalArrayParameterAttribute(object[] arg) { }
    }

    [AttributeUsage(AttributeTargets.Interface, Inherited = false)]
    public sealed class NestedNonInheritedAttributeWithOnePositionalArrayParameterAttribute : Attribute
    {
        public NestedNonInheritedAttributeWithOnePositionalArrayParameterAttribute(object[] arg) { }
    }
}

[AttributeUsage(AttributeTargets.Interface, Inherited = true)]
public sealed class NonNestedInheritedAttributeWithOnePositionalArrayParameterAttribute : Attribute
{
    public NonNestedInheritedAttributeWithOnePositionalArrayParameterAttribute(object[] arg) { }
}

[AttributeUsage(AttributeTargets.Interface, Inherited = false)]
public sealed class NonNestedNonInheritedAttributeWithOnePositionalArrayParameterAttribute : Attribute
{
    public NonNestedNonInheritedAttributeWithOnePositionalArrayParameterAttribute(object[] arg) { }
}

The test case using I7 fails with:

System.NullReferenceException : Object reference not set to an instance of an object.

Stack Trace: 
AttributeUtil.GetArguments(IList`1 constructorArguments) line 68
AttributeUtil.ReadAttributeValue(CustomAttributeTypedArgument argument) line 85
AttributeUtil.GetArguments(IList`1 constructorArguments, Type[]& constructorArgTypes, Object[]& constructorArgs) line 62
AttributeUtil.CreateInfo(CustomAttributeData attribute) line 35
AttributeUtil.GetNonInheritableAttributes(MemberInfo member)+MoveNext() line 135
NonInheritableAttributesContributor.Generate(ClassEmitter emitter) line 37
BaseInterfaceProxyGenerator.GenerateType(String typeName, INamingScope namingScope) line 136
<>c__DisplayClass13_0.<GetProxyType>b__0(CacheKey cacheKey) line 85
SynchronizedDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory) line 68
BaseProxyGenerator.GetProxyType() line 77
DefaultProxyBuilder.CreateInterfaceProxyTypeWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options) line 115
ProxyGenerator.CreateInterfaceProxyTypeWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options) line 1558
ProxyGenerator.CreateInterfaceProxyWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, IInterceptor[] interceptors) line 835
ProxyGenerator.CreateInterfaceProxyWithoutTarget(Type interfaceToProxy, IInterceptor[] interceptors) line 728
...

That is, the error surfaces only when the following conditions are met:

I haven't studied this in more detail yet, but it seems something goes wrong in the "special case for handling arrays in attributes" logic here.

stakx commented 1 year ago

If #639 gets merged, we can half the number of test cases here, because the distinction between nested / non-nested attribute types will no longer be relevant.

If we additionally eliminate the distinction between inheritable / non-inheritable attributes (since DynamicProxy generally won't replicate the former), we can reduce the tests for this issue to just this:

namespace Castle.DynamicProxy.Tests;

using System;

using NUnit.Framework;

[TestFixture]
public class Tests : BasePEVerifyTestCase
{
    [TestCase(typeof(I3))]
    [TestCase(typeof(I4))]
    public void Can_proxy_type(Type interfaceTypeToProxy)
    {
        _ = generator.CreateInterfaceProxyWithoutTarget(interfaceTypeToProxy);
    }

    [NonInheritedAttributeWithOnePositionalArrayParameter(null)]
    public interface I3 { }

    [NonInheritedAttributeWithOnePositionalArrayParameter(new object[0])]
    public interface I4 { }

    [AttributeUsage(AttributeTargets.Interface, Inherited = false)]
    public sealed class NonInheritedAttributeWithOnePositionalArrayParameterAttribute : Attribute
    {
        public NonInheritedAttributeWithOnePositionalArrayParameterAttribute(object[] arg) { }
    }
}