aspnet-api-versioning icon indicating copy to clipboard operation
aspnet-api-versioning copied to clipboard

UnsupportedApiVersion error when giving some different content type

Open PranjaliSoni opened this issue 3 years ago • 15 comments

We are using 4.1.1 version of APiVersioning and 3.1 version of Asp.NetCore I have a controller with an action which accepts only Content-Type "application/merge-patch+json"

[HttpPatch("{id}")]
[Consumes("application/merge-patch+json")]
public async Task<Object> CreateObjectAsync(
..
)
{
..
..
}

So while quering for the given api with content type "application/merge-patch+json".. It works fine but quering for some other valid content type for example "application/json" gives 400 bad request and following output:

{
    "error": {
        "code": "UnsupportedApiVersion",
        "message": "The HTTP resource that matches the request URI 'https://bb-df-wus2-1.farmbeats-dogfood.azure.net/farmers/pra88888m?api-version=2021-03-31-preview' does not support the API version '2021-03-31-preview'."
    }
}

and on using APiVersioning version as "5.0.0" I got the same response with different status code. Can you please help me with this?

PranjaliSoni avatar May 04 '21 14:05 PranjaliSoni

Are you able to show at least the skeleton of the rest of your controller and perhaps the configuration setup? Which route/action is giving you trouble? You seem to indicate that the one you've shown works. If no media types match, I would have expected HTTP 406 (Not Acceptable). A response will contain the same error, but may be HTTP 400 or 405 depending on whether the HTTP method is supported by any version.

Repros always the best, but with a little more information I can probably figure out what's happening here.

commonsensesoftware avatar May 04 '21 17:05 commonsensesoftware

@commonsensesoftware

        [HttpPatch("{Id}")]
        [Consumes(CommonConstants.ApplicationMergePatchContentType)]
        [ProducesResponseType(typeof(Models.Entities.Object), StatusCodes.Status201Created)]
        [ProducesResponseType(typeof(Models.Entities.Object), StatusCodes.Status200OK)]
        [ProducesDefaultResponseType(typeof(ErrorResponse))]
        public async Task<ActionResult<Models.Entities.Object>> CreateOrUpdateObjectAsync(
            [Required][ValidateId(CommonConstants.MaxIdLength, MinIdLength = CommonConstants.MinIdLength)] string Id,
            [FromBody] Models.Entities.Object farmerUpdate,
            CancellationToken cancellationToken)
        {
        }

This is the structure of the controller . One of the headers "Content-Type" which is set in this attribute on the controller [Consumes(CommonConstants.ApplicationMergePatchContentType)] and is made to accept value application/merge-patch+json ...but when we give some other value in content type header .. it gives error api version not supported (400 bad request).. let me know if you need any other information?

We want the api versioning library to not throw unsupported error so that we can error out properly.

PranjaliSoni avatar May 15 '21 14:05 PranjaliSoni

This provides a little more about the action, but it doesn't provide any information about the controller or application setup. I still have a number of questions.

  1. Are you versioning by media type? a. The URL in error message suggests you're versioning by query string; it's possible you're using both methods b. Your comments suggest you're versioning by media type, but perhaps you're not
  2. I think you're saying that PATCH with application/merge-patch+json works, but not with application/json. a. It appears you are trying implement RFC 7386 b. You've only mapped application/merge-patch+json with the ConsumesAttribute, so I would expect application/json to fail c. I would expect HTTP 415 (Unsupported Media Type) if application/json is in the Content-Type header for this action d. Like media types, the API version is a constraint used to filter down candidates. It's possible, things are short-circuit before other failures have a chance to execute. This could mean no candidates were found before HTTP 406 or 415 was evaluated.

"We want the api versioning library to not throw unsupported error so that we can error out properly."

I'm not 100% sure what that means. API Versioning has very few errors it throws. It will, however, return client errors - without exception - for invalid requests. There are ways to change the error response; for example, a different status code, message, or payload. It's unclear if that's what you mean by "...error out properly." I want to make sure I understand your scenario before I send you down that path.

commonsensesoftware avatar May 18 '21 15:05 commonsensesoftware

