diff --git a/src/Modules/Grand.Module.Api/Attributes/EnableQueryAttribute.cs b/src/Modules/Grand.Module.Api/Attributes/EnableQueryAttribute.cs
index f2d108cc5..c2132a961 100644
--- a/src/Modules/Grand.Module.Api/Attributes/EnableQueryAttribute.cs
+++ b/src/Modules/Grand.Module.Api/Attributes/EnableQueryAttribute.cs
@@ -1,8 +1,10 @@
-using Microsoft.AspNetCore.Mvc.Filters;
+using Grand.Module.Api.Constants;
+using Grand.Module.Api.Queries;
+using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.Filters;
using System.Linq.Dynamic.Core;
-using Microsoft.AspNetCore.Http;
-using Grand.Module.Api.Constants;
+using System.Linq.Dynamic.Core.Exceptions;
namespace Grand.Module.Api.Attributes;
@@ -14,34 +16,62 @@ public override void OnActionExecuted(ActionExecutedContext context)
if (context.Result is not ObjectResult result || result.Value == null)
return;
- if (result.Value is IQueryable queryable)
+ if (result.Value is not IQueryable queryable)
+ return;
+
+ try
+ {
+ result.Value = ApplyQueryOptions(queryable, context.HttpContext.Request.Query);
+ }
+ catch (ApiQueryOptionException ex)
+ {
+ //a rejected query option is the client's mistake, not ours - this filter runs after the
+ //action, so without this the exception would leave the pipeline as a 500
+ context.Result = new BadRequestObjectResult(new { error = ex.Message });
+ }
+ catch (ParseException ex)
{
- queryable = ApplyQueryOptions(queryable, context.HttpContext.Request.Query, context.HttpContext.Response);
- result.Value = queryable;
+ context.Result = new BadRequestObjectResult(new { error = $"$filter could not be parsed: {ex.Message}" });
}
}
- private static IQueryable ApplyQueryOptions(IQueryable queryable, IQueryCollection query, HttpResponse response)
+ private static IQueryable ApplyQueryOptions(IQueryable queryable, IQueryCollection query)
{
+ var elementType = queryable.ElementType;
+
if (query.TryGetValue("$filter", out var filter))
- queryable = queryable.Where(filter.ToString());
+ {
+ ApiQueryOptions.ValidateFilter(filter.ToString(), elementType);
+ queryable = queryable.Where(ApiQueryOptions.FilterConfig, filter.ToString());
+ }
if (query.TryGetValue("$orderby", out var orderBy))
- queryable = queryable.OrderBy(orderBy.ToString());
+ queryable = queryable.OrderBy(ApiQueryOptions.FilterConfig,
+ ApiQueryOptions.ParseOrderBy(orderBy.ToString(), elementType));
if (query.TryGetValue("$select", out var select))
- queryable = queryable.Select($"new({select})");
+ queryable = queryable.Select(ApiQueryOptions.SelectConfig,
+ ApiQueryOptions.ParseSelect(select.ToString(), elementType));
+
+ if (query.TryGetValue("$skip", out var skipValue))
+ {
+ if (!int.TryParse(skipValue, out var skip) || skip < 0)
+ throw new ApiQueryOptionException("$skip must be a non-negative integer");
- if (query.TryGetValue("$skip", out var skipValue) && int.TryParse(skipValue, out var skip))
queryable = queryable.Skip(skip);
+ }
- if (query.TryGetValue("$top", out var topValue) && int.TryParse(topValue, out var top))
+ if (query.TryGetValue("$top", out var topValue))
{
- top = Math.Min(top, Configurations.MaxLimit);
- queryable = queryable.Take(top);
+ if (!int.TryParse(topValue, out var top) || top < 0)
+ throw new ApiQueryOptionException("$top must be a non-negative integer");
+
+ queryable = queryable.Take(Math.Min(top, Configurations.MaxLimit));
}
else
+ {
queryable = queryable.Take(Configurations.MaxLimit);
+ }
return queryable;
}
diff --git a/src/Modules/Grand.Module.Api/Queries/ApiQueryOptions.cs b/src/Modules/Grand.Module.Api/Queries/ApiQueryOptions.cs
new file mode 100644
index 000000000..c06dcdc76
--- /dev/null
+++ b/src/Modules/Grand.Module.Api/Queries/ApiQueryOptions.cs
@@ -0,0 +1,203 @@
+using System.Linq.Dynamic.Core;
+using System.Linq.Dynamic.Core.CustomTypeProviders;
+using System.Reflection;
+using System.Text.RegularExpressions;
+
+namespace Grand.Module.Api.Queries;
+
+///
+/// Raised when a client sends a query option the API refuses to run. Mapped to 400 by the caller;
+/// it is never an internal error, so it must not surface as 500.
+///
+public class ApiQueryOptionException(string message) : Exception(message);
+
+///
+/// Parses and restricts the OData-like query options.
+/// $filter reaches a real expression parser, so it is fenced on three sides: a parsing config that
+/// denies types, context keywords and object construction; a length limit; and a whitelist that
+/// accepts only members of the projected model. $orderby and $select never reach the parser as raw
+/// text - they are read as field lists and rebuilt here.
+///
+public static class ApiQueryOptions
+{
+ ///
+ /// Long enough for a realistic filter, short enough that a pathological expression cannot make
+ /// the parser the slow part of the request.
+ ///
+ public const int MaxFilterLength = 512;
+
+ public const int MaxFields = 50;
+
+ ///
+ /// Methods a filter may name. Everything here is a cheap, side-effect free string or text
+ /// operation; nothing that reflects, constructs, or walks a collection.
+ ///
+ private static readonly HashSet AllowedMethods = new(StringComparer.OrdinalIgnoreCase) {
+ "Contains", "StartsWith", "EndsWith", "ToLower", "ToUpper", "Trim", "Length", "Equals"
+ };
+
+ ///
+ /// Operators and literals the parser understands that are not members of the model.
+ ///
+ private static readonly HashSet Keywords = new(StringComparer.OrdinalIgnoreCase) {
+ "and", "or", "not", "true", "false", "null", "iif"
+ };
+
+ private static readonly Regex Identifier = new(@"[A-Za-z_][A-Za-z0-9_]*", RegexOptions.Compiled);
+
+ ///
+ /// String literals hold user text, not member names, so they are removed before the whitelist
+ /// runs - otherwise a product named "Password" would fail its own search.
+ ///
+ private static readonly Regex StringLiteral = new(@"""(?:[^""\\]|\\.)*""|'(?:[^'\\]|\\.)*'", RegexOptions.Compiled);
+
+ ///
+ /// Denies everything the parser can reach outside the model: no type resolution, no `it`/`root`
+ /// context keywords, no `new`, no assembly probing, no Equals/ToString on object.
+ ///
+ public static ParsingConfig FilterConfig { get; } = new() {
+ AreContextKeywordsEnabled = false,
+ AllowNewToEvaluateAnyType = false,
+ DisallowNewKeyword = true,
+ ResolveTypesBySimpleName = false,
+ SupportCastingToFullyQualifiedTypeAsString = false,
+ LoadAdditionalAssembliesFromCurrentDomainBaseDirectory = false,
+ AllowEqualsAndToStringMethodsOnObject = false,
+ RestrictOrderByToPropertyOrField = true,
+ CustomTypeProvider = new NoCustomTypesProvider()
+ };
+
+ ///
+ /// $select is rebuilt from validated field names, so `new` has to be available for that one
+ /// projection - and for nothing else.
+ ///
+ public static ParsingConfig SelectConfig { get; } = new() {
+ AreContextKeywordsEnabled = false,
+ AllowNewToEvaluateAnyType = false,
+ ResolveTypesBySimpleName = false,
+ SupportCastingToFullyQualifiedTypeAsString = false,
+ LoadAdditionalAssembliesFromCurrentDomainBaseDirectory = false,
+ AllowEqualsAndToStringMethodsOnObject = false,
+ RestrictOrderByToPropertyOrField = true,
+ CustomTypeProvider = new NoCustomTypesProvider()
+ };
+
+ ///
+ /// Checks that a filter names nothing outside the model it runs against.
+ ///
+ public static void ValidateFilter(string filter, Type elementType)
+ {
+ if (string.IsNullOrWhiteSpace(filter))
+ throw new ApiQueryOptionException("$filter is empty");
+
+ if (filter.Length > MaxFilterLength)
+ throw new ApiQueryOptionException($"$filter exceeds {MaxFilterLength} characters");
+
+ var members = MemberNames(elementType);
+ var expression = StringLiteral.Replace(filter, " ");
+
+ foreach (Match match in Identifier.Matches(expression))
+ {
+ var name = match.Value;
+ if (Keywords.Contains(name) || AllowedMethods.Contains(name) || members.Contains(name))
+ continue;
+
+ throw new ApiQueryOptionException($"'{name}' is not a queryable field of {elementType.Name}");
+ }
+ }
+
+ ///
+ /// Reads "Field asc, Other desc" and gives it back with every field checked against the model.
+ ///
+ public static string ParseOrderBy(string orderBy, Type elementType)
+ {
+ var members = MemberNames(elementType);
+ var parts = SplitFields(orderBy, "$orderby");
+ var ordering = new List(parts.Count);
+
+ foreach (var part in parts)
+ {
+ var tokens = part.Split(' ', StringSplitOptions.RemoveEmptyEntries);
+ if (tokens.Length > 2)
+ throw new ApiQueryOptionException($"'{part}' is not a valid $orderby entry");
+
+ if (!members.Contains(tokens[0]))
+ throw new ApiQueryOptionException($"'{tokens[0]}' is not a queryable field of {elementType.Name}");
+
+ var direction = tokens.Length == 2 ? tokens[1].ToLowerInvariant() : "asc";
+ if (direction != "asc" && direction != "desc")
+ throw new ApiQueryOptionException($"'{tokens[1]}' is not a sort direction");
+
+ ordering.Add($"{tokens[0]} {direction}");
+ }
+
+ return string.Join(", ", ordering);
+ }
+
+ ///
+ /// Reads a field list and builds the projection itself, so no client text reaches the parser.
+ ///
+ public static string ParseSelect(string select, Type elementType)
+ {
+ var members = MemberNames(elementType);
+ var fields = SplitFields(select, "$select");
+
+ foreach (var field in fields)
+ if (!members.Contains(field))
+ throw new ApiQueryOptionException($"'{field}' is not a queryable field of {elementType.Name}");
+
+ return $"new({string.Join(", ", fields)})";
+ }
+
+ private static List SplitFields(string value, string option)
+ {
+ if (string.IsNullOrWhiteSpace(value))
+ throw new ApiQueryOptionException($"{option} is empty");
+
+ var fields = value.Split(',', StringSplitOptions.RemoveEmptyEntries)
+ .Select(x => x.Trim())
+ .Where(x => x.Length > 0)
+ .ToList();
+
+ if (fields.Count == 0)
+ throw new ApiQueryOptionException($"{option} is empty");
+
+ if (fields.Count > MaxFields)
+ throw new ApiQueryOptionException($"{option} lists more than {MaxFields} fields");
+
+ return fields;
+ }
+
+ private static HashSet MemberNames(Type elementType)
+ {
+ return new HashSet(
+ elementType.GetProperties(BindingFlags.Public | BindingFlags.Instance).Select(x => x.Name),
+ StringComparer.OrdinalIgnoreCase);
+ }
+
+ ///
+ /// Leaves the parser with no types to resolve at all.
+ ///
+ private class NoCustomTypesProvider : IDynamicLinqCustomTypeProvider
+ {
+ public HashSet GetCustomTypes()
+ {
+ return [];
+ }
+
+ public Dictionary> GetExtensionMethods()
+ {
+ return [];
+ }
+
+ public Type ResolveType(string typeName)
+ {
+ return null;
+ }
+
+ public Type ResolveTypeBySimpleName(string simpleTypeName)
+ {
+ return null;
+ }
+ }
+}
diff --git a/src/Tests/Grand.Module.Api.Tests/Queries/ApiQueryOptionsTests.cs b/src/Tests/Grand.Module.Api.Tests/Queries/ApiQueryOptionsTests.cs
new file mode 100644
index 000000000..2ac02abb4
--- /dev/null
+++ b/src/Tests/Grand.Module.Api.Tests/Queries/ApiQueryOptionsTests.cs
@@ -0,0 +1,101 @@
+using Grand.Module.Api.DTOs.Catalog;
+using Grand.Module.Api.Queries;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System.Linq.Dynamic.Core;
+using Assert = Microsoft.VisualStudio.TestTools.UnitTesting.Assert;
+
+namespace Grand.Module.Api.Tests.Queries;
+
+[TestClass]
+public class ApiQueryOptionsTests
+{
+ private static readonly Type ElementType = typeof(ProductDto);
+
+ [TestMethod]
+ public void Filter_AcceptsAFieldOfTheModel()
+ {
+ ApiQueryOptions.ValidateFilter("Name.Contains(\"shirt\") and Published == true", ElementType);
+ }
+
+ [TestMethod]
+ public void Filter_KeepsALiteralOutOfTheFieldCheck()
+ {
+ //the text is data, not a member name - a product called "Password" must stay searchable
+ ApiQueryOptions.ValidateFilter("Name == \"Password\"", ElementType);
+ }
+
+ [TestMethod]
+ [DataRow("PasswordHash != null", DisplayName = "field the model does not expose")]
+ [DataRow("it.GetType().Assembly != null", DisplayName = "reflection through the context keyword")]
+ [DataRow("\"\".GetType().Assembly.GetTypes().Length > 0", DisplayName = "type walk from a literal")]
+ [DataRow("System.IO.File.ReadAllText(\"appsettings.json\") != null", DisplayName = "fully qualified type")]
+ [DataRow("new(Name as X).X != null", DisplayName = "object construction")]
+ [DataRow("Name.Equals(Name, StringComparison.Ordinal)", DisplayName = "type reference in an argument")]
+ public void Filter_RejectsAnythingOutsideTheModel(string filter)
+ {
+ Assert.ThrowsExactly(() => ApiQueryOptions.ValidateFilter(filter, ElementType));
+ }
+
+ [TestMethod]
+ public void Filter_RejectsAnOverlongExpression()
+ {
+ var filter = string.Join(" or ", Enumerable.Repeat("Name == \"x\"", 100));
+
+ var ex = Assert.ThrowsExactly(
+ () => ApiQueryOptions.ValidateFilter(filter, ElementType));
+
+ StringAssert.Contains(ex.Message, "characters");
+ }
+
+ ///
+ /// The whitelist decides what runs; this checks the parser it runs on is locked down too, so a
+ /// future change to the whitelist cannot quietly re-open type access.
+ ///
+ [TestMethod]
+ public void FilterConfig_RefusesToResolveTypes()
+ {
+ var source = new[] { new ProductDto { Name = "shirt" } }.AsQueryable();
+
+ Assert.ThrowsExactly(
+ () => source.Where(ApiQueryOptions.FilterConfig, "it.GetType().Name == \"ProductDto\"").ToList());
+ }
+
+ [TestMethod]
+ public void OrderBy_NormalizesDirectionAndDefaultsToAscending()
+ {
+ Assert.AreEqual("Name asc, Sku desc", ApiQueryOptions.ParseOrderBy("Name, Sku DESC", ElementType));
+ }
+
+ [TestMethod]
+ [DataRow("PasswordHash")]
+ [DataRow("Name sideways")]
+ [DataRow("Name asc extra")]
+ [DataRow("")]
+ public void OrderBy_RejectsWhatItCannotVerify(string orderBy)
+ {
+ Assert.ThrowsExactly(() => ApiQueryOptions.ParseOrderBy(orderBy, ElementType));
+ }
+
+ [TestMethod]
+ public void Select_BuildsTheProjectionFromCheckedFields()
+ {
+ Assert.AreEqual("new(Id, Name)", ApiQueryOptions.ParseSelect("Id, Name", ElementType));
+ }
+
+ [TestMethod]
+ [DataRow("PasswordHash", DisplayName = "field the model does not expose")]
+ [DataRow("Name as Alias", DisplayName = "expression rather than a field")]
+ [DataRow("it.GetType()", DisplayName = "reflection")]
+ public void Select_RejectsAnythingThatIsNotAPlainField(string select)
+ {
+ Assert.ThrowsExactly(() => ApiQueryOptions.ParseSelect(select, ElementType));
+ }
+
+ [TestMethod]
+ public void Select_RejectsMoreFieldsThanTheLimit()
+ {
+ var select = string.Join(",", Enumerable.Range(0, ApiQueryOptions.MaxFields + 1).Select(_ => "Name"));
+
+ Assert.ThrowsExactly(() => ApiQueryOptions.ParseSelect(select, ElementType));
+ }
+}