OData / WebApi

OData Web API: A server library built upon ODataLib and WebApi
https://docs.microsoft.com/odata
Other
856 stars 473 forks source link

Unable to read DateTimeOffset type from ODataQueryOptions when ODataQuerySettings.EnableConstantParameterization = false #2690

Open shagilt opened 2 years ago

shagilt commented 2 years ago

Unable to read DateTimeOffset type from ODataQueryOptions when ODataQuerySettings.EnableConstantParameterization = false

My ODataQuerySettings.EnableConstantParameterization is set to false. I am seeing below error when ExpressionBinderBase.cs is trying to read DateTimeOffset type from ODataQueryOptions. I don’t see this issue when EnableConstantParameterization is set to true.

System.InvalidOperationException: The binary operator GreaterThan is not defined for the types 'System.Nullable1[System.DateTime]' and 'System.Nullable1[System.DateTimeOffset]'. at System.Linq.Expressions.Expression.GetUserDefinedBinaryOperatorOrThrow(ExpressionType binaryType, String name, Expression left, Expression right, Boolean liftToNull) at System.Linq.Expressions.Expression.GetComparisonOperator(ExpressionType binaryType, String opName, Expression left, Expression right, Boolean liftToNull) at System.Linq.Expressions.Expression.GreaterThan(Expression left, Expression right, Boolean liftToNull, MethodInfo method) at System.Linq.Expressions.Expression.MakeBinary(ExpressionType binaryType, Expression left, Expression right, Boolean liftToNull, MethodInfo method, LambdaExpression conversion) at System.Linq.Expressions.Expression.MakeBinary(ExpressionType binaryType, Expression left, Expression right, Boolean liftToNull, MethodInfo method) at Microsoft.AspNet.OData.Query.Expressions.ExpressionBinderBase.CreateBinaryExpression(BinaryOperatorKind binaryOperator, Expression left, Expression right, Boolean liftToNull, Boolean containsDateFunction) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindBinaryOperatorNode(BinaryOperatorNode binaryOperatorNode) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindSingleValueNode(SingleValueNode node) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.Bind(QueryNode node) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindBinaryOperatorNode(BinaryOperatorNode binaryOperatorNode) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindSingleValueNode(SingleValueNode node) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.Bind(QueryNode node) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindBinaryOperatorNode(BinaryOperatorNode binaryOperatorNode) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindSingleValueNode(SingleValueNode node) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.Bind(QueryNode node) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindExpression(SingleValueNode expression, RangeVariable rangeVariable, Type elementType) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindFilterClause(FilterBinder binder, FilterClause filterClause, Type filterType) at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.Bind(IQueryable baseQuery, FilterClause filterClause, Type filterType, ODataQueryContext context, ODataQuerySettings querySettings) at Microsoft.AspNet.OData.Query.FilterQueryOption.ApplyTo(IQueryable query, ODataQuerySettings querySettings) -----Removed code------ at lambda_method(Closure , Object ) at Microsoft.Extensions.Internal.ObjectMethodExecutorAwaitable.Awaiter.GetResult() at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.AwaitableObjectResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments) at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.g__Awaited|12_0(ControllerActionInvoker invok--TRUNCATED--

When EnableConstantParameterization is false, I think left variable in CreateBinaryExpression() is MemberAccess and code to handle this type is missing. Maybe we can get DateTime from PropertyInfo like this - ((PropertyInfo)(left as MemberExpression).Member?

Version: Microsoft.AspNetCore.OData 7.5.6

corranrogue9 commented 2 years ago

@shagilt, I'm seeing this code which could potentially be the issue (deals with the DateTime vs DateTimeOffset comparison, uses the EnableConstantParameterization flag, only matches the types exactly, and not the nullable variants), but I don't see why that code would work with the flag enabled if it doesn't work when the flag is disabled. Can you provide your model and a sample request so I can repro?

shagilt commented 2 years ago

My entity name is Film:

public class Film
    {
        [Key]
        public string ID { get; set; }
        public string Title { get; set; }
        public int Rating { get; set; }
        public DateTime ReleaseDateTime { get; set; } = DateTime.MinValue;
    }

This my controller definition and get ODataQueryOptions. public IActionResult Get(ODataQueryOptions<Film> options)

We do some validation before calling the storage layer by doing below code, and error is thrown when odataQueryOptions.Filter.ApplyTo is called.

           var settings = new ODataValidationSettings();

            // Arithmetic operators to allow for querying using $filter.
            settings.AllowedArithmeticOperators =
                AllowedArithmeticOperators.None;

            settings.AllowedFunctions =
                AllowedFunctions.Contains |
                AllowedFunctions.ToLower |
                AllowedFunctions.Any;

            if(this.ODataVersion == ODataVersion.V4)
            {
                settings.AllowedFunctions = settings.AllowedFunctions | AllowedFunctions.Cast;
            }

            settings.AllowedLogicalOperators =
                AllowedLogicalOperators.And |
                AllowedLogicalOperators.Or |
                AllowedLogicalOperators.Not |
                AllowedLogicalOperators.Equal |
                AllowedLogicalOperators.GreaterThan |
                AllowedLogicalOperators.GreaterThanOrEqual |
                AllowedLogicalOperators.LessThan |
                AllowedLogicalOperators.LessThanOrEqual |
                AllowedLogicalOperators.NotEqual;

            // do not allow expand
            settings.AllowedQueryOptions =
                AllowedQueryOptions.Filter |
                AllowedQueryOptions.OrderBy |
                AllowedQueryOptions.Skip |
                AllowedQueryOptions.Top |
                AllowedQueryOptions.SkipToken |
                AllowedQueryOptions.Filter |
                AllowedQueryOptions.Select |
                AllowedQueryOptions.Count;

            // Validate settings after getting query details for case sensitive schema
            options.Validate(settings);

            if (options.Filter != null)
            {
                // construct query terms list
                var odataQuerySettings = new ODataQuerySettings();
                odataQuerySettings.EnsureStableOrdering = false;
                odataQuerySettings.HandleNullPropagation = HandleNullPropagationOption.True;
                odataQuerySettings.PageSize = 100;
                odataQuerySettings.EnableConstantParameterization = false; <<-- No Error if this is true
                IList list = Activator.CreateInstance(typeof(List<>).MakeGenericType(this.TargetType)) as IList;
                IQueryable queryable = options.Filter.ApplyTo(list.AsQueryable(), odataQuerySettings); // <<-- Error from here
             }
corranrogue9 commented 2 years ago

Thanks, I'll take a look to see if I can repro. @chrisspre had some reservations that the higher-level design shouldn't allow such a bug to happen in the first place. I will loop back with him to see if we can do some refactoring.