Okay .. @commonsensesoftware To simplify.. Let me give you the exact scenario ..I tried the same on one of your examples .. for the sample used for api-versioning (https://github.com/microsoft/aspnet-api-versioning/tree/master/samples/aspnetcore)..

I appended Consumes attribute over one of the post APis in OrdersController

/// <summary>
        /// Gets a single order.
        /// </summary>
        /// <param name="id">The requested order identifier.</param>
        /// <returns>The requested order.</returns>
        /// <response code="200">The order was successfully retrieved.</response>
        /// <response code="404">The order does not exist.</response>
        [HttpGet( "{​​​​​id:int}​​​​​" )]
        [Produces( "application/json" )]
        [Consumes( "application/merge-patch+json" )]
        [ProducesResponseType( typeof( Order ), 200 )]
        [ProducesResponseType( 404 )]
        public IActionResult Get( int id ) => Ok( new Order() {​​​​​ Id = id, Customer = "John Doe" }​​​​​ );

Now some of the answers to your questions:

1.There is versioning only on string and not on any mediaType/ContentType. 2. This should accept only "application/merge-patch+json" Content type and error out with HTTP 415 (Unsupported Media Type)with application/json is in the Content-Type header for this action.

Outputs:

  1. On giving content type "application/merge-patch+json" and version "1.0" it works fine and gives the output { "id": 42, "createdDate": "2021-03-05T00:00:00+00:00", "customer": "kkk" } with status code 201 Created

2. Issue : But on giving content type "application/json" and same version "1.0" { "error": { "code": "UnsupportedApiVersion", "message": "The HTTP resource that matches the request URI 'http://localhost:5000/api/Orders' with API version '1.0' does not support HTTP method 'POST'.", "innerError": null } } with Status Code 405 Method Not Allowed.

It should not have errored out by api-versioning library as the version is correct and it should others contents types. I hope this examples gives you more context about controller and application-setup.

PranjaliSoni avatar May 20 '21 11:05 PranjaliSoni

This makes a lot more sense. Out of curiosity, does this work without versioning? I would not expect it to. [Produces("application/json")] means that the action allows Accept: application/json and [Consumes("application/merge-patch+json")] allows Content-Type: application/merge-patch+json. When you send:

PATCH api/Orders?api-version=1.0 HTTP/2
Host: localhost:5000
Content-Type: application/json
Content-Length: 142

{
 "id": 42,
 "createdDate": "2021-03-05T00:00:00+00:00",
 "customer": "kkk"
}

I agree, the server should respond with HTTP 415 (Media Type Unsupported). Without versioning does it?

This is an interesting edge case. I don't think I've seen it before, but I suspect I know why it's happening.

Propagating 406 and 415 is hard for ASP.NET Core (and presumably all web frameworks). This decision happens not at the controller level, but at during serialization/deserialization. I as recall 406 is harder because first an action has to be selected, executed, and the decision is made trying to resolve a corresponding OutputFormatter. If the resolution fails, the client receives 406. You should think 415 would be selected early during the request, but it seems it isn't matched until the InputFormatter is selected during model binding.

So what does that have to do with versioning? The versioning endpoint matcher policy invalidates any valid candidate by API version. This should be the last step and constraint. Unfortunately, it seems that media type does not invalidate a candidate; at least, not before API versioning sees it. That's why you don't receive 406 or 415. API versioning returns 400 when no candidate for a given route doesn't match. It returns 405 if there is some candidate, in at least one version, that could match. This is often the case if a HTTP method is added or removed between versions. I'm not entirely sure why/where the wires are crossed. There is a similar when versioning by media type, which can be addressed by changing the IErrorResponseProvider.

Aside: I realize it's just my opinion, but I've read RFC 7396 soup to nuts, which is the final specification. I don't see anything additional value that the media type application/patch-merge+json provides. The same behavior is achieved with PATCH (or the legacy MERGE) with application/json. Protocols such as OData have been doing so successfully for years. Complete replacement of the document should be using PUT or POST, but could be also be PATCH. It's more than a reasonable assumption that any request to PATCH will be a partial document. A more practical extension is using RFC 7240, which allows the client to prefer whether they want content back from the server. A client that creates or updates a resource often, but not always, already knows what the entire resource looks like. Echoing it back is, therefore, not generally useful.

I'm sure you have a valid reason why you want this behavior. I won't try to talk you out of it or deflect from the issue at hand. However, I will point out that you can unblock yourself by simply using PATCH + application/json. It will take some time to investigate this further to understand the behavior and possible solutions.

commonsensesoftware avatar May 20 '21 17:05 commonsensesoftware

@commonsensesoftware I verified its working without versioning. It gives proper error 415 UnSupportedMediaType. You can verify it once on some sample controllers.

Yes, We want "application/merge-patch+json" according to some guidelines. I'll reconfirm on Patch + application/json thing. But We want to support "application/merge-patch+json" and we might need any other content type also for that matter for any scenario. We shall try to fix this to get unblocked early.

PranjaliSoni avatar May 20 '21 19:05 PranjaliSoni

Thanks for confirming. I'll see what I can find.

commonsensesoftware avatar May 20 '21 19:05 commonsensesoftware

@commonsensesoftware any updates?

PranjaliSoni avatar Jun 07 '21 14:06 PranjaliSoni

My opinionated dogma aside 😉 , I'm not able to repro your scenario. I've tried against the latest 5.0.0 release and back to 4.1.1. I'm not sure if you have a particular reason to be back on 4.1.1, but 5.0.0 will have the latest features and fixes. 5.0.0 also multi-targets so you can still use .NET Core 3.1.

I tried following all the points from the discussion, but clearly our streams are crossed. You first mentioned putting [Consumes( "application/merge-patch+json" )] on a GET operation, which I would not expect to work because they don't (or shouldn't) accept any content. I'm not completely sure if that can be made to work. You also mentioned putting it on the OrdersController in a sample project. Based on what I saw, I assume the sample you are referring to is in the Swagger sample and you made the changes to the 1.0 version of the controller.

