[Fixes #7115] Update property type check in SaveTempDataPropertyFilterBase to match TempDataSerializer requirements

This commit is contained in:
Kiran Challa 2018-01-09 15:53:29 -08:00
parent b196708a3e
commit 00c6b53b06
6 changed files with 87 additions and 65 deletions

View File

@ -98,10 +98,14 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
var propertyType = Nullable.GetUnderlyingType(property.PropertyType) ?? property.PropertyType; var propertyType = Nullable.GetUnderlyingType(property.PropertyType) ?? property.PropertyType;
if (!(propertyType.GetTypeInfo().IsPrimitive || propertyType == typeof(string))) if (!TempDataSerializer.CanSerializeType(propertyType, out var errorMessage))
{ {
throw new InvalidOperationException( var messageWithPropertyInfo = Resources.FormatTempDataProperties_InvalidType(
Resources.FormatTempDataProperties_PrimitiveTypeOrString(property.DeclaringType.FullName, property.Name, nameof(TempDataAttribute))); property.DeclaringType.FullName,
property.Name,
nameof(TempDataAttribute));
throw new InvalidOperationException($"{messageWithPropertyInfo} {errorMessage}");
} }
} }

View File

@ -157,34 +157,50 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
public void EnsureObjectCanBeSerialized(object item) public void EnsureObjectCanBeSerialized(object item)
{ {
var itemType = item.GetType(); item = item ?? throw new ArgumentNullException(nameof(item));
Type actualType = null;
if (itemType.IsArray) var itemType = item.GetType();
if (!CanSerializeType(itemType, out var errorMessage))
{ {
itemType = itemType.GetElementType(); throw new InvalidOperationException(errorMessage);
} }
else if (itemType.GetTypeInfo().IsGenericType) }
public static bool CanSerializeType(Type typeToSerialize, out string errorMessage)
{
typeToSerialize = typeToSerialize ?? throw new ArgumentNullException(nameof(typeToSerialize));
errorMessage = null;
Type actualType = null;
if (typeToSerialize.IsArray)
{ {
if (ClosedGenericMatcher.ExtractGenericInterface(itemType, typeof(IList<>)) != null) actualType = typeToSerialize.GetElementType();
}
else if (typeToSerialize.GetTypeInfo().IsGenericType)
{
if (ClosedGenericMatcher.ExtractGenericInterface(typeToSerialize, typeof(IList<>)) != null)
{ {
var genericTypeArguments = itemType.GenericTypeArguments; var genericTypeArguments = typeToSerialize.GenericTypeArguments;
Debug.Assert(genericTypeArguments.Length == 1, "IList<T> has one generic argument"); Debug.Assert(genericTypeArguments.Length == 1, "IList<T> has one generic argument");
actualType = genericTypeArguments[0]; actualType = genericTypeArguments[0];
} }
else if (ClosedGenericMatcher.ExtractGenericInterface(itemType, typeof(IDictionary<,>)) != null) else if (ClosedGenericMatcher.ExtractGenericInterface(typeToSerialize, typeof(IDictionary<,>)) != null)
{ {
var genericTypeArguments = itemType.GenericTypeArguments; var genericTypeArguments = typeToSerialize.GenericTypeArguments;
Debug.Assert( Debug.Assert(
genericTypeArguments.Length == 2, genericTypeArguments.Length == 2,
"IDictionary<TKey, TValue> has two generic arguments"); "IDictionary<TKey, TValue> has two generic arguments");
// Throw if the key type of the dictionary is not string. // The key must be of type string.
if (genericTypeArguments[0] != typeof(string)) if (genericTypeArguments[0] != typeof(string))
{ {
var message = Resources.FormatTempData_CannotSerializeDictionary( errorMessage = Resources.FormatTempData_CannotSerializeDictionary(
typeof(TempDataSerializer).FullName, genericTypeArguments[0]); typeof(TempDataSerializer).FullName,
throw new InvalidOperationException(message); genericTypeArguments[0],
typeof(string).FullName);
return false;
} }
else else
{ {
@ -193,14 +209,16 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
} }
} }
actualType = actualType ?? itemType; actualType = actualType ?? typeToSerialize;
if (!IsSimpleType(actualType)) if (!IsSimpleType(actualType))
{ {
var underlyingType = Nullable.GetUnderlyingType(actualType) ?? actualType; errorMessage = Resources.FormatTempData_CannotSerializeType(
var message = Resources.FormatTempData_CannotSerializeType( typeof(TempDataSerializer).FullName,
typeof(TempDataSerializer).FullName, underlyingType); actualType);
throw new InvalidOperationException(message); return false;
} }
return true;
} }
private static IList<TVal> ConvertArray<TVal>(JArray array) private static IList<TVal> ConvertArray<TVal>(JArray array)

View File

@ -683,7 +683,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
=> string.Format(CultureInfo.CurrentCulture, GetString("TempData_CannotDeserializeToken"), p0, p1); => string.Format(CultureInfo.CurrentCulture, GetString("TempData_CannotDeserializeToken"), p0, p1);
/// <summary> /// <summary>
/// The '{0}' cannot serialize a dictionary with a key of type '{1}'. /// The '{0}' cannot serialize a dictionary with a key of type '{1}'. The key must be of type '{2}'.
/// </summary> /// </summary>
internal static string TempData_CannotSerializeDictionary internal static string TempData_CannotSerializeDictionary
{ {
@ -691,10 +691,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
} }
/// <summary> /// <summary>
/// The '{0}' cannot serialize a dictionary with a key of type '{1}'. /// The '{0}' cannot serialize a dictionary with a key of type '{1}'. The key must be of type '{2}'.
/// </summary> /// </summary>
internal static string FormatTempData_CannotSerializeDictionary(object p0, object p1) internal static string FormatTempData_CannotSerializeDictionary(object p0, object p1, object p2)
=> string.Format(CultureInfo.CurrentCulture, GetString("TempData_CannotSerializeDictionary"), p0, p1); => string.Format(CultureInfo.CurrentCulture, GetString("TempData_CannotSerializeDictionary"), p0, p1, p2);
/// <summary> /// <summary>
/// The '{0}' cannot serialize an object of type '{1}'. /// The '{0}' cannot serialize an object of type '{1}'.
@ -795,18 +795,18 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
=> string.Format(CultureInfo.CurrentCulture, GetString("ViewEnginesAreRequired"), p0, p1, p2); => string.Format(CultureInfo.CurrentCulture, GetString("ViewEnginesAreRequired"), p0, p1, p2);
/// <summary> /// <summary>
/// The '{0}.{1}' property with {2} is invalid. A property using {2} must be a primitive or string type. /// The '{0}.{1}' property with {2} is invalid.
/// </summary> /// </summary>
internal static string TempDataProperties_PrimitiveTypeOrString internal static string TempDataProperties_InvalidType
{ {
get => GetString("TempDataProperties_PrimitiveTypeOrString"); get => GetString("TempDataProperties_InvalidType");
} }
/// <summary> /// <summary>
/// The '{0}.{1}' property with {2} is invalid. A property using {2} must be a primitive or string type. /// The '{0}.{1}' property with {2} is invalid.
/// </summary> /// </summary>
internal static string FormatTempDataProperties_PrimitiveTypeOrString(object p0, object p1, object p2) internal static string FormatTempDataProperties_InvalidType(object p0, object p1, object p2)
=> string.Format(CultureInfo.CurrentCulture, GetString("TempDataProperties_PrimitiveTypeOrString"), p0, p1, p2); => string.Format(CultureInfo.CurrentCulture, GetString("TempDataProperties_InvalidType"), p0, p1, p2);
/// <summary> /// <summary>
/// The '{0}.{1}' property with {2} is invalid. A property using {2} must have a public getter and setter. /// The '{0}.{1}' property with {2} is invalid. A property using {2} must have a public getter and setter.

View File

@ -263,7 +263,7 @@
<value>Cannot deserialize {0} of type '{1}'.</value> <value>Cannot deserialize {0} of type '{1}'.</value>
</data> </data>
<data name="TempData_CannotSerializeDictionary" xml:space="preserve"> <data name="TempData_CannotSerializeDictionary" xml:space="preserve">
<value>The '{0}' cannot serialize a dictionary with a key of type '{1}'.</value> <value>The '{0}' cannot serialize a dictionary with a key of type '{1}'. The key must be of type '{2}'.</value>
</data> </data>
<data name="TempData_CannotSerializeType" xml:space="preserve"> <data name="TempData_CannotSerializeType" xml:space="preserve">
<value>The '{0}' cannot serialize an object of type '{1}'.</value> <value>The '{0}' cannot serialize an object of type '{1}'.</value>
@ -286,8 +286,8 @@
<data name="ViewEnginesAreRequired" xml:space="preserve"> <data name="ViewEnginesAreRequired" xml:space="preserve">
<value>'{0}.{1}' must not be empty. At least one '{2}' is required to locate a view for rendering.</value> <value>'{0}.{1}' must not be empty. At least one '{2}' is required to locate a view for rendering.</value>
</data> </data>
<data name="TempDataProperties_PrimitiveTypeOrString" xml:space="preserve"> <data name="TempDataProperties_InvalidType" xml:space="preserve">
<value>The '{0}.{1}' property with {2} is invalid. A property using {2} must be a primitive or string type.</value> <value>The '{0}.{1}' property with {2} is invalid.</value>
</data> </data>
<data name="TempDataProperties_PublicGetterSetter" xml:space="preserve"> <data name="TempDataProperties_PublicGetterSetter" xml:space="preserve">
<value>The '{0}.{1}' property with {2} is invalid. A property using {2} must have a public getter and setter.</value> <value>The '{0}.{1}' property with {2} is invalid. A property using {2} must have a public getter and setter.</value>

View File

@ -16,8 +16,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
{ {
[Theory] [Theory]
[InlineData(typeof(TestController_OneTempDataProperty))] [InlineData(typeof(TestController_OneTempDataProperty))]
[InlineData(typeof(TestController_ListOfString))]
[InlineData(typeof(TestController_OneNullableTempDataProperty))] [InlineData(typeof(TestController_OneNullableTempDataProperty))]
[InlineData(typeof(TestController_TwoTempDataProperties))] [InlineData(typeof(TestController_TwoTempDataProperties))]
[InlineData(typeof(TestController_NullableNonPrimitiveTempDataProperty))]
public void AddsTempDataPropertyFilter_ForTempDataAttributeProperties(Type type) public void AddsTempDataPropertyFilter_ForTempDataAttributeProperties(Type type)
{ {
// Arrange // Arrange
@ -58,23 +60,6 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
Assert.Same(expected, tempDataPropertyHelper.PropertyInfo); Assert.Same(expected, tempDataPropertyHelper.PropertyInfo);
} }
[Fact]
public void DoesNotInitializeFilterFactory_ThrowsInvalidOperationException_NonPrimitiveType()
{
// Arrange
var provider = new TempDataApplicationModelProvider();
var defaultProvider = new DefaultApplicationModelProvider(Options.Create(new MvcOptions()));
var context = new ApplicationModelProviderContext(new[] { typeof(TestController_OneValid_OneInvalidProperty).GetTypeInfo() });
defaultProvider.OnProvidersExecuting(context);
// Act
var exception = Assert.Throws<InvalidOperationException>(() =>
provider.OnProvidersExecuting(context));
Assert.Equal($"The '{typeof(TestController_OneValid_OneInvalidProperty).FullName}.{nameof(TestController_OneValid_OneInvalidProperty.Test2)}' property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)} must be a primitive or string type.", exception.Message);
}
[Fact] [Fact]
public void ThrowsInvalidOperationException_PrivateSetter() public void ThrowsInvalidOperationException_PrivateSetter()
{ {
@ -89,7 +74,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
var exception = Assert.Throws<InvalidOperationException>(() => var exception = Assert.Throws<InvalidOperationException>(() =>
provider.OnProvidersExecuting(context)); provider.OnProvidersExecuting(context));
Assert.Equal($"The '{typeof(TestController_PrivateSet).FullName}.{nameof(TestController_NonPrimitiveType.Test)}' property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)} must have a public getter and setter.", exception.Message); Assert.Equal(
$"The '{typeof(TestController_PrivateSet).FullName}.{nameof(TestController_NonPrimitiveType.Test)}'"
+ $" property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)}"
+ " must have a public getter and setter.",
exception.Message);
} }
[Fact] [Fact]
@ -106,26 +95,34 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
var exception = Assert.Throws<InvalidOperationException>(() => var exception = Assert.Throws<InvalidOperationException>(() =>
provider.OnProvidersExecuting(context)); provider.OnProvidersExecuting(context));
Assert.Equal($"The '{typeof(TestController_NonPrimitiveType).FullName}.{nameof(TestController_NonPrimitiveType.Test)}' property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)} must be a primitive or string type.", exception.Message); Assert.Equal(
$"The '{typeof(TestController_NonPrimitiveType).FullName}.{nameof(TestController_NonPrimitiveType.Test)}'"
+ $" property with {nameof(TempDataAttribute)} is invalid. The '{typeof(TempDataSerializer).FullName}'"
+ $" cannot serialize an object of type '{typeof(Object).FullName}'.",
exception.Message);
} }
[Fact] [Fact]
public void ThrowsInvalidOperationException_ForNullableNonPrimitiveType() public void ThrowsInvalidOperationException_NonStringDictionaryKey()
{ {
// Arrange // Arrange
var provider = new TempDataApplicationModelProvider(); var provider = new TempDataApplicationModelProvider();
var defaultProvider = new DefaultApplicationModelProvider(Options.Create(new MvcOptions())); var defaultProvider = new DefaultApplicationModelProvider(Options.Create(new MvcOptions()));
var controllerType = typeof(TestController_NullableNonPrimitiveTempDataProperty);
var context = new ApplicationModelProviderContext(new[] { controllerType.GetTypeInfo() }); var context = new ApplicationModelProviderContext(
new[] { typeof(TestController_NonStringDictionaryKey).GetTypeInfo() });
defaultProvider.OnProvidersExecuting(context); defaultProvider.OnProvidersExecuting(context);
// Act & Assert // Act & Assert
var exception = Assert.Throws<InvalidOperationException>(() => var exception = Assert.Throws<InvalidOperationException>(() =>
provider.OnProvidersExecuting(context)); provider.OnProvidersExecuting(context));
Assert.Equal($"The '{controllerType.FullName}.{nameof(TestController_NullableNonPrimitiveTempDataProperty.DateTime)}'" Assert.Equal(
+ $" property with {nameof(TempDataAttribute)} is invalid. A property using {nameof(TempDataAttribute)} " $"The '{typeof(TestController_NonStringDictionaryKey).FullName}.{nameof(TestController_NonStringDictionaryKey.Test)}'"
+ $"must be a primitive or string type.", exception.Message); + $" property with {nameof(TempDataAttribute)} is invalid. The '{typeof(TempDataSerializer).FullName}'"
+ $" cannot serialize a dictionary with a key of type '{typeof(Object)}'. The key must be of type"
+ $" '{typeof(string).FullName}'.",
exception.Message);
} }
public class TestController_NullableNonPrimitiveTempDataProperty public class TestController_NullableNonPrimitiveTempDataProperty
@ -159,13 +156,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
public int? Test2 { get; set; } public int? Test2 { get; set; }
} }
public class TestController_OneValid_OneInvalidProperty public class TestController_ListOfString
{ {
[TempData] [TempData]
public int Test { get; set; } public IList<string> Test { get; set; }
[TempData]
public IList<string> Test2 { get; set; }
} }
public class TestController_PrivateSet public class TestController_PrivateSet
@ -179,5 +173,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
[TempData] [TempData]
public object Test { get; set; } public object Test { get; set; }
} }
public class TestController_NonStringDictionaryKey
{
[TempData]
public IDictionary<object, object> Test { get; set; }
}
} }
} }

View File

@ -68,7 +68,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
testProvider.EnsureObjectCanBeSerialized(value); testProvider.EnsureObjectCanBeSerialized(value);
}); });
Assert.Equal($"The '{typeof(TempDataSerializer).FullName}' cannot serialize a dictionary " + Assert.Equal($"The '{typeof(TempDataSerializer).FullName}' cannot serialize a dictionary " +
$"with a key of type '{type}'.", $"with a key of type '{type}'. The key must be of type 'System.String'.",
exception.Message); exception.Message);
} }