Fix NRE in case of invalid, nested sort property

GeorgDangl · Oct 16, 2019 · 42d120d · 42d120d
1 parent d08a99f
commit 42d120d
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 All notable changes to **LightQuery** are documented here.
 
+## v1.7.2:
+- Fix possible `NullReferenceException` in case of relational sorting where an invalid property name is passed via the query. Thanks to GitHub user @smitpatel for discovering it!
+
 ## v1.7.1:
 - Fixed a bug that caused Entity Framework Core to throw an `InvalidOperationException` when sorting was applied to projections to a class that inherited from the base query type. The error was an incorrectly used reflection invocation to determine the type the query applies to
 

diff --git a/src/LightQuery.Shared/QueryableProcessor.cs b/src/LightQuery.Shared/QueryableProcessor.cs
@@ -1,6 +1,4 @@
 using System;
-using System.Collections;
-using System.Collections.Generic;
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
@@ -11,7 +9,7 @@ public static class QueryableProcessor
     {
         private static (Type declaringType, PropertyInfo property) GetPropertyInfoRecursively(this IQueryable queryable, string propName)
         {
-            string[] nameParts = propName.Split('.');
+            var nameParts = propName.Split('.');
             if (nameParts.Length == 1)
             {
                 var property = queryable.ElementType.GetTypeInfo().GetProperty(CamelizeString(propName)) ?? queryable.ElementType.GetTypeInfo().GetProperty(propName);
@@ -20,12 +18,12 @@ private static (Type declaringType, PropertyInfo property) GetPropertyInfoRecurs
 
             //Getting Root Property - Ex : propName : "User.Name" -> User
             var propertyInfo = queryable.ElementType.GetTypeInfo().GetProperty(CamelizeString(nameParts[0])) ?? queryable.ElementType.GetTypeInfo().GetProperty(nameParts[0]);
-            var originalDeclaringType = propertyInfo.DeclaringType;
             if (propertyInfo == null)
             {
                 return (null, null);
             }
 
+            var originalDeclaringType = propertyInfo.DeclaringType;
             for (int i = 1; i < nameParts.Length; i++)
             {
                 propertyInfo = propertyInfo.PropertyType.GetProperty(CamelizeString(nameParts[i])) ?? propertyInfo.PropertyType.GetProperty(nameParts[i]);
@@ -82,7 +80,7 @@ public static IQueryable ApplySorting(this IQueryable queryable, QueryOptions qu
             queryable = queryable.WrapInNullChecksIfAccessingNestedProperties(queryable.ElementType, queryOptions.SortPropertyName);
             var wrappedExpression = Expression.Call(typeof(Queryable),
                 orderMethodName,
-                new [] { orderingProperty.declaringType, orderingProperty.property.PropertyType },
+                new[] { orderingProperty.declaringType, orderingProperty.property.PropertyType },
                 queryable.Expression,
                 Expression.Quote(orderByExp));
             var result = queryable.Provider.CreateQuery(wrappedExpression);

diff --git a/...y.EntityFrameworkCore.Tests.Integration/ControllerTests/AsyncLightQueryControllerTests.cs b/...y.EntityFrameworkCore.Tests.Integration/ControllerTests/AsyncLightQueryControllerTests.cs
@@ -332,5 +332,21 @@ public async Task CanSortByNestedProperty()
             Assert.Equal("Iris", actualResponse[1].UserName);
             Assert.Equal("Hank", actualResponse[2].UserName);
         }
+
+        [Fact]
+        public async Task DoesNotReturnErrorButUnsortedListInCaseOfInvalidPropertyName()
+        {
+            var url = "AsyncLightQuery?sort=unknownProperty";
+            var actualResponse = await GetResponse<List<User>>(url);
+            Assert.NotEmpty(actualResponse);
+        }
+
+        [Fact]
+        public async Task DoesNotReturnErrorButUnsortedListInCaseOfInvalidPropertyNameWithRelationalSorting()
+        {
+            var url = "AsyncLightQuery?sort=unknownProperty.name";
+            var actualResponse = await GetResponse<List<User>>(url);
+            Assert.NotEmpty(actualResponse);
+        }
     }
 }
diff --git a/test/LightQuery.Tests.Integration/ControllerTests/LightQueryControllerTests.cs b/test/LightQuery.Tests.Integration/ControllerTests/LightQueryControllerTests.cs
@@ -332,5 +332,21 @@ public async Task CanSortByNestedProperty()
             Assert.Equal("Iris", actualResponse[1].UserName);
             Assert.Equal("Hank", actualResponse[2].UserName);
         }
+
+        [Fact]
+        public async Task DoesNotReturnErrorButUnsortedListInCaseOfInvalidPropertyName()
+        {
+            var url = "LightQuery?sort=unknownProperty";
+            var actualResponse = await GetResponse<List<User>>(url);
+            Assert.NotEmpty(actualResponse);
+        }
+
+        [Fact]
+        public async Task DoesNotReturnErrorButUnsortedListInCaseOfInvalidPropertyNameWithRelationalSorting()
+        {
+            var url = "LightQuery?sort=unknownProperty.name";
+            var actualResponse = await GetResponse<List<User>>(url);
+            Assert.NotEmpty(actualResponse);
+        }
     }
 }