Issue 391: OData $filter returns incorrect results for byte arrays when using Linq2Objects

12 files changed, 213 insertions, 22 deletions

diff --git a/src/System.Web.Http.OData/OData/Query/Expressions/FilterBinder.cs b/src/System.Web.Http.OData/OData/Query/Expressions/FilterBinder.cs
index da96bbf0..6e38ecdb 100644
--- a/src/System.Web.Http.OData/OData/Query/Expressions/FilterBinder.cs
+++ b/src/System.Web.Http.OData/OData/Query/Expressions/FilterBinder.cs
@@ -36,7 +36,6 @@ namespace System.Web.Http.OData.Query.Expressions
         private static readonly MethodInfo _stringCompareMethodInfo = typeof(string).GetMethod("Compare", new[] { typeof(string), typeof(string), typeof(StringComparison) });
 
         private static readonly Expression _nullConstant = Expression.Constant(null);
-        private static readonly Expression _nullStringConstant = Expression.Constant(null, typeof(string));
         private static readonly Expression _falseConstant = Expression.Constant(false);
         private static readonly Expression _trueConstant = Expression.Constant(true);
         private static readonly Expression _zeroConstant = Expression.Constant(0);
@@ -1015,8 +1014,8 @@ namespace System.Web.Http.OData.Query.Expressions
             if (left.Type == typeof(string) || right.Type == typeof(string))
             {
                 // convert nulls of type object to nulls of type string to make the String.Compare call work
-                left = ConvertNullsToString(left);
-                right = ConvertNullsToString(right);
+                left = ConvertNull(left, typeof(string));
+                right = ConvertNull(right, typeof(string));
 
                 // Use string.Compare instead of comparison for gt, ge, lt, le between two strings since direct comparisons are not supported
                 switch (binaryOperator)
@@ -1035,7 +1034,26 @@ namespace System.Web.Http.OData.Query.Expressions
 
             if (_binaryOperatorMapping.TryGetValue(binaryOperator, out binaryExpressionType))
             {
-                return Expression.MakeBinary(binaryExpressionType, left, right, liftToNull, method: null);
+                if (left.Type == typeof(byte[]) || right.Type == typeof(byte[]))
+                {
+                    left = ConvertNull(left, typeof(byte[]));
+                    right = ConvertNull(right, typeof(byte[]));
+
+                    switch (binaryExpressionType)
+                    {
+                        case ExpressionType.Equal:
+                            return Expression.MakeBinary(binaryExpressionType, left, right, liftToNull, method: Linq2ObjectsComparisonMethods.AreByteArraysEqualMethodInfo);
+                        case ExpressionType.NotEqual:
+                            return Expression.MakeBinary(binaryExpressionType, left, right, liftToNull, method: Linq2ObjectsComparisonMethods.AreByteArraysNotEqualMethodInfo);
+                        default:
+                            IEdmPrimitiveType binaryType = EdmLibHelpers.GetEdmPrimitiveTypeOrNull(typeof(byte[]));
+                            throw new ODataException(Error.Format(SRResources.BinaryOperatorNotSupported, binaryType.FullName(), binaryType.FullName(), binaryOperator));
+                    }
+                }
+                else
+                {
+                    return Expression.MakeBinary(binaryExpressionType, left, right, liftToNull, method: null);
+                }
             }
             else
             {
@@ -1073,19 +1091,6 @@ namespace System.Web.Http.OData.Query.Expressions
             }
         }
 
-        private static Expression ConvertNullsToString(Expression expression)
-        {
-            ConstantExpression constantExpression = expression as ConstantExpression;
-            if (constantExpression != null && constantExpression.Value == null)
-            {
-                return _nullStringConstant;
-            }
-            else
-            {
-                return expression;
-            }
-        }
-
         private static IEnumerable<Expression> ExtractValueFromNullableArguments(IEnumerable<Expression> arguments)
         {
             return arguments.Select(arg => ExtractValueFromNullableExpression(arg));
@@ -1243,5 +1248,18 @@ namespace System.Web.Http.OData.Query.Expressions
         {
             return type == typeof(T) || type == typeof(T?);
         }
+
+        private static Expression ConvertNull(Expression expression, Type type)
+        {
+            ConstantExpression constantExpression = expression as ConstantExpression;
+            if (constantExpression != null && constantExpression.Value == null)
+            {
+                return Expression.Constant(null, type);
+            }
+            else
+            {
+                return expression;
+            }
+        }
     }
 }
