Skip to content

NullReferenceException in ApiVersionParameterDescriptionContext.RemoveAllParametersExcept #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dbrink opened this issue Feb 9, 2018 · 5 comments · Fixed by #252
Closed
Assignees

Comments

@dbrink
Copy link

dbrink commented Feb 9, 2018

When using the ApiExplorer in AspNet Core a null reference exception is thrown if the API defines a parameter as part of a route, but does not have matching parameter on the controller method.

[HttpGet("foo/{unmatched}"]
public string GetFoo()
{
   return "foo";
}

With no matching C# parameter on GetFoo() the ApiParameterDescription's ModelMetadata is null. A null coalescing operator should fix the problem.

void RemoveAllParametersExcept( ApiParameterDescription parameter )
{
// code omitted
   if ( otherParameter?.ModelMetadata.DataTypeName == nameof( ApiVersion ) )
   {
      collection.Remove( otherParameter );
   }
// code omitted
}

@commonsensesoftware
Copy link
Collaborator

Yep ... except you want to coalesce on the model metadata like so otherParameter.ModelMetadata?.DataTypeName == nameof( ApiVersion ). It's not the parameter that's null, it's the model metadata.

I had new feature I was about to roll out for the API Explorer anyway. I should have this patched and published tonight or tomorrow. Thanks.

@dbrink
Copy link
Author

dbrink commented Feb 9, 2018

Ah, yes. Thanks for the catch and incorporating the fix.

@wburgers
Copy link

wburgers commented Mar 19, 2018

Is it possible to backport this bugfix to version 1.0.2 of Microsoft.AspNetCore.Mvc.Versioning.ApiExplorer?

@commonsensesoftware
Copy link
Collaborator

It's possible, but kind of a pain. Is there a particular reason to not move forward? As an army of one, it's hard to maintain multiple different version branches on 3 difference platforms. This would require creating a divergent branch with the fix or publishing a package version with the fix which doesn't correspond back to source. Neither is very amicable scenario.

@wburgers
Copy link

No, there is no particular reason.
We just lag behind in our asp.net core maintenance 😇.
I'll see if I can upgrade our webapp.
Thanks for getting back to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants