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
105 changes: 74 additions & 31 deletions src/Microsoft.TestPlatform.Common/Filtering/FilterExpression.cs
Expand Up @@ -127,30 +127,33 @@ private static void ProcessOperator(Stack<FilterExpression> filterStack, Operato
properties = Enumerable.Empty<string>();
}

bool valid = false;
if (_condition != null)
return IterateFilterExpression<string[]?>((current, result) =>
{
valid = _condition.ValidForProperties(properties, propertyProvider);
if (!valid)
if (current._condition != null)
{
invalidProperties = new string[1] { _condition.Name };
}
}
else
{
invalidProperties = _left!.ValidForProperties(properties, propertyProvider);
var invalidRight = _right!.ValidForProperties(properties, propertyProvider);
if (null == invalidProperties)
{
invalidProperties = invalidRight;
bool valid = false;
valid = current._condition.ValidForProperties(properties, propertyProvider);
if (!valid)
{
invalidProperties = new string[1] { current._condition.Name };
}
}
else if (null != invalidRight)
else
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
invalidProperties = invalidProperties.Concat(invalidRight).ToArray();
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();
}
}
}
return invalidProperties;
});

return invalidProperties;
}

/// <summary>
Expand Down Expand Up @@ -265,7 +268,42 @@ internal static FilterExpression Parse(string filterString, out FastFilter? fast

return filterStack.Pop();
}
private T IterateFilterExpression<T>(Func<FilterExpression, Stack<T>, T> getNodeValue)
{
FilterExpression? current = this;
Stack<FilterExpression> filterStack = new();
Stack<T> result = new();
do
{
// Push root's right child and then root to stack then Set root as root's left child.
while (current != null)
{
if (current._right != null)
{
filterStack.Push(current._right);
}
filterStack.Push(current);
current = current._left;
}

// If the popped item has a right child and the right child is at top of stack,
// then remove the right child from stack, push the root back and set root as root's right child.
current = filterStack.Pop();
if (filterStack.Count > 0 && current._right == filterStack.Peek())
{
filterStack.Pop();
filterStack.Push(current);
current = current._right;
continue;
}

result.Push(getNodeValue(current, result));
current = null;
} while (filterStack.Count > 0);

TPDebug.Assert(result.Count == 1, "Result stack should have one element at the end.");
return result.Peek();
}
/// <summary>
/// Evaluate filterExpression with given propertyValueProvider.
/// </summary>
Expand All @@ -274,19 +312,24 @@ internal static FilterExpression Parse(string filterString, out FastFilter? fast
internal bool Evaluate(Func<string, object?> propertyValueProvider)
{
ValidateArg.NotNull(propertyValueProvider, nameof(propertyValueProvider));
bool filterResult = false;
if (null != _condition)
{
filterResult = _condition.Evaluate(propertyValueProvider);
}
else
{
// & or | operator
bool leftResult = _left!.Evaluate(propertyValueProvider);
bool rightResult = _right!.Evaluate(propertyValueProvider);
filterResult = _areJoinedByAnd ? leftResult && rightResult : leftResult || rightResult;
}
return filterResult;

return IterateFilterExpression<bool>((current, result) =>
{
bool filterResult = false;
if (null != current._condition)
{
filterResult = current._condition.Evaluate(propertyValueProvider);
}
else
{
// & or | operator
bool rightResult = current._right != null ? result.Pop() : false;
bool leftResult = current._left != null ? result.Pop() : false;
filterResult = current._areJoinedByAnd ? leftResult && rightResult : leftResult || rightResult;
}
return filterResult;
});

Evangelink marked this conversation as resolved.
Show resolved Hide resolved
}

internal static IEnumerable<string> TokenizeFilterExpressionString(string str)
Expand Down