Merge pull request castleproject#642 from stakx/bug/custom-attribute-with-null-ctor-arg

Fix `NullReferenceException` caused by `null` argument for custom attribute's array-typed ctor parameter

Author: stakx

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 Bugfixes:
+- Failure proxying type that has a non-inheritable custom attribute type applied where `null` argument is given for array parameter  (@stakx, #637)
 - Nested custom attribute types do not get replicated (@stakx, #638)
 
 ## 5.1.0 (2022-08-02)

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

@@ -14,6 +14,7 @@
 
 namespace Castle.DynamicProxy.Tests
 {
+	using System;
 	using System.IO;
 	using System.Linq;
 	using System.Reflection;
@@ -178,5 +179,22 @@ public void Can_proxy_type_with_non_inheritable_attribute_depending_on_array_of_
 				.Cast<NonInheritableWithArray2Attribute>().Single();
 			CollectionAssert.AreEqual(attribute.Values, new[] { "1", "2", "3" });
 		}
+
+		[TestCase(typeof(HasAttributeWithNullAsPositionalArgument))]
+		[TestCase(typeof(HasAttributeWithEmptyArrayAsPositionalArgument))]
+		[TestCase(typeof(HasAttributeWithNullInArrayAsPositionalArgument))]
+		public void Can_proxy_type_with_attribute_with_positional_array_parameter(Type classType)
+		{
+			_ = generator.CreateClassProxy(classType);
+		}
+
+		[NonInheritableAttributeWithPositionalArrayParameter(null)]
+		public class HasAttributeWithNullAsPositionalArgument{ }
+
+		[NonInheritableAttributeWithPositionalArrayParameter(new object[0])]
+		public class HasAttributeWithEmptyArrayAsPositionalArgument { }
+
+		[NonInheritableAttributeWithPositionalArrayParameter(new object[1] { null })]
+		public class HasAttributeWithNullInArrayAsPositionalArgument { }
 	}
 }

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

@@ -0,0 +1,29 @@
+// Copyright 2004-2022 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
+{
+	using System;
+
+#if FEATURE_SERIALIZATION
+	[Serializable]
+#endif
+	[AttributeUsage(AttributeTargets.All, Inherited = false)]
+	public class NonInheritableAttributeWithPositionalArrayParameterAttribute : Attribute
+	{
+		public NonInheritableAttributeWithPositionalArrayParameterAttribute(object[] arg)
+		{
+		}
+	}
+}

src/Castle.Core/DynamicProxy/Internal/AttributeUtil.cs

@@ -28,12 +28,7 @@ public static CustomAttributeInfo CreateInfo(CustomAttributeData attribute)
 		{
 			Debug.Assert(attribute != null, "attribute != null");
 
-			// .NET Core does not provide CustomAttributeData.Constructor, so we'll implement it
-			// by finding a constructor ourselves
-			Type[] constructorArgTypes;
-			object[] constructorArgs;
-			GetArguments(attribute.ConstructorArguments, out constructorArgTypes, out constructorArgs);
-			var constructor = attribute.AttributeType.GetConstructor(constructorArgTypes);
+			object[] constructorArgs = GetArguments(attribute.ConstructorArguments);
 
 			PropertyInfo[] properties;
 			object[] propertyValues;
@@ -43,26 +38,14 @@ public static CustomAttributeInfo CreateInfo(CustomAttributeData attribute)
 				attribute.AttributeType,
 				attribute.NamedArguments, out properties, out propertyValues, out fields, out fieldValues);
 
-			return new CustomAttributeInfo(constructor,
+			return new CustomAttributeInfo(attribute.Constructor,
 			                               constructorArgs,
 			                               properties,
 			                               propertyValues,
 			                               fields,
 			                               fieldValues);
 		}
 
-		private static void GetArguments(IList<CustomAttributeTypedArgument> constructorArguments,
-			out Type[] constructorArgTypes, out object[] constructorArgs)
-		{
-			constructorArgTypes = new Type[constructorArguments.Count];
-			constructorArgs = new object[constructorArguments.Count];
-			for (var i = 0; i < constructorArguments.Count; i++)
-			{
-				constructorArgTypes[i] = constructorArguments[i].ArgumentType;
-				constructorArgs[i] = ReadAttributeValue(constructorArguments[i]);
-			}
-		}
-
 		private static object[] GetArguments(IList<CustomAttributeTypedArgument> constructorArguments)
 		{
 			var arguments = new object[constructorArguments.Count];
@@ -77,15 +60,18 @@ private static object[] GetArguments(IList<CustomAttributeTypedArgument> constru
 		private static object ReadAttributeValue(CustomAttributeTypedArgument argument)
 		{
 			var value = argument.Value;
-			if (argument.ArgumentType.IsArray == false)
+
+			if (argument.ArgumentType.IsArray && value is IList<CustomAttributeTypedArgument> values)
 			{
-				return value;
+				// `CustomAttributeInfo` represents array values as `ReadOnlyCollection<CustomAttributeTypedArgument>`,
+				// but `CustomAttributeBuilder` will require plain arrays, so we need a (recursive) conversion:
+				var arguments = GetArguments(values);
+				var array = new object[arguments.Length];
+				arguments.CopyTo(array, 0);
+				return array;
 			}
-			//special case for handling arrays in attributes
-			var arguments = GetArguments((IList<CustomAttributeTypedArgument>)value);
-			var array = new object[arguments.Length];
-			arguments.CopyTo(array, 0);
-			return array;
+
+			return value;
 		}
 
 		private static void GetSettersAndFields(Type attributeType, IEnumerable<CustomAttributeNamedArgument> namedArguments,