Bug: StatusCode 400 instead of 404 on valid version with parameter constraint violation

[codepunkt]

The following example is reduced to its bare minimum. I have enabled URL-Path versioning for ASP.Net Web API according to the documentation in the Wiki:

this.config.MapHttpAttributeRoutes(new DefaultInlineConstraintResolver()
{
    ConstraintMap =
    {
        ["apiVersion"] = typeof(ApiVersionRouteConstraint)
    }
});
this.config.AddApiVersioning();

Controller is defined like this:

[ApiVersion("1.0")]
[RoutePrefix("api/v{version:apiVersion}/resource")]
public class ResourceController : ApiController
{
    [HttpGet]
    [Route("{id:int}")]
    public async Task<IHttpActionResult> Get([FromUri] int id)
    {
        return Ok();
    }

    [HttpDelete]
    [Route("{id}")]
    public async Task<IHttpActionResult> Delete(int id)
    {
        return Ok();
    }
}

What happens is this:

• GET-request to api/v1/resource/123 → server responds with 200 OK.
• GET-request to api/v2/resource/123 → server responds with 400 Bad Request, including the standard error response with error code UnsupportedApiVersion as described in the documentation wiki
• GET-request to api/v1/resource/asdf → server responds with 400 Bad Request, including UnsupportedApiVersion just as the previous example.

This seems wrong. The given URL would never match a route when GET-requested.

When i add the :int constraint to the DELETE route or remove the route altogether, the GET-request to api/v1/resource/asdf returns 404 Not Found with an empty body as expected.

[commonsensesoftware]

Is that your full WebApiConfig.cs? If not, can you please provide it?

[codepunkt]

@commonsensesoftware sure. here it is:

internal class Startup
{
    private List<RequestHeaderMapping> requestHeaderMappings = new List<RequestHeaderMapping> {
        new RequestHeaderMapping(
            "Accept",
            "text/html",
            StringComparison.InvariantCultureIgnoreCase,
            true,
            "application/json"
        )
    };

    public void Configuration(IAppBuilder app)
    {
        HttpConfiguration config = new HttpConfiguration();
        config.EnableCors();
        config.Formatters.JsonFormatter.SerializerSettings.ContractResolver
                = new CamelCasePropertyNamesContractResolver();

        config.MapHttpAttributeRoutes(new DefaultInlineConstraintResolver()
        {
            ConstraintMap =
            {
                ["apiVersion"] = typeof(ApiVersionRouteConstraint)
            }
        });
        config.AddApiVersioning();

        foreach (RequestHeaderMapping mapping in this.requestHeaderMappings)
        {
            config.Formatters.JsonFormatter.MediaTypeMappings.Add(mapping);
        }

        app.UseWebApi(config);
        config.EnsureInitialized();
    }
}

You can remove the request header mappings, CORS support, serializer settings and EnsureInitialized and the error persists.

[commonsensesoftware]

Great! Thanks. I just wanted to get the whole picture before I replied.

The behavior you're seeing is expected, but let me explain why. Part of it is how routing works in Web API and part of it has to do with how API versioning reports unmatched routes. First, the direct routes (aka Attribute Routing) produced via these attributes generate the following routes:

• api/v{version:apiVersion}/resource/{id:int}
• api/v{version:apiVersion}/resource/{id}

The HTTP method is treated like a route constraint and is not part of the path.

API versioning does not get involved with routing - at all. It only steps in during the route-to-controller and route-to-action selection process. The default behavior in Web API would treat duplicate routes as a developer mistake (e.g. 500). API versioning extends the default behavior by one step and tries to disambiguate routes by API version. When a match is made, things flow through as normal. When things don't match, that's where the behavior might be different from what you'd expect.

Here's how the selector decides how it will respond:

• If no routes match, then return 404 (Not Found) as normal
• If at least one route could match, then return 400 (Bad Request) and indicate the API version is not supported

The main reason for this behavior is to ease the transition for clients between API versions. If you sunset part of your API, returning 400 with a clear error message lets them know that it's no longer supported. While it's possible to return 404, it's a little misleading because for that particular API version it will never succeed (404 is not permanent) nor will it give you a more informative error message.

In your scenario, you have two, overlapping routes. In the current release, you would also see 400 when a particular HTTP method is not supported by an API version. There was some feedback to retain the 405 response and I agree. In the 1.1.0-* release, you'll receive 405 and the standard error response instead of 400.

Hopefully, that clears up the explanation. If you're still not satisfied with that answer, there is a way to change the behavior which is not documented on the wiki. You can override the way error responses are provided. In the current release, it looks like:

configuration.AddApiVersioning(
 options =>
 {
    var defaultBadRequest = options.CreateBadRequest;
    options.CreateBadRequest = ( request, code, message, messageDetail ) =>
    {
       var response = defaultBadRequest( request, code, message, messageDetail );

       if ( code == "UnsupportedApiVersion" )
       {
          response.StatusCode = HttpStatusCode.NotFound;
       }

       return response;
    }
 }
)

