huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Do not cache expressions containing method calls 

- #5655
- also make `ExpressionTextCache` more robust for defence-in-depth

nits:
- two `null` expression nodes are equal
- declare data properties as `TheoryData<T>`
This commit is contained in:
Doug Bunting 2017-01-05 20:38:58 -08:00
parent 842d661ac2
commit 8ee3d45ef1
3 changed files with 94 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs
View File

@ -61,11 +61,14 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
switch (part.NodeType)
{
case ExpressionType.Call:
// Will exit loop if at Method().Property or [i,j].Property. In that case (like [i].Property),
// don't cache and don't remove ".Model" (if that's .Property).
containsIndexers = true;
lastIsModel = false;
var methodExpression = (MethodCallExpression)part;
if (IsSingleArgumentIndexer(methodExpression))
{
containsIndexers = true;
lastIsModel = false;
length += "[99]".Length;
part = methodExpression.Object;
segmentCount++;

55
src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionTextCache.cs
View File

@ -37,6 +37,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
while (true)
{
if (expression1 == null && expression2 == null)
{
return true;
}
if (expression1 == null || expression2 == null)
{
return false;
@ -47,31 +52,41 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
return false;
}
if (expression1.NodeType == ExpressionType.MemberAccess)
switch (expression1.NodeType)
{
var memberExpression1 = (MemberExpression)expression1;
var memberName1 = memberExpression1.Member.Name;
expression1 = memberExpression1.Expression;
case ExpressionType.MemberAccess:
var memberExpression1 = (MemberExpression)expression1;
var memberName1 = memberExpression1.Member.Name;
expression1 = memberExpression1.Expression;
var memberExpression2 = (MemberExpression)expression2;
var memberName2 = memberExpression2.Member.Name;
expression2 = memberExpression2.Expression;
var memberExpression2 = (MemberExpression)expression2;
var memberName2 = memberExpression2.Member.Name;
expression2 = memberExpression2.Expression;
// If identifier contains "__", it is "reserved for use by the implementation" and likely compiler-
// or Razor-generated e.g. the name of a field in a delegate's generated class.
if (memberName1.Contains("__") && memberName2.Contains("__"))
{
return true;
}
// If identifier contains "__", it is "reserved for use by the implementation" and likely
// compiler- or Razor-generated e.g. the name of a field in a delegate's generated class.
if (memberName1.Contains("__") && memberName2.Contains("__"))
{
return true;
}
if (!string.Equals(memberName1, memberName2, StringComparison.OrdinalIgnoreCase))
{
if (!string.Equals(memberName1, memberName2, StringComparison.OrdinalIgnoreCase))
{
return false;
}
break;
case ExpressionType.ArrayIndex:
// Shouldn't be cached. Just in case, ensure indexers are all different.
return false;
}
}
else
{
return true;
case ExpressionType.Call:
// Shouldn't be cached. Just in case, ensure indexers and other calls are all different.
return false;
default:
// Everything else terminates name generation. Haven't found a difference so far...
return true;
}
}
}

60
test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionHelperTest.cs
View File

@ -3,6 +3,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using Xunit;
@ -12,7 +13,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
{
private readonly ExpressionTextCache _expressionTextCache = new ExpressionTextCache();
public static IEnumerable<object[]> ExpressionAndTexts
public static TheoryData<Expression, string> ExpressionAndTexts
{
get
{
@ -80,11 +81,35 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
(Expression<Func<IList<TestModel>, int>>)(model => model[2].PreferredCategories[i].CategoryId),
"[2].PreferredCategories[3].CategoryId"
},
{
(Expression<Func<IList<TestModel>, string>>)(model => model.FirstOrDefault().Name),
"Name"
},
{
(Expression<Func<IList<TestModel>, string>>)(model => model.FirstOrDefault().Model),
"Model"
},
{
(Expression<Func<IList<TestModel>, int>>)(model => model.FirstOrDefault().SelectedCategory.CategoryId),
"SelectedCategory.CategoryId"
},
{
(Expression<Func<IList<TestModel>, string>>)(model => model.FirstOrDefault().SelectedCategory.CategoryName.MainCategory),
"SelectedCategory.CategoryName.MainCategory"
},
{
(Expression<Func<IList<TestModel>, int>>)(model => model.FirstOrDefault().PreferredCategories.Count),
"PreferredCategories.Count"
},
{
(Expression<Func<IList<TestModel>, int>>)(model => model.FirstOrDefault().PreferredCategories.FirstOrDefault().CategoryId),
"CategoryId"
},
};
}
}
public static IEnumerable<object[]> CachedExpressions
public static TheoryData<Expression> CachedExpressions
{
get
{
@ -104,7 +129,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
}
}
public static IEnumerable<object[]> IndexerExpressions
public static TheoryData<Expression> IndexerExpressions
{
get
{
@ -123,7 +148,29 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
}
}
public static IEnumerable<object[]> EquivalentExpressions
public static TheoryData<Expression> UnsupportedExpressions
{
get
{
var i = 2;
var j = 3;
return new TheoryData<Expression>
{
// Indexers that have multiple arguments.
(Expression<Func<TestModel[][], string>>)(model => model[23][3].Name),
(Expression<Func<TestModel[][], string>>)(model => model[i][3].Name),
(Expression<Func<TestModel[][], string>>)(model => model[23][j].Name),
(Expression<Func<TestModel[][], string>>)(model => model[i][j].Name),
// Calls that aren't indexers.
(Expression<Func<IList<TestModel>, string>>)(model => model.FirstOrDefault().Name),
(Expression<Func<IList<TestModel>, string>>)(model => model.FirstOrDefault().SelectedCategory.CategoryName.MainCategory),
(Expression<Func<IList<TestModel>, int>>)(model => model.FirstOrDefault().PreferredCategories.FirstOrDefault().CategoryId),
};
}
}
public static TheoryData<Expression, Expression> EquivalentExpressions
{
get
{
@ -167,7 +214,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
}
}
public static IEnumerable<object[]> NonEquivalentExpressions
public static TheoryData<Expression, Expression> NonEquivalentExpressions
{
get
{
@ -253,7 +300,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
[Theory]
[MemberData(nameof(IndexerExpressions))]
public void GetExpressionText_DoesNotCacheIndexerExpression(LambdaExpression expression)
[MemberData(nameof(UnsupportedExpressions))]
public void GetExpressionText_DoesNotCacheIndexerOrUnspportedExpression(LambdaExpression expression)
{
// Act - 1
var text1 = ExpressionHelper.GetExpressionText(expression, _expressionTextCache);