diff --git a/src/System.Web.Http.OData/OData/Query/Expressions/Linq2ObjectsComparisonMethods.cs b/src/System.Web.Http.OData/OData/Query/Expressions/Linq2ObjectsComparisonMethods.cs
new file mode 100644
index 00000000..10721255
--- /dev/null
+++ b/src/System.Web.Http.OData/OData/Query/Expressions/Linq2ObjectsComparisonMethods.cs
@@ -0,0 +1,57 @@
+// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information.
+
+using System.Reflection;
+namespace System.Web.Http.OData.Query.Expressions
+{
+    internal static class Linq2ObjectsComparisonMethods
+    {
+        /// <summary>Method info for byte array comparison.</summary>
+        public static readonly MethodInfo AreByteArraysEqualMethodInfo =
+            typeof(Linq2ObjectsComparisonMethods).GetMethod("AreByteArraysEqual");
+
+        /// <summary>Method info for byte array comparison.</summary>
+        public static readonly MethodInfo AreByteArraysNotEqualMethodInfo =
+            typeof(Linq2ObjectsComparisonMethods).GetMethod("AreByteArraysNotEqual");
+
+        /// <summary>Compares two byte arrays for equality.</summary>
+        /// <param name="left">First byte array.</param>
+        /// <param name="right">Second byte array.</param>
+        /// <returns>true if the arrays are equal; false otherwise.</returns>
+        public static bool AreByteArraysEqual(byte[] left, byte[] right)
+        {
+            if (Object.ReferenceEquals(left, right))
+            {
+                return true;
+            }
+
+            if (left == null || right == null)
+            {
+                return false;
+            }
+
+            if (left.Length != right.Length)
+            {
+                return false;
+            }
+
+            for (int i = 0; i < left.Length; i++)
+            {
+                if (left[i] != right[i])
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        /// <summary>Compares two byte arrays for equality.</summary>
+        /// <param name="left">First byte array.</param>
+        /// <param name="right">Second byte array.</param>
+        /// <returns>true if the arrays are not equal; false otherwise.</returns>
+        public static bool AreByteArraysNotEqual(byte[] left, byte[] right)
+        {
+            return !AreByteArraysEqual(left, right);
+        }
+    }
+}
diff --git a/src/System.Web.Http.OData/Properties/SRResources.Designer.cs b/src/System.Web.Http.OData/Properties/SRResources.Designer.cs
index 845e2979..db2eb2f6 100644
--- a/src/System.Web.Http.OData/Properties/SRResources.Designer.cs
+++ b/src/System.Web.Http.OData/Properties/SRResources.Designer.cs
@@ -124,6 +124,15 @@ namespace System.Web.Http.OData.Properties {
         }
         
         /// <summary>
+        ///   Looks up a localized string similar to A binary operator with incompatible types was detected. Found operand types &apos;{0}&apos; and &apos;{1}&apos; for operator kind &apos;{2}&apos;..
+        /// </summary>
+        internal static string BinaryOperatorNotSupported {
+            get {
+                return ResourceManager.GetString("BinaryOperatorNotSupported", resourceCulture);
+            }
+        }
+        
+        /// <summary>
         ///   Looks up a localized string similar to Cannot autoCreate binding because there are two or more candidate EntitySets..
         /// </summary>
         internal static string CannotAutoCreateMultipleCandidates {
diff --git a/src/System.Web.Http.OData/Properties/SRResources.resx b/src/System.Web.Http.OData/Properties/SRResources.resx
index c6d2130a..1f76d9a4 100644
--- a/src/System.Web.Http.OData/Properties/SRResources.resx
+++ b/src/System.Web.Http.OData/Properties/SRResources.resx
@@ -435,4 +435,7 @@
   <data name="KeyValueCannotBeNull" xml:space="preserve">
     <value>Key property '{0}' of type '{1}' is null. Key properties cannot have null values.</value>
   </data>
+  <data name="BinaryOperatorNotSupported" xml:space="preserve">
+    <value>A binary operator with incompatible types was detected. Found operand types '{0}' and '{1}' for operator kind '{2}'.</value>
+  </data>
 </root>
\ No newline at end of file
diff --git a/src/System.Web.Http.OData/System.Web.Http.OData.csproj b/src/System.Web.Http.OData/System.Web.Http.OData.csproj
index 127acb20..b2c65769 100644
--- a/src/System.Web.Http.OData/System.Web.Http.OData.csproj
+++ b/src/System.Web.Http.OData/System.Web.Http.OData.csproj
@@ -192,6 +192,7 @@
     <Compile Include="OData\ODataMetadataControllerConfigurationAttribute.cs" />
     <Compile Include="OData\ODataResultOfT.cs" />
     <Compile Include="OData\Query\Expressions\ClrSafeFunctions.cs" />
+    <Compile Include="OData\Query\Expressions\Linq2ObjectsComparisonMethods.cs" />
     <Compile Include="OData\Query\HandleNullPropagationOption.cs" />
     <Compile Include="OData\Query\HandleNullPropagationOptionHelper.cs" />
     <Compile Include="OData\Query\ODataQuerySettings.cs" />
diff --git a/test/System.Web.Http.OData.Test/OData/Query/Expressions/DataModel.cs b/test/System.Web.Http.OData.Test/OData/Query/Expressions/DataModel.cs
index dac21da8..fca34570 100644
--- a/test/System.Web.Http.OData.Test/OData/Query/Expressions/DataModel.cs
+++ b/test/System.Web.Http.OData.Test/OData/Query/Expressions/DataModel.cs
@@ -61,6 +61,7 @@ namespace System.Web.Http.OData.Query.Expressions
         public DateTime DateTimeProp { get; set; }
         public DateTimeOffset DateTimeOffsetProp { get; set; }
         public byte[] ByteArrayProp { get; set; }
+        public byte[] ByteArrayPropWithNullValue { get; set; }
         public TimeSpan TimeSpanProp { get; set; }
         public decimal DecimalProp { get; set; }
         public double DoubleProp { get; set; }
diff --git a/test/System.Web.Http.OData.Test/OData/Query/Expressions/FilterBinderTests.cs b/test/System.Web.Http.OData.Test/OData/Query/Expressions/FilterBinderTests.cs
index 20ca8d79..c2374b47 100644
--- a/test/System.Web.Http.OData.Test/OData/Query/Expressions/FilterBinderTests.cs
+++ b/test/System.Web.Http.OData.Test/OData/Query/Expressions/FilterBinderTests.cs
@@ -1166,8 +1166,7 @@ namespace System.Web.Http.OData.Query.Expressions
         [InlineData("UIntProp eq 12", "$it => (Convert($it.UIntProp) == Convert(12))")]
         [InlineData("CharProp eq 'a'", "$it => (Convert($it.CharProp.ToString()) == \"a\")")]
         [InlineData("CharArrayProp eq 'a'", "$it => (new String($it.CharArrayProp) == \"a\")")]
-        // [InlineData("BinaryProp eq binary'23ABFF'", "$it => ($it.BinaryProp.ToArray() == value(System.Byte[]))")] , 
-        // Issue 391: The above test doesn't work in linq2objects but works in linq2sql and EF.
+        [InlineData("BinaryProp eq binary'23ABFF'", "$it => ($it.BinaryProp.ToArray() == value(System.Byte[]))")]
         [InlineData("XElementProp eq '<name />'", "$it => ($it.XElementProp.ToString() == \"<name />\")")]
         public void NonstandardEdmPrimtives(string filter, string expression)
         {
@@ -1188,6 +1187,47 @@ namespace System.Web.Http.OData.Query.Expressions
         }
 
         [Theory]
+        [InlineData("BinaryProp eq binary'23ABFF'", "$it => ($it.BinaryProp.ToArray() == value(System.Byte[]))", true, true)]
+        [InlineData("BinaryProp ne binary'23ABFF'", "$it => ($it.BinaryProp.ToArray() != value(System.Byte[]))", false, false)]
+        [InlineData("ByteArrayProp eq binary'23ABFF'", "$it => ($it.ByteArrayProp == value(System.Byte[]))", true, true)]
+        [InlineData("ByteArrayProp ne binary'23ABFF'", "$it => ($it.ByteArrayProp != value(System.Byte[]))", false, false)]
+        [InlineData("binary'23ABFF' eq binary'23ABFF'", "$it => (value(System.Byte[]) == value(System.Byte[]))", true, true)]
+        [InlineData("binary'23ABFF' ne binary'23ABFF'", "$it => (value(System.Byte[]) != value(System.Byte[]))", false, false)]
+        [InlineData("ByteArrayPropWithNullValue ne binary'23ABFF'", "$it => ($it.ByteArrayPropWithNullValue != value(System.Byte[]))", true, true)]
+        [InlineData("ByteArrayPropWithNullValue ne ByteArrayPropWithNullValue", "$it => ($it.ByteArrayPropWithNullValue != $it.ByteArrayPropWithNullValue)", false, false)]
+        [InlineData("ByteArrayPropWithNullValue ne null", "$it => ($it.ByteArrayPropWithNullValue != null)", false, false)]
+        [InlineData("ByteArrayPropWithNullValue eq null", "$it => ($it.ByteArrayPropWithNullValue == null)", true, true)]
+        [InlineData("null ne ByteArrayPropWithNullValue", "$it => (null != $it.ByteArrayPropWithNullValue)", false, false)]
+        [InlineData("null eq ByteArrayPropWithNullValue", "$it => (null == $it.ByteArrayPropWithNullValue)", true, true)]
+        public void ByteArrayComparisons(string filter, string expression, bool withNullPropagation, object withoutNullPropagation)
+        {
+            var filters = VerifyQueryDeserialization<DataTypes>(filter, expression, NotTesting);
+            RunFilters(filters,
+                new DataTypes
+                {
+                    BinaryProp = new Binary(new byte[] { 35, 171, 255 }),
+                    ByteArrayProp = new byte[] { 35, 171, 255 }
+                },
+                new { WithNullPropagation = withNullPropagation, WithoutNullPropagation = withoutNullPropagation });
+        }
+
+        [Theory]
+        [InlineData("binary'23ABFF' ge binary'23ABFF'", "GreaterThanOrEqual")]
+        [InlineData("binary'23ABFF' le binary'23ABFF'", "LessThanOrEqual")]
+        [InlineData("binary'23ABFF' lt binary'23ABFF'", "LessThan")]
+        [InlineData("binary'23ABFF' gt binary'23ABFF'", "GreaterThan")]
+        [InlineData("binary'23ABFF' add binary'23ABFF'", "Add")]
+        [InlineData("binary'23ABFF' sub binary'23ABFF'", "Subtract")]
+        [InlineData("binary'23ABFF' mul binary'23ABFF'", "Multiply")]
+        [InlineData("binary'23ABFF' div binary'23ABFF'", "Divide")]
+        public void DisAllowed_ByteArrayComparisons(string filter, string op)
+        {
+            Assert.Throws<ODataException>(
+                () => VerifyQueryDeserialization<DataTypes>(filter),
+                Error.Format("A binary operator with incompatible types was detected. Found operand types 'Edm.Binary' and 'Edm.Binary' for operator kind '{0}'.", op));
+        }
+
+        [Theory]
         [InlineData("NullableUShortProp eq 12", "$it => (Convert($it.NullableUShortProp.Value) == Convert(12))")]
         [InlineData("NullableULongProp eq 12L", "$it => (Convert($it.NullableULongProp.Value) == Convert(12))")]
         [InlineData("NullableUIntProp eq 12", "$it => (Convert($it.NullableUIntProp.Value) == Convert(12))")]
diff --git a/test/System.Web.Http.OData.Test/OData/Query/Expressions/Linq2ObjectsComparisonMethodsTest.cs b/test/System.Web.Http.OData.Test/OData/Query/Expressions/Linq2ObjectsComparisonMethodsTest.cs
new file mode 100644
index 00000000..1337379f
--- /dev/null
+++ b/test/System.Web.Http.OData.Test/OData/Query/Expressions/Linq2ObjectsComparisonMethodsTest.cs
@@ -0,0 +1,43 @@
+// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information.
+
+using Microsoft.TestCommon;
+
+namespace System.Web.Http.OData.Query.Expressions
+{
+    public class Linq2ObjectsComparisonMethodsTest
+    {
+        public static TheoryDataSet<byte[], byte[], bool> AreByteArraysEqualDataset
+        {
+            get
+            {
+                return new TheoryDataSet<byte[], byte[], bool>
+                {
+                    { new byte[] { 1, 2, 3}, new byte[] { 1, 2, 3} , true},
+                    { new byte[] { 1,2,3}, new byte[] { 1, 2} , false},
+                    { new byte[] { 1,2}, new byte[] { 1, 2, 3} , false},
+                    { null, new byte[] { 1, 2, 3} , false},
+                    { new byte[] { 1, 2, 3}, null , false},
+                    { null, null , true},
+                };
+            }
+        }
+
+        [Theory]
+        [PropertyData("AreByteArraysEqualDataset")]
+        public void AreByteArraysEqual(byte[] left, byte[] right, bool result)
+        {
+            Assert.Equal(
+                result,
+                Linq2ObjectsComparisonMethods.AreByteArraysEqual(left, right));
+        }
+
+        [Theory]
+        [PropertyData("AreByteArraysEqualDataset")]
+        public void AreByteArraysNotEqual(byte[] left, byte[] right, bool result)
+        {
+            Assert.Equal(
+                !result,
+                Linq2ObjectsComparisonMethods.AreByteArraysNotEqual(left, right));
+        }
+    }
+}
diff --git a/test/System.Web.Http.OData.Test/OData/Query/PartialTrustTest.cs b/test/System.Web.Http.OData.Test/OData/Query/PartialTrustTest.cs
index 3cc42a68..5ea1f0fe 100644
--- a/test/System.Web.Http.OData.Test/OData/Query/PartialTrustTest.cs
+++ b/test/System.Web.Http.OData.Test/OData/Query/PartialTrustTest.cs
@@ -7,10 +7,11 @@ namespace System.Web.Http.OData.Query
     [PartialTrustRunner]
     public class PartialTrustTest : QueryCompositionTests
     {
-        [Fact(Skip="Moq doesn't work in partial trust")]
+        [Fact]
         public override void QueryableUsesConfiguredAssembliesResolver()
         {
-            throw new NotImplementedException();
+            // NO-OP
+            // Moq doesn't work in partial trust
         }
     }
 }
diff --git a/test/System.Web.Http.OData.Test/OData/Query/QueryCompositionControllers.cs b/test/System.Web.Http.OData.Test/OData/Query/QueryCompositionControllers.cs
index bc9adce5..c5d035e9 100644
--- a/test/System.Web.Http.OData.Test/OData/Query/QueryCompositionControllers.cs
+++ b/test/System.Web.Http.OData.Test/OData/Query/QueryCompositionControllers.cs
@@ -22,7 +22,8 @@ namespace System.Web.Http.OData.Query
                     {
                         new QueryCompositionCustomer { Id = 111, Name = "Primary" },
                         new QueryCompositionCustomer { Id = 112, Name = "Secondary" },
-                    }
+                    },
+                    Image = new byte[] { 1, 2, 3 }
                 }, 
                 new QueryCompositionCustomer 
                 { 
@@ -184,6 +185,7 @@ namespace System.Web.Http.OData.Query
         public QueryCompositionCustomer RelationshipManager { get; set; }
         public IEnumerable<string> Tags { get; set; }
         public IEnumerable<QueryCompositionCustomer> Contacts { get; set; }
+        public byte[] Image { get; set; }
     }
 
     public class QueryCompositionAddress