I saw things work as expected. If you want to support multiple media types, then you have to add them all:

/// <summary>
/// Places a new order.
/// </summary>
/// <param name="order">The order to place.</param>
/// <returns>The created order.</returns>
/// <response code="201">The order was successfully placed.</response>
/// <response code="400">The order is invalid.</response>
[HttpPost]
[MapToApiVersion( "1.0" )]
[Produces( "application/json" )]
[Consumes( "application/json", "application/merge-patch+json" )]  // ← list all supported media types
[ProducesResponseType( typeof( Order ), 201 )]
[ProducesResponseType( 400 )]
public IActionResult Post( [FromBody] Order order )
{
    order.Id = 42;
    return CreatedAtAction( nameof( Get ), new { id = order.Id }, order );
}

With this in place, the Swagger UI will show both media types and they both succeed with 201. These are the only changes I made in the sample project.

ASIDE: More dogma, but application/merge-patch+json - by spec - is only supposed to be used with PATCH, but it seems ASP.NET doesn't care. It worked with POST just fine. Something to consider in your final solution.

At this point, it's not clear to me how our two setups are different. If you're able to take a sample project, repro the issue, and drop a copy of it here, that will go a long way to making sure I'm on the same page as you.

commonsensesoftware avatar Jun 22 '21 18:06 commonsensesoftware

Hiii, @commonsensesoftware I tried again and can still see the issue, let me help you repro the issue in OrdersController of https://github.com/microsoft/aspnet-api-versioning/tree/master/samples/aspnetcore

  1. I tried on both versions 5.0.0 and 4.1.1
  2. You can repro this issue by using SwaggerSample 1.0 version a. Use Post API of OrdersControllers(correction: not get, that was a typo) b. Add [Consumes( "application/merge-patch+json" )] to the api (not application/json, I want to support only 1) c. Try hitting the API from postman with Content-Type header set as "application/json". (Swagger automatically handles this so you dont the see the issue there)
/// <summary>
        /// Places a new order.
        /// </summary>
        /// <param name="order">The order to place.</param>
        /// <returns>The created order.</returns>
        /// <response code="201">The order was successfully placed.</response>
        /// <response code="400">The order is invalid.</response>
        [HttpPost]
        [MapToApiVersion( "1.0" )]
        [Produces( "application/json" )]
        [Consumes( "application/merge-patch+json" )]
        [ProducesResponseType( typeof( Order ), 201 )]
        [ProducesResponseType( 400 )]
        public IActionResult Post( [FromBody] Order order )
        {
            order.Id = 42;
            return CreatedAtAction( nameof( Get ), new { id = order.Id }, order );
        }

Expected behaviour: It errors out saying media type is not correct(Unsupported media Type) Reason : Only "application/merge-patch+json" is supported.

Actual Behaviour: It errored out with 405 Method not allowed saying api version is not correct(I didnot even change the api-version)

{
    "error": {
        "code": "UnsupportedApiVersion",
        "message": "The HTTP resource that matches the request URI 'http://localhost:5000/api/Orders' with API version '1.0' does not support HTTP method 'POST'.",
        "innerError": null
    }
}

You should be able to repro the issue with this. Let me know if you have any queries. Thanks

PranjaliSoni avatar Jun 23 '21 19:06 PranjaliSoni

I'll reiterate that application/merge-patch+json is only meant for the PATCH method. Here's what I added:

/// <summary>
/// Updates an existing order.
/// </summary>
/// <param name="order">The order to update.</param>
/// <returns>The created order.</returns>
/// <response code="204">The order was successfully updated.</response>
/// <response code="400">The order is invalid.</response>
/// <response code="404">The order does not exist.</response>
[MapToApiVersion( "1.0" )]
[HttpPatch( "{id:int}" )]
[Produces( "application/json" )]
[Consumes( "application/merge-patch+json" )]
[ProducesResponseType( 204 )]
[ProducesResponseType( 400 )]
[ProducesResponseType( 404 )]
public IActionResult Patch( int id, [FromBody] Order order ) => NoContent();

I then tested it with the HTTP REPL instead of the Swagger UI. These are from the default endpoint http://localhost:5000/api> in the REPL.

Defaulting to application/json

patch orders/42?api-version=1.0 -c "{"customer":"Bob Smith"}"

HTTP/1.1 415 Unsupported Media Type
Content-Length: 0
Date: Thu, 24 Jun 2021 15:45:43 GMT
Server: Kestrel

Specifying application/merge-patch+json

patch orders/42?api-version=1.0 -c "{"customer":"Bob Smith"}" -h "Content-Type:application/merge-patch+json"

HTTP/1.1 204 No Content
api-deprecated-versions: 0.9
api-supported-versions: 1.0, 2.0, 3.0
Date: Thu, 24 Jun 2021 15:46:12 GMT
Server: Kestrel

This is exactly what I would expect to have happen. Perhaps something is wrong in your PostMan setup?

commonsensesoftware avatar Jun 24 '21 15:06 commonsensesoftware

@commonsensesoftware
I looked into this issue and I think it is either DotNet core routing issue or API versioning library issue.

So I have created 3 different PRs to see how API versioning lib is behaving according to changes.

Setup-1

  • V1 Orders controller has PATCH method with attribute [Consumes("application/merge-patch+json")]
  • V2 Orders controller has PATCH method with attribute [Consumes("application/merge-patch+json")]

PR: https://github.com/BlackRider97/aspnet-api-versioning/pull/1

It is working correctly as expected.

Case-1 Correct content type -H "Content-Type: application/merge-patch+json" Expected status code: 201 Found status code: 201 ✅

Case-2 Incorrect content type -H "Content-Type: application/json" Expected status code: 415 Unsupported Media Type Found status code: 415 Unsupported Media Type ✅

Setup-2

  • V1 Orders controller has PATCH method with attribute [Consumes("application/merge-patch+json")]
  • V2 Orders controller has PATCH method with attribute [Consumes("application/json")]

PR: https://github.com/BlackRider97/aspnet-api-versioning/pull/2

It is NOT WORKING AT ALL as expected.

Case-1 Correct content type -H "Content-Type: application/merge-patch+json" Expected status code: 201 Found status code: 405 Method Not Allowed ❌

Case-2 Incorrect content type -H "Content-Type: application/json" Expected status code: 415 Unsupported Media Type Found status code: 405 Method Not Allowed ❌

Setup-3

  • V1 Orders controller has PATCH method with attribute [Consumes("application/merge-patch+json")]
  • V2 Orders controller has PATCH method without any Consumes attribute

PR: https://github.com/BlackRider97/aspnet-api-versioning/pull/3

It is NOT WORKING PARTIALLY as expected.

Case-1 Correct content type -H "Content-Type: application/merge-patch+json" Expected status code: 201 Found status code: 201 ✅

Case-2 Incorrect content type -H "Content-Type: application/json" Expected status code: 415 Unsupported Media Type Found status code: 405 Method Not Allowed ❌

Now @PranjaliSoni is talking about setup-3. For her it is not working as expected when wrong content type is passed.

BlackRider97 avatar Jun 25 '21 14:06 BlackRider97

@BlackRider97, @PranjaliSoni thanks for the updated information; this is useful. This was a very deep investigation. Lucky you, you're the winner of two bugs!

The first bug is in API Versioning and will happen when:

  1. There are multiple candidates for a route template
  2. The actions specify different media types
  3. The only reason for the lack of a match is media type

The reason you get 405 instead of 400 is because there is other matching candidates. It is possible that methods are added or removed between versions, which is the reason for 405 (e.g. PATCH isn't being found, when it should be).

It should be possible to properly report 415 instead of 405, but unfortunately I'm not currently able to fix it due to a bug I've discovered (and reported) in dotnet/aspnetcore#33865. Something in the routing system is filtering out possible candidates. The lack of all the candidate information is preventing me from properly detecting 415 across API versions (which is this special case).

The possible, temporary workaround is:

  • Use heterogenous media types across your controllers
  • Explicitly return 415 if a request reaches a controller that shouldn't support it; an action filter to help here
  • If you need OpenAPI/Swagger support, you'll need some type of filter to fix-up (e.g. remove) media types on APIs that don't actually support it

This is a yucky, lame solution, but I'm blocked up stream. I suspect it may take weeks or even months for the ASP.NET team to fix this issue. We'll have to see how much of a priority they make it. You might be able to flex your muscle on their repo. The linked issue illustrates how to repro the issue, even without API Versioning.

commonsensesoftware avatar Jun 26 '21 02:06 commonsensesoftware

I just wanted to circle back around and provide a quick update on this issue. After a lot of researching, I can confirm that this is indeed a bug and a misunderstanding (on my part) about what extensions to the routing system are necessary. I've made the necessary changes, which you can now find in the new main branch. Expect the fix to be available in the next major release (6.0). I'm very hesitant to backport this change to 5.x patch because it results in a behavioral breaking change to routing; regardless, of the method that you use.

On a positive note, 415 will be properly reported without needing to change the error response status code. If you version by media type, and only by media type, then clients will receive 415 when there's no match.

The code should be ready if you want to play around with it. I'm actively working getting things setup to publish new packages.

commonsensesoftware avatar Mar 19 '22 18:03 commonsensesoftware

The Preview 2 release available and you can pick up the changes from the new package Asp.Versioning.Mvc. While I could make the change in 5.1, I've decided not to because it results in a behavioral breaking change that will likely not be obvious.

I've lifted this use case into the test suite. I believe it will now do what you expect in a more consistent manner for 404, 405, and 415. If you're able to confirm that would help close this issue out.

The latest examples can be found here.

commonsensesoftware avatar Apr 09 '22 18:04 commonsensesoftware

6.0 has been officially released. It contains the official, final release of the changes discussed throughout this thread. You discovered a pretty big hole in the implementation, but a lot of good things came of it. Thanks for reporting it. If you continue to have any issues in 6.0, feel free to comment or we can reopen the issue. Thanks.

commonsensesoftware avatar Aug 24 '22 02:08 commonsensesoftware