Check for properties that can't befound

If you give ModelExpressionProvider a lambda with a private property
you'll end up here. This wasn't common before, but it seems like users
are more likely to try it with pages.

Model Metadata and Model Binding don't handle private properties, so
supporting it in Model Expressions seems less than useful.

This isn't a breaking change because this case would have resulted in a
null-ref. Addresses #6400
This commit is contained in:
Ryan Nowak 2017-06-25 20:40:42 -07:00
parent c351712419
commit a23307e2b1
7 changed files with 213 additions and 11 deletions

View File

@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Diagnostics;
using System.Globalization;
using System.Linq.Expressions;
using System.Reflection;
@ -60,7 +61,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
return FromModel(viewData, metadataProvider);
}
containerType = memberExpression.Expression.Type;
// memberExpression.Expression can be null when this is a static field or property.
//
// This can be the case if the expression is like (m => Person.Name) where Name is a static field
// or property on the Person type.
containerType = memberExpression.Expression?.Type;
legalExpression = true;
break;
@ -86,16 +92,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
}
};
ModelMetadata metadata;
if (propertyName == null)
{
// Ex:
// m => 5 (arbitrary expression)
// m => foo (arbitrary expression)
// m => m.Widgets[0] (expression ending with non-property-access)
metadata = metadataProvider.GetMetadataForType(typeof(TResult));
}
else
ModelMetadata metadata = null;
if (containerType != null && propertyName != null)
{
// Ex:
// m => m.Color (simple property access)
@ -103,6 +101,22 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
// m => m.Widgets[0].Size (expression ending with property-access)
metadata = metadataProvider.GetMetadataForType(containerType).Properties[propertyName];
}
if (metadata == null)
{
// Ex:
// m => 5 (arbitrary expression)
// m => foo (arbitrary expression)
// m => m.Widgets[0] (expression ending with non-property-access)
//
// This can also happen for any case where we cannot retrieve a model metadata.
// This will happen for:
// - fields
// - statics
// - non-visibility (internal/private)
metadata = metadataProvider.GetMetadataForType(typeof(TResult));
Debug.Assert(metadata != null);
}
return viewData.ModelExplorer.GetExplorerForExpression(metadata, modelAccessor);
}

View File

