AspNetCoreOData icon indicating copy to clipboard operation
AspNetCoreOData copied to clipboard

Adding condition :min(1) to route parameter breaks OData routing

Open AndrewZenith opened this issue 1 year ago • 3 comments

Assemblies affected .NET 9.0 Microsoft.AspNetCore.OData 9.1.0 Microsoft.OData.Core 8.21

Describe the bug [ApiExplorerSettings(GroupName = "OData")] [ODataRouteComponent("global")] [Route("/odata/global")] public class SystemDataTypeController(GlobalDbContext _db) : ODataController { [EnableQuery] [HttpGet("SystemDataType")] public IQueryable<SystemDataType> Get() { return _db.SystemDataTypes.AsNoTracking(); }

[EnableQuery]
[HttpGet("SystemDataType/{id:int}")] // <--------------------------------- This works, but id:int:min(1) breaks the routing
[HttpGet("SystemDataType({id:int:min(1)})")] <--------------------------- This works, with or without condition
public SingleResult<SystemDataType> GetById([FromRoute] uint id)
{
    return SingleResult.Create(_db.SystemDataTypes.Where(x => x.Id == id).AsNoTracking());
}

AndrewZenith avatar Nov 15 '24 05:11 AndrewZenith

Can you elaborate what exactly do you mean by "breaks OData routing"? Does it throw an exception when trying to access the endpoint? What is the status code?

Can you share the output of the $odata route debug endpoint with and without the constraint added?

julealgon avatar Nov 15 '24 13:11 julealgon

The endpoint is pushed into the Non-OData Endpoint Mappings in the :min(1) case

The following is seen in the console:

'odata/global/SystemDataType/{id:int:min(1)}' on the action 'GetById' in controller 'SystemDataType' is not a valid OData path template. Bad Request - Error in query syntax.

AndrewZenith avatar Nov 15 '24 19:11 AndrewZenith

Ok, I can repro too. Definitely a bug, apparently caused by the presence of the parenthesis in the constraint, considering something like this still works fine: {id:int:required}.

Even with a single constraint it also triggers the problem, so {id:min(1)} is enough.

This feels to me like an opportunity to unify code between OData and ASPNET Core itself. Apparently, OData is not properly parsing Route Constraints here, meaning it is not using strong-enough abstractions to read the route template string.

Error log is coming from here, on the attribute routing convention logic: https://github.com/OData/AspNetCoreOData/blob/9a0c7dbbfbc141f21f9bad50504f7f3978742ead/src/Microsoft.AspNetCore.OData/Routing/Conventions/AttributeRoutingConvention.cs#L159-L205

But the actual exception is in Microsoft.OData.Core in ODataUriParser: https://github.com/OData/odata.net/blob/be48a19940bcded91bae4db83b79ec9a50312382/src/Microsoft.OData.Core/UriParser/ODataUriParser.cs#L287-L291

julealgon avatar Nov 15 '24 20:11 julealgon