fix(parsing): guard unsafe string parsing in filter parsers (#58)

- Add validation before Substring operations to prevent index exceptions
- Replace int.Parse with TryParse for group indices in logical group
parsing
- Add ILogger? parameter to parser methods for warning on malformed
input
- Malformed filter keys are logged and skipped instead of throwing
- Add 12 regression tests for malformed input handling

Author: erlendellefsen

CLAUDE.md

@@ -39,10 +39,10 @@ dotnet pack JsonApiToolkit/JsonApiToolkit.csproj -p:PackageVersion=VERSION -c Re
 ```bash
 # Format code with CSharpier
 dotnet tool restore
-dotnet csharpier .
+dotnet csharpier format .
 
-# Check formatting
-dotnet csharpier . --check
+# Check formatting (CI uses this)
+dotnet csharpier check .
 ```
 
 ### Documentation
@@ -98,6 +98,7 @@ Enable detailed logging for query processing and troubleshooting:
 
 8. **Helpers** (`Helpers/`)
    - `EfIncludePathHelper`: Utilities for building EF Core Include expressions
+   - `ReflectionMethodCache`: Cached reflection method lookups with defensive checks and clear error messages
 
 ### Key Patterns
 
@@ -109,6 +110,7 @@ Enable detailed logging for query processing and troubleshooting:
 - **JSON column detection**: Collections and complex objects without ID properties are automatically mapped as JSON attributes instead of relationships (useful for EF Core owned entities stored as JSON columns)
 - **Pagination safety**: Invalid page numbers are automatically clamped to valid ranges (page 1 for negative/zero, last page for overflow)
 - **Include whitelisting**: Use `AllowedIncludesAttribute` on controller actions to restrict which relationships can be included, preventing unauthorized data exposure
+- **Graceful error handling**: Malformed query parameters are logged and skipped rather than throwing exceptions
 
 ### Service Registration
 
@@ -140,10 +142,14 @@ JsonApiToolkit provides a comprehensive error handling system with standardized
 
 Tests are organized by component:
 - `Controllers/`: Controller behavior tests
-- `Extensions/`: Query extension tests  
+- `Extensions/`: Query extension tests (filtering, sorting, pagination, includes)
 - `Mapping/`: Entity mapping tests
 - `Models/`: Model validation tests
-- `Parsing/`: Query parser tests
+- `Parsing/`: Query parser tests (including malformed input handling)
+- `Helpers/`: Helper class tests (ReflectionMethodCache, etc.)
+- `Integration/`: Full pipeline integration tests
+- `Filters/`: Exception filter tests
+- `Attributes/`: Attribute behavior tests (AllowedIncludes, etc.)
 
 ## Development Guidelines
 
@@ -166,4 +172,13 @@ Tests are organized by component:
 
 ## Package Publication
 
-The project publishes to GitHub Packages. Use semantic versioning for releases. The CI/CD pipeline automatically builds, tests, and publishes on GitHub releases.
+The project publishes to GitHub Packages. Use semantic versioning for releases. The CI/CD pipeline automatically builds, tests, and publishes on GitHub releases.
+
+## Refactoring Roadmap
+
+The project has a structured refactoring plan in `.claude/`:
+- `REFACTORING_ROADMAP.md` - Phase-by-phase plan with checklists
+- `CODEBASE_ANALYSIS.md` - Analysis of issues found in the codebase
+- `REPO_IMPROVEMENTS.md` - Repository setup tasks (CI/CD, branch protection)
+
+Use `/roadmap` command to get current status and next tasks.

JsonApiToolkit.Tests/Parsing/JsonApiQueryParserTests.cs

@@ -373,4 +373,184 @@ public void Parse_WithOrChainMixedSyntax_CorrectlyIdentifiesFilterTypes()
         var includeFilter = orGroup.Filters.First(f => f.Field == "comments.status");
         Assert.True(includeFilter.IsIncludeFilter);
     }
+
+    [Fact]
+    public void Parse_WithMalformedFilterKey_TooShort_IgnoresFilter()
+    {
+        // Arrange - filter key too short to be valid
+        var httpContext = new DefaultHttpContext();
+        httpContext.Request.Query = new QueryCollection(
+            new Dictionary<string, Microsoft.Extensions.Primitives.StringValues>
+            {
+                ["filter["] = "value", // Too short, missing closing bracket
+                ["filter[name]"] = "valid", // Valid filter to ensure parsing continues
+            }
+        );
+
+        // Act
+        QueryParameters parameters = JsonApiQueryParser.Parse(httpContext.Request);
+
+        // Assert - should only have the valid filter
+        Assert.NotNull(parameters.Filter);
+        Assert.Single(parameters.Filter.Filters);
+        Assert.Equal("name", parameters.Filter.Filters[0].Field);
+    }
+
+    [Fact]
+    public void Parse_WithMalformedFilterKey_MissingClosingBracket_IgnoresFilter()
+    {
+        // Arrange - filter key without closing bracket
+        var httpContext = new DefaultHttpContext();
+        httpContext.Request.Query = new QueryCollection(
+            new Dictionary<string, Microsoft.Extensions.Primitives.StringValues>
+            {
+                ["filter[name"] = "value", // Missing closing bracket
+                ["filter[age]"] = "25", // Valid filter
+            }
+        );
+
+        // Act
+        QueryParameters parameters = JsonApiQueryParser.Parse(httpContext.Request);
+
+        // Assert - should only have the valid filter
+        Assert.NotNull(parameters.Filter);
+        Assert.Single(parameters.Filter.Filters);
+        Assert.Equal("age", parameters.Filter.Filters[0].Field);
+    }
+
+    [Fact]
+    public void Parse_WithMalformedOrGroupIndex_NonNumeric_IgnoresFilter()
+    {
+        // Arrange - OR group with non-numeric index
+        var httpContext = new DefaultHttpContext();
+        httpContext.Request.Query = new QueryCollection(
+            new Dictionary<string, Microsoft.Extensions.Primitives.StringValues>
+            {
+                ["filter[or][abc][name][eq]"] = "value", // Non-numeric index
+                ["filter[or][0][age][eq]"] = "25", // Valid OR filter
+            }
+        );
+
+        // Act
+        QueryParameters parameters = JsonApiQueryParser.Parse(httpContext.Request);
+
+        // Assert - should only have the valid OR filter
+        Assert.NotNull(parameters.Filter);
+        Assert.Single(parameters.Filter.Groups);
+        Assert.Single(parameters.Filter.Groups[0].Filters);
+        Assert.Equal("age", parameters.Filter.Groups[0].Filters[0].Field);
+    }
+
+    [Fact]
+    public void Parse_WithMalformedOrGroupKey_MissingParts_IgnoresFilter()
+    {
+        // Arrange - OR group key with missing parts
+        var httpContext = new DefaultHttpContext();
+        httpContext.Request.Query = new QueryCollection(
+            new Dictionary<string, Microsoft.Extensions.Primitives.StringValues>
+            {
+                ["filter[or][0]"] = "value", // Too short, missing field parts
+                ["filter[or][1][name][eq]"] = "test", // Valid OR filter
+            }
+        );
+
+        // Act
+        QueryParameters parameters = JsonApiQueryParser.Parse(httpContext.Request);
+
+        // Assert - should only have the valid OR filter
+        Assert.NotNull(parameters.Filter);
+        Assert.Single(parameters.Filter.Groups);
+        Assert.Single(parameters.Filter.Groups[0].Filters);
+        Assert.Equal("name", parameters.Filter.Groups[0].Filters[0].Field);
+    }
+
+    [Fact]
+    public void Parse_WithAllMalformedFilters_ReturnsEmptyFilterGroup()
+    {
+        // Arrange - all filters are malformed
+        var httpContext = new DefaultHttpContext();
+        httpContext.Request.Query = new QueryCollection(
+            new Dictionary<string, Microsoft.Extensions.Primitives.StringValues>
+            {
+                ["filter["] = "value",
+                ["filter[name"] = "test",
+                ["filter"] = "invalid",
+            }
+        );
+
+        // Act
+        QueryParameters parameters = JsonApiQueryParser.Parse(httpContext.Request);
+
+        // Assert - should have no filters
+        Assert.Null(parameters.Filter);
+    }
+}
+
+public class JsonApiFilterParserMalformedInputTests
+{
+    [Theory]
+    [InlineData("filter[")] // Too short
+    [InlineData("filter[x")] // Missing closing bracket
+    [InlineData("filter")] // No brackets at all
+    [InlineData("filte[x]")] // Wrong prefix
+    [InlineData("")] // Empty string
+    public void ParseComplexFilter_WithMalformedKey_IgnoresFilter(string key)
+    {
+        // Arrange
+        var group = new FilterGroup();
+
+        // Act
+        JsonApiFilterParser.ParseComplexFilter(key, "value", group);
+
+        // Assert - should not add any filter
+        Assert.Empty(group.Filters);
+    }
+
+    [Fact]
+    public void ParseComplexFilter_WithValidKey_AddsFilter()
+    {
+        // Arrange
+        var group = new FilterGroup();
+
+        // Act
+        JsonApiFilterParser.ParseComplexFilter("filter[name][eq]", "value", group);
+
+        // Assert
+        Assert.Single(group.Filters);
+        Assert.Equal("name", group.Filters[0].Field);
+    }
+
+    [Fact]
+    public void ParseLogicalGroup_WithMalformedIndices_SkipsInvalidFilters()
+    {
+        // Arrange
+        var httpContext = new DefaultHttpContext();
+        httpContext.Request.Query = new QueryCollection(
+            new Dictionary<string, Microsoft.Extensions.Primitives.StringValues>
+            {
+                ["filter[or][invalid][name][eq]"] = "bad", // Non-numeric index
+                ["filter[or][-1][name][eq]"] = "negative", // Negative index (valid int, but unusual)
+                ["filter[or][0][status][eq]"] = "active", // Valid
+            }
+        );
+
+        var parentGroup = new FilterGroup();
+
+        // Act
+        JsonApiFilterParser.ParseLogicalGroup(
+            httpContext.Request,
+            "or",
+            LogicalOperator.Or,
+            parentGroup
+        );
+
+        // Assert - should have filters from valid indices only
+        Assert.Single(parentGroup.Groups);
+        var orGroup = parentGroup.Groups[0];
+
+        // -1 is a valid integer, so it should be parsed (just unusual)
+        // "invalid" should be skipped
+        Assert.True(orGroup.Filters.Count >= 1); // At least the valid one
+        Assert.Contains(orGroup.Filters, f => f.Field == "status");
+    }
 }

JsonApiToolkit/Attributes/AllowedIncludesAttribute.cs

@@ -57,7 +57,11 @@ public override void OnActionExecuting(ActionExecutingContext context)
         }
 
         var request = context.HttpContext.Request;
-        var queryParams = JsonApiQueryParser.Parse(request);
+        var logger =
+            context.HttpContext.RequestServices.GetService(
+                typeof(ILogger<AllowedIncludesAttribute>)
+            ) as ILogger<AllowedIncludesAttribute>;
+        var queryParams = JsonApiQueryParser.Parse(request, logger);
 
         if (queryParams.Include == null || queryParams.Include.Count == 0)
         {
@@ -72,7 +76,8 @@ public override void OnActionExecuting(ActionExecutingContext context)
                 queryParams.Include,
                 queryParams.Include,
                 _allowedIncludes,
-                context
+                context,
+                logger
             );
             return;
         }
@@ -88,25 +93,22 @@ public override void OnActionExecuting(ActionExecutingContext context)
                 queryParams.Include,
                 validationResult.ForbiddenIncludes,
                 _allowedIncludes,
-                context
+                context,
+                logger
             );
         }
 
         base.OnActionExecuting(context);
     }
 
-    private void ThrowForbiddenException(
+    private static void ThrowForbiddenException(
         List<string> requestedIncludes,
         List<string> forbiddenIncludes,
         string[] allowedIncludes,
-        ActionExecutingContext context
+        ActionExecutingContext context,
+        ILogger? logger
     )
     {
-        var logger =
-            context.HttpContext.RequestServices.GetService(
-                typeof(ILogger<AllowedIncludesAttribute>)
-            ) as ILogger<AllowedIncludesAttribute>;
-
         if (logger != null && forbiddenIncludes.Count > 0)
         {
             logger.LogWarning(