Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix stack overflow in FilterExpression.ValidForProperties #3946

Merged
merged 16 commits into from Aug 24, 2022
Merged
33 changes: 14 additions & 19 deletions src/Microsoft.TestPlatform.Common/Filtering/FilterExpression.cs
Expand Up @@ -119,8 +119,6 @@ private static void ProcessOperator(Stack<FilterExpression> filterStack, Operato
/// </summary>
internal string[]? ValidForProperties(IEnumerable<string>? properties, Func<string, TestProperty?>? propertyProvider)
{
string[]? invalidProperties = null;

if (null == properties)
{
// if null, initialize to empty list so that invalid properties can be found.
Expand All @@ -133,26 +131,23 @@ private static void ProcessOperator(Stack<FilterExpression> filterStack, Operato
{
bool valid = false;
valid = current._condition.ValidForProperties(properties, propertyProvider);
if (!valid) //if it's not valid will add it to the function's return array
{
invalidProperties = new string[1] { current._condition.Name };
}
// if it's not valid will add it to the function's return array
return !valid ? new string[1] { current._condition.Name } : null;
}
else
{
// concatenate the children node's result to get their parent result
var invalidRight = current._right != null ? result.Pop() : null;
invalidProperties = current._left != null ? result.Pop() : null;

if (null == invalidProperties)
{
invalidProperties = invalidRight;
}
else if (null != invalidRight)
{
invalidProperties = invalidProperties.Concat(invalidRight).ToArray();
}
// concatenate the children node's result to get their parent result
var invalidRight = current._right != null ? result.Pop() : null;
var invalidProperties = current._left != null ? result.Pop() : null;

if (null == invalidProperties)
{
invalidProperties = invalidRight;
}
else if (null != invalidRight)
{
invalidProperties = invalidProperties.Concat(invalidRight).ToArray();
}

return invalidProperties;
});

Expand Down
Expand Up @@ -94,14 +94,19 @@ public void FastFilterWithSingleEqualsClause()
[TestMethod]
public void ValidForPropertiesHandlesBigFilteringExpressions()
{
StringBuilder testCaseFilter = new("Test1");
StringBuilder testCaseFilter = new("Category=Test1");

for (int i = 0; i < 1e5; i++) // creating a 100k filter cases string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (int i = 0; i < 1e5; i++) // creating a 100k filter cases string
// Create filter with 100k conditions.
for (int i = 0; i < 100_000; i++)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nitpick.

{
testCaseFilter.Append("|Test2");
}

var filterExpressionWrapper = new FilterExpressionWrapper(testCaseFilter.ToString());
Assert.IsNull(filterExpressionWrapper.ValidForProperties(new List<string>() { "FullyQualifiedName" }, null));
string[]? invalidProperties = filterExpressionWrapper.ValidForProperties(new List<string>() { "FullyQualifiedName" }, null);

Assert.IsNotNull(invalidProperties);
Assert.AreEqual(invalidProperties?.Count(), 1);
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
Assert.AreEqual(invalidProperties![0], "Category");
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
}

[TestMethod]
Expand Down