-
Notifications
You must be signed in to change notification settings - Fork 704
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
Unhandled exception when you call an HTTP POST action with GET #52
Comments
This looks like a possible false positive. In the new POST action, it is returning the location header that is generated by using the route template with the key notAGet and the values To test the theory, I added and modified your example as: [HttpPost("notAGet")]
public IActionResult NotAGet() => CreatedAtRoute("GetMessageById", new { id = 43 }, null ); The request looks like: POST /api/v1/helloworld/notAGet HTTP/1.1
Host: localhost The operation succeeds with the response: HTTP/1.1 201 Created
api-supported-versions: 1.0
Location: http://localhost/api/v1/HelloWorld/43 In addition, the 500 response that I received clearly indicated that one or more of the route values were not present in the template. Maybe that information is only disclosed during debugging. Let me know if this is not the behavior you are expecting. |
@commonsensesoftware - sorry, my example is probably a bit confusing, so I'll strip it down to the simplest case. Take this controller [ApiVersion("1.0")] if you issue a GET request as follows: You get an unhandled ArgumentNullException from the ApiVersioning middleware. It throws at line 26 of Microsoft.AspNetCore.Mvc.Versioning.ActionSelectionContext.cs. This will end up being an Http 500 response. I think you should get an Http 404 in this instance because there is no matching route for the GET request. At least that is what happens if you do the same thing to a controller in a project without any Api versioning e.g.
} GET /api/helloworld/notAGet HTTP/1.1 Gives a 404... Hope that makes sense... |
I see. Yes, this is a bug. The original implementation of the ActionSelector class returns |
No problem. Thanks for fixing! |
…actions are matched and return HTTP 404 instead. Fixes #52.
A bit obscure I know, but I came across this bug when I was doing daft things to my Api to see if it reacted OK (users do daft things occasionally!)
Its easy to reproduce with the Asp.Net Core samples HelloWorldController. Just add an HttpPost action that does anything you like - I have added one called "NotAGet"
`[ApiVersion("1.0")]
[Route("api/v{version:apiVersion}/[controller]")]
public class HelloWorldController : Controller
{
// GET api/v{version}/helloworld
[HttpGet]
public IActionResult Get() => Ok(new { Controller = GetType().Name, Version = HttpContext.GetRequestedApiVersion().ToString() });
Run the project and paste the following into a browser address bar:
http:///api/v1/helloworld/notAGet
You will get an unhandled ArgumentNullException in the application, ultimately resulting in an HTTP 500 being returned to the browser.
Obviously this is a pretty low priority as it only occurs when someone does something silly, but ideally it still shouldn't throw an unhandled exception...
The text was updated successfully, but these errors were encountered: