Error when SkipToken contains a nullable string

Hello,

This issue is copied from repository OData/WebAPI after I found that the fix should be made in AspNetCoreOData.

Original issue is #2041 from OData/WebApi.

I run into the same issue. So I'm to trying to implement paging functionality into my application and thus I using the odata generated next link in the result of a OData WEB API call.

"@odata.nextLink": "http://127.0.0.1:50057/odata/views/ReportTranslations?$count=true&$orderby=Code&$skiptoken=Code-%27ADDRESSLINES%27,LanguageID-2,ReportName-null,Translation-%27Addresslines%27"

As mentioned before the error occurs when null value is used in the uri. In my sample ReportName-null.

I have investigated this in de OData source code and the problem occurs in DefaultSkipTokenHandler.ApplyToCore.

Specifically this code.

foreach (KeyValuePair<string, object> item in propertyValuePairs)
            {
                string key = item.Key;
                MemberExpression property = Expression.Property(param, key);
                object value = item.Value;

                Expression compare = null;
                ODataEnumValue enumValue = value as ODataEnumValue;
                if (enumValue != null)
                {
                    value = enumValue.Value;
                }

                Expression constant = parameterizeConstant ? LinqParameterContainer.Parameterize(value.GetType(), value) : Expression.Constant(value);
                if (directionMap.ContainsKey(key) && directionMap[key] == OrderByDirection.Descending)
                {
                    compare = ExpressionBinderHelper.CreateBinaryExpression(BinaryOperatorKind.LessThan, property, constant, true, querySettings);
                }
                else
                {
                    compare = ExpressionBinderHelper.CreateBinaryExpression(BinaryOperatorKind.GreaterThan, property, constant, true, querySettings);
                }

The CreateBinaryExpression fails because the constant variable refers to ODataNull which it does not support. It does a string compare internally that expects a value or null.

The ODataNull originals from the following code.

IDictionary<string, object> propertyValuePairs = PopulatePropertyValuePairs(skipTokenRawValue, context);

Which is called prior to iterating through all skiptoken values.

I could simple solve this by calling the following code prior to calling CreateBinaryExpression.

Expression constant = null;
                if (value.GetType() == typeof(ODataNullValue))
                {
                    constant = parameterizeConstant ? LinqParameterContainer.Parameterize(typeof(string), null) : Expression.Constant(value);
                }
                else
                    constant = parameterizeConstant ? LinqParameterContainer.Parameterize(value.GetType(), value) : Expression.Constant(value);

So basically I'm converting ODataNull to a null string which the CreateBinaryExpression support.

I know this is not a clean solution and more like a quick fix, because I'm assuming the data type to be a string. I would prefer a change to PopulatePropertyValuePairs to also return the data type of each skiptoken value. It checks the EdmModel so it has the information. Having the type the proper Expression constant variable could be created. The result is the same.

Also I would prefer to have this fixed in the OData code and added to the next release. It does not seem to be a big change. So would be great if this could be picked up.

As a temporary solution I'm busy implementing my own version of a SkipTokenHandler. Using dependency injection I can add my own implementation but in order to fix the code I have to basically copy a lot of code for a fairly simple code change. I cannot simple copy DefaultSkipTokenHandler and change the code, A lot of the dependent code is internal. So I have to copy a lot of classes let only access to the resource file containing the error messages.

Originally posted by @nicovos in https://github.com/OData/WebApi/issues/2041#issuecomment-1067764917