NullReferenceException in TryMatchModelVersion with OData + OpenAPI and only the default version

[icnocop]

trafficstars

Hi.

When I try to get swagger.json for an OData API which exposes only the default version, the following exception occurs:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Asp.Versioning.ApiExplorer.ODataApiDescriptionProvider.TryMatchModelVersion(ApiDescription description, IReadOnlyList`1 items, IODataRoutingMetadata& metadata) in E:\github\icnocop\aspnet-api-versioning-preview3-fixes\src\AspNetCore\OData\src\Asp.Versioning.OData.ApiExplorer\ApiExplorer\ODataApiDescriptionProvider.cs:line 198
   at Asp.Versioning.ApiExplorer.ODataApiDescriptionProvider.OnProvidersExecuted(ApiDescriptionProviderContext context) in E:\github\icnocop\aspnet-api-versioning-preview3-fixes\src\AspNetCore\OData\src\Asp.Versioning.OData.ApiExplorer\ApiExplorer\ODataApiDescriptionProvider.cs:line 120
   at Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.GetCollection(ActionDescriptorCollection actionDescriptors)
   at Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.get_ApiDescriptionGroups()
   at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GetSwagger(String documentName, String host, String basePath)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

I will create a PR with a sample project that can reproduce the issue.

I'm using the latest code from the dev/css/preview3-fixes branch with an ASP.NET Core web application targeting .NET Core 3.1.

It seems to be different than https://github.com/dotnet/aspnet-api-versioning/issues/837

Thank you.

[commonsensesoftware]

Interesting. I'll have a look. The strange thing is the current source doesn't show anything on line 198. The repro will definitely help. Thanks

[icnocop]

This is in the dev/css/preview3-fixes branch so this is line 198: https://github.com/dotnet/aspnet-api-versioning/blob/f5cd8e50f569fb0f641cca0bd2dd673ae2b0c6fa/src/AspNetCore/OData/src/Asp.Versioning.OData.ApiExplorer/ApiExplorer/ODataApiDescriptionProvider.cs#L198

[commonsensesoftware]

Doh!

Hmm... looks like apiVersion is somehow coming up null. That shouldn't happen. Defending against null is easy, but I suspect there is a little more to the issue.

[icnocop]

I'm thinking my example code is missing a call to what was previously MapVersionedODataRoute, but I'm not sure that method is available for a web application targeting .net core 3.1 in the latest code for an OData API.

[commonsensesoftware]

I found the root cause. It's here:

https://github.com/dotnet/aspnet-api-versioning/blob/9f7ecd8f1ac25eb3b6a59701f4567f703060f076/src/AspNetCore/OData/src/Asp.Versioning.OData/OData/ODataMultiModelApplicationModelProvider.cs#L124

This is an over-optimization. OData adds some additional endpoints based on convention, but doesn't copy the existing metadata from any other endpoints. This is problematic when there are multiple EDMs, but also if API Versioning never got a chance to visit the endpoint. As a result, you can end up with a versioned endpoint that doesn't have the API Versioning metadata attached.

The first clue is skipping over the null check and watching it filter out endpoints successfully. The following should have been listed:

• api/People
• api/People/$count

But the $count endpoint is dynamically added by convention without any metadata. Handling the null keeps it from blowing up, but it's still wrong. The endpoints need to be fixed up regardless of whether there is more than one EDM.

You might also notice 3 endpoints in your repro, but that's because there is a mismatch in the route templates (and a different problem). There should only be 2.

The fix will go into the next release. Thanks for putting together the repo. It helped track down the issue.

[commonsensesoftware]

6.0 has been officially released and contains this fix. Thanks for taking the time to report it and help narrow down the culprit.