@ -523,6 +523,32 @@ Products: Music Systems, Televisions (3)";
Assert.Equal(expected, response, ignoreLineEndingDifferences: true);
}
// We want to make sure that for 'wierd' model expressions involving:
// - fields
// - statics
// - private
//
// These tests verify that we don't throw, and can evaluate the expression to get the model
// value. One quirk of behavior for these cases is that we can't return a correct model metadata
// instance (this is true for anything other than a public instance property). We're not overly
// concerned with that, and so the accuracy of the model metadata is is not verified by the test.
[Theory]
[InlineData("GetWeirdWithHtmlHelpers")]
[InlineData("GetWeirdWithTagHelpers")]
public async Task WeirdModelExpressions_CanAccessModelValues(string action)
{
// Arrange
var url = "http://localhost/HtmlGeneration_WeirdExpressions/" + action;
// Act
var response = await Client.GetStringAsync(url);
// Assert
Assert.Contains("Hello, Field World!", response);
Assert.Contains("Hello, Static World!", response);
Assert.Contains("Hello, Private World!", response);
}
private static HttpRequestMessage RequestWithLocale(string url, string locale)
{
var request = new HttpRequestMessage(HttpMethod.Get, url);

View File

@ -10,6 +10,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
{
public class ExpressionMetadataProviderTest
{
private string PrivateProperty { get; set; }
public static string StaticProperty { get; set; }
public string Field = "Hello";
[Fact]
public void FromLambdaExpression_GetsExpectedMetadata_ForIdentityExpression()
{
@ -148,6 +154,72 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
Assert.Same(myModel, metadata.Container.Model);
}
// A private property can't be found by the model metadata provider, so return the property's type
// as a best-effort mechanism.
[Fact]
public void FromLambdaExpression_ForPrivateProperty_ReturnsMetadataOfExpressionType()
{
// Arrange
var provider = new EmptyModelMetadataProvider();
var viewData = new ViewDataDictionary<ExpressionMetadataProviderTest>(provider);
// Act
var explorer = ExpressionMetadataProvider.FromLambdaExpression(
m => m.PrivateProperty,
viewData,
provider);
// Assert
Assert.NotNull(explorer);
Assert.NotNull(explorer.Metadata);
Assert.Equal(ModelMetadataKind.Type, explorer.Metadata.MetadataKind);
Assert.Equal(typeof(string), explorer.ModelType);
}
// A static property can't be found by the model metadata provider, so return the property's type
// as a best-effort mechanism.
[Fact]
public void FromLambdaExpression_ForStaticProperty_ReturnsMetadataOfExpressionType()
{
// Arrange
var provider = new EmptyModelMetadataProvider();
var viewData = new ViewDataDictionary<ExpressionMetadataProviderTest>(provider);
// Act
var explorer = ExpressionMetadataProvider.FromLambdaExpression(
m => ExpressionMetadataProviderTest.StaticProperty,
viewData,
provider);
// Assert
Assert.NotNull(explorer);
Assert.NotNull(explorer.Metadata);
Assert.Equal(ModelMetadataKind.Type, explorer.Metadata.MetadataKind);
Assert.Equal(typeof(string), explorer.ModelType);
}
// A field can't be found by the model metadata provider, so return the field's type
// as a best-effort mechanism.
[Fact]
public void FromLambdaExpression_ForField_ReturnsMetadataOfExpressionType()
{
// Arrange
var provider = new EmptyModelMetadataProvider();
var viewData = new ViewDataDictionary<ExpressionMetadataProviderTest>(provider);
// Act
var explorer = ExpressionMetadataProvider.FromLambdaExpression(
m => m.Field,
viewData,
provider);
// Assert
Assert.NotNull(explorer);
Assert.NotNull(explorer.Metadata);
Assert.Equal(ModelMetadataKind.Type, explorer.Metadata.MetadataKind);
Assert.Equal(typeof(string), explorer.ModelType);
}
private class TestModel
{
public Category SelectedCategory { get; set; }

View File

@ -0,0 +1,22 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using HtmlGenerationWebSite.Models;
using Microsoft.AspNetCore.Mvc;
namespace HtmlGenerationWebSite.Controllers
{
public class HtmlGeneration_WeirdExpressionsController : Controller
{
public IActionResult GetWeirdWithHtmlHelpers()
{
return View(new WeirdModel());
}
public IActionResult GetWeirdWithTagHelpers()
{
return View(new WeirdModel());
}
}
}

View File

@ -0,0 +1,14 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System.ComponentModel.DataAnnotations;
namespace HtmlGenerationWebSite.Models
{
public class WeirdModel
{
public string Field = "Hello, Field World!";
public static string StaticProperty { get; set; } = "Hello, Static World!";
}
}

View File

@ -0,0 +1,27 @@
@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers
@using HtmlGenerationWebSite.Models
@model WeirdModel
@functions{
private string PrivateProperty { get; set; } = "Hello, Private World!";
}
<html>
<body>
<div>
<p>Field</p>
@Html.LabelFor(m => m.Field)
@Html.TextBoxFor(m => m.Field)
</div>
<div>
<p>Static Property</p>
@Html.LabelFor(m => WeirdModel.StaticProperty)
@Html.TextBoxFor(m => WeirdModel.StaticProperty)
</div>
<div>
<p>Private Property</p>
@Html.LabelFor(m => PrivateProperty)
@Html.TextBoxFor(m => PrivateProperty)
</div>
</body>
</html>

View File

@ -0,0 +1,27 @@
@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers
@using HtmlGenerationWebSite.Models
@model WeirdModel
@functions{
private string PrivateProperty { get; set; } = "Hello, Private World!";
}
<html>
<body>
<div>
<p>Field</p>
<label asp-for="Field" />
<input asp-for="Field" />
</div>
<div>
<p>Static Property</p>
<label asp-for="@WeirdModel.StaticProperty" />
<input asp-for="@WeirdModel.StaticProperty" />
</div>
<div>
<p>Private Property</p>
<label asp-for="@PrivateProperty" />
<input asp-for="@PrivateProperty" />
</div>
</body>
</html>