diff --git a/test/System.Web.Http.OData.Test/OData/Query/QueryCompositionTests.cs b/test/System.Web.Http.OData.Test/OData/Query/QueryCompositionTests.cs
index 8e476beb..a5b03dfd 100644
--- a/test/System.Web.Http.OData.Test/OData/Query/QueryCompositionTests.cs
+++ b/test/System.Web.Http.OData.Test/OData/Query/QueryCompositionTests.cs
@@ -353,6 +353,21 @@ namespace System.Web.Http.OData.Query
                 String.Join(",", customers.Select(customer => customer.Id)));
         }
 
+        [Fact]
+        public void QueryableWorksWith_ByteArrayComparison()
+        {
+            HttpServer server = new HttpServer(InitializeConfiguration("QueryCompositionCustomer", useCustomEdmModel: false));
+            HttpClient client = new HttpClient(server);
+
+            // unsupported operator starting with $ - throws
+            HttpResponseMessage response = client.GetAsync("http://localhost:8080/QueryCompositionCustomer/?$filter=Image eq binary'010203'").Result;
+
+            // using low level api works fine
+            response.EnsureSuccessStatusCode();
+            List<QueryCompositionCustomer> customers = response.Content.ReadAsAsync<List<QueryCompositionCustomer>>().Result;
+            Assert.Equal(new[] { 11 }, customers.Select(customer => customer.Id));
+        }
+
         private static HttpConfiguration InitializeConfiguration(string controllerName, bool useCustomEdmModel)
         {
             HttpConfiguration config = new HttpConfiguration();
diff --git a/test/System.Web.Http.OData.Test/System.Web.Http.OData.Test.csproj b/test/System.Web.Http.OData.Test/System.Web.Http.OData.Test.csproj
index a2d74690..a717895f 100644
--- a/test/System.Web.Http.OData.Test/System.Web.Http.OData.Test.csproj
+++ b/test/System.Web.Http.OData.Test/System.Web.Http.OData.Test.csproj
@@ -138,6 +138,7 @@
     <Compile Include="OData\Builder\Conventions\Attributes\AttributeEdmPropertyConventionTests.cs" />
     <Compile Include="OData\Builder\Conventions\Attributes\NotMappedAttributeConventionTests.cs" />
     <Compile Include="OData\Builder\EdmModelAsserts.cs" />
+    <Compile Include="OData\Query\Expressions\Linq2ObjectsComparisonMethodsTest.cs" />
     <Compile Include="OData\Query\HandleNullPropagationOptionHelperTest.cs" />
     <Compile Include="OData\Query\ODataQuerySettingsTest.cs" />
     <Compile Include="OData\Query\OrderByPropertyNodeTest.cs" />