Be aware that this capability will be available in the 1.1.0 release, but the way that it's wired up will not be the same. The simple delegate property CreateBadRequestDelegate CreateBadRequest will be supplanted by IErrorResponseProvider ErrorResponses. You can map your alternate behaviors using the standard, documented error responses. All API versioning-related responses always use this mechanism.

[codepunkt]

Thank you for the very thorough and helpful explanation!
You have explained why the wrong error code is sent and which parts of the routing stack and the API versioning handle the decision-making that led to me creating this issue - which is very appreciated.

Nevertheless you did not answer the question i would deem most important: why does the http verb, which is an integral part of routing in every technology i've developed APIs with so far, seem to be an aftersight here?

Given my route definition, a GET request to /api/v1/resource/asdf could never match. Reporting that there might be another API version that could be supported for this request is completely misleading. No GET request to /api/v{anyVersion}/resource/asdf will ever return anything meaningful.

Couldn't the check if a route could match be extended to include the http verb?

The HTTP method is treated like a route constraint and is not part of the path.
I'm sure this can be and think this should be changed. It should not be part of the path, but handled like it was in route matching.

I understand the part about returning 400s when sunsetted parts of an API are requested with a higher version number - shouldn't it be possible to automatically detect if a version that exists on the API is requested for an endpoint that does no longer exist in this version?

[commonsensesoftware]

I can't argue about some of the idiosyncrasies of routing, especially in Web API. Things are more sane in ASP.NET Core.

In this scenario though, the URL is the route and a HTTP method is not part of that. Remember that the URL alone is the resource identifier as imposed by the Uniform Interface REST constraint. According to RFC 2616 §5.1.1, the server should return 405 if the requested method for a known resource (e.g. URL) is not supported. You could argue that 501 be returned, which is also called out in the spec. §10.4.6 calls out that the Allow header should return the supported HTTP methods in these cases.

Not to just cite specs to you, but I think the behavior you are seeing is related to Issue #65. Essentially, things execute correctly, but the status code should be 405 and not 400 as you are observing. This behavior should mimic what Web API does by default. I expect that you'll get a 405 for GET or DELETE when one of them does not exist. That fix is in the 1.1.0-beta package. In the current release, you can only change this behavior using the previously illustrated method. The one caveat is that there is no additional information that you can use to distinguish between scenarios where you might want to return 404 versus 405. This is something I'm planning on addressing before the official 1.1.0 release.

Disambiguating the difference between when a 404 should be returned because route constraints were not met versus 400 because it's not supported for a particular API version is quite tricky. Here's the logic:

• 0 matches and 0 candidates = 404
• 1 match and 1+ candidates = [result of action]
• 1+ matches and 1+ candidates = 500 (developer mistake)
• 0 matches and 1+ candidates
  • if at least one action, for any version matches the requested verb = 405
  • unsupported api version = 400

The router matches routes to controller types (in Web API; only actions in Core). I don't see a clear way to disambiguate candidates to definitively provide a 404. Consider:

• V1 = values/{id}
• V2 = values/{id:int}

Regardless of API versioning method used, both routes look like values/{id} to the router. API versioning will see both candidates. A request for values/abc succeeds in V1, but would fail with 400 in V2. The challenge is how to distinguish between the resource you asked for doesn't exist (404) and the thing you asked me to do is not supported (400).

In the older, internal Microsoft REST guidelines, it specifically stated things need to behave by responding with 400. Right or wrong, I was trying to avoid inventing a specification. The current guidelines are not so specific. I'm pretty sure I know what Fielding would say ;).

From what I've learned from trying support this for the past few years (internal to public), there is no definitive best answer for a response that will satisfy everyone. In general, things have fallen on the 400 side because it's more informative for client diagnostics. I realized that some service authors may not agree with the responses - either by status code or payload. This is why the hook points exist in the first place.

I'm not sure that the default configuration and experience can be improved, but I'm open to ideas. Now that API versioning by media type is supported, someone recently asked about returning 406 instead of 400 for similar reasons. What I'm taking from this feedback is that there needs to be:

• A better abstraction/API controlling how responses are generated for failed requests related to API versioning
• More or better default behaviors for particular styles of API versioning (ex: if you use media types, then you almost certainly want 406 responses for unsupported versions)

This is certainly an area of improvement I'm looking to include in 1.1.0. There some changes in "beta1", but I think I need to rethink this some more. You are not the first to bring up this discussion. I'm still listening. :)

[commonsensesoftware]

Per you example ... while the request looks like api/v1/resource/asdf, what the router sees and produces for candidates is api/v{version}/resource/{id}. The HTTP method is not part of this process; however, in Web API there is a second selection process (IHttpActionSelector). If the HTTP method doesn't match, this is where a 405 is supposed to be returned. I'm not sure that's what you were hoping for, but it's still more informative than 400.

[codepunkt]

1+ matches and 1+ candidates = 500 (developer mistake)

Mistake of the developer using the API should never result in 500, which is reserved for server errors only.

if at least one action, for any version matches the requested verb = 405

If an action for another version than the given one matches the requested verb - how can this be "Method not supported"? After all, the verb is supported - just on another version.

unsupported api version = 400

Depending on where the api version is read from, that might be acceptable. If it is read from the URL, i strongly suggest not sending a 400 as by definition the URL is part of the resource identifier and therefore the version is aswell - so you have not found the resource and should send 404.

Other than that, i'm with you.

[commonsensesoftware]

Sorry, I should have clarified. When I said:

... (developer mistake)

I meant service author mistake. It should not be possible to have multiple versions, for multiple routes match a single request. In this case, things are ambiguous. This is the same behavior Web API uses out of the box. One common way this can happen is if you just input the wrong version on your controller (classic copy & paste error). It can also happen if one of the services is version-neutral. You cannot mix versioned and version-neutral implementations of a service; for fairly obvious reasons. Version-neutral means all versions and not no version, which has confused some people.

The trick with 405 is that it's unclear in the selection process which method is not supported. When this scenario happens, there are zero matches, but a set of routes, methods, and versions. Since there is no correlation between them, the best that can be done (for now) is to produce a unioned set of all methods, regardless of version. If that set doesn't contain the requested method, then it's impossible that any version ever supported it, which makes it reliable to return 405. For example, the client sends DELETE, but v1 supported GET and v2 supported GET and POST. There's no scenario where DELETE could not result in 405.

When that's not the case, however, I haven't been able to figure out how to provide a better response - yet. Maybe it helps to clarify the filtering process:

• Candidates = match possible candidates
• Filtered Candidates = constraints applied to candidates
• Best Candidate = filtered candidate matching requested API version

That's an oversimplification, but it's easy to understand. The important factor here is that the candidates are filtered before the resolution by API version can be begin. In your case where you have the route constraint, the constraint causes there to be no remaining candidates to match with the requested API version. Once that happens, the selector is left in a spot where it either has to return 400 or 404. In a much older, internal version of the libraries, it used to always be 404. There are some scenarios where you genuinely want the 400 (unsupported API) and other times when you want 404 (ex: non-existent route /foo/bar).

It's always been my goal to avoid changing or otherwise making service authors re-learn anything they know about how the routing infrastructure works - that includes route constraints. I think you've stumbled across a limitation. I would certainly love to solve this. Using your own custom route constraints are a key and powerful part of the platform. You should be able to use them and have them behave the way you expect. Ultimately, I think that's the root cause to this issue. You could get the same behavior you want by validating with an action filter or directly in the controller; however, as I said, that would require you to do something different and that's not a place I want to leave service authors. You should be able to light things up the way you want.

Great discussion. Thanks for pushing the issue forward and tolerating my tome-like responses. I'll take a look how feasible it is to disambiguate whether initial route matching failed because of route constraints. The failure of route constraints to match will always produce a 404. If that's possible, I think this issue can be addressed. Should that be the case, I'll spin up a new issue describing the exact problem more succinctly and queue up work to knock it out. I'll let you know how it goes. Any other thoughts or you have are welcome. Thanks.

[codepunkt]

@commonsensesoftware
You've done a great job on answering my questions and reacting to my criticism. I'm amazed by how much effort you go through to explain the underlying mechanisms to me, and to others who might read this conversation. Thanks a lot! You rock! :-)

[commonsensesoftware]

I just wanted to provide a quick follow up on the discussion. Unfortunately, the built-in ASP.NET routing behavior doesn't quite work as one would expect. Inline with our thread, consider the following:

[ApiVersion( "1.0" )]
[Route( "api/values" )]
public class ValuesController : Controller
{
  [HttpGet( "{id}" )]
  public IActionResult Get( string id ) => Ok( id );
}

[ApiVersion( "2.0" )]
[Route( "api/values" )]
public class Values2Controller : Controller
{
  [HttpGet( "{id:int}" )]
  public IActionResult Get( int id ) => Ok( id );
}

Since the IActionSelector interface is supposed to receive all candidates and select the best one, you'd expect that the request GET http://localhost/api/values/123 would produce two candidates; however, I found that it only produces one. This has to do with the results produced by the IActionSelectorDecisionTreeProvider, which is currently not extended or customized by API versioning.

I'm not sure if this would be considered a bug (due to the decision tree not reporting all candidates) or the expected behavior (which seems wrong to me). This part of the stack is way down in the low-level infrastructure that isn't really documented. I'm trying to get some F2F time with the ASP.NET team to see if I can get some input that can help drive this forward. I'd rather not have to implement a custom decision tree.

For now, I'm going to leave things as is. It's not ideal, but I don't think it makes sense to change things until I have a better understanding of what must be done. I'm imminently about to publish Beta2, which will contain a small refactoring to the IErrorResponseProvider. You should be able to use the combination of HTTP status code and error code to provide the appropriate error response (ex: 404 instead of 400). The standard error response codes are now exposed as constants in the ErrorCodes type. This will hopefully tide you over until the right fix gets in. I suspect that it might take a while. I'll keep this issue open until then. When I have a full triage, I'll likely open a new issue to that articulates the problem more succinctly.

Thanks again for the input. It helps make things better for everyone. ;)