dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.59k stars 10.06k forks source link

Media type application/problem+json still lost in combination with ProducesAttribute #39802

Open dsun1 opened 2 years ago

dsun1 commented 2 years ago

Is there an existing issue for this?

Describe the bug

This issue is the same as reported and resolved here: https://github.com/dotnet/aspnetcore/issues/19510

Having tested with the project that is the previous issue's minimal repro (in .NET 6 instead of .NET core 3.1), I've found the same issue still persists.

That is, a controller with ProducesAttribute set to application/json will set the response header media-type to application/json when it's returning a ProblemDetails or ValidationProblemDetails. It's supposed to return application/problem+json.

Expected Behavior

When the controller or framework returns a ProblemDetails or ValidationProblemDetails, the response header media type should be application/problem+json as per https://datatracker.ietf.org/doc/html/rfc7807#section-3

Steps To Reproduce

Minimalistic repro project: https://github.com/dsun1/ProblemMediaTypeIssue

Attribute a controller class or method with ProducesAttribute("application/json"). If that controller returns a problem, or if the framework returns a validation problem, the response media type will be application/json instead of application/problem+json.

Exceptions (if any)

No response

.NET Version

6.0.101

Anything else?

dotnet --info

.NET SDK (reflecting any global.json):
 Version:   6.0.101
 Commit:    ef49f6213a

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  11.0
 OS Platform: Darwin
 RID:         osx.10.16-x64
 Base Path:   /usr/local/share/dotnet/sdk/6.0.101/

Host (useful for support):
  Version: 6.0.1
  Commit:  3a25a7f1cc

.NET SDKs installed:
  2.1.818 [/usr/local/share/dotnet/sdk]
  3.1.416 [/usr/local/share/dotnet/sdk]
  5.0.404 [/usr/local/share/dotnet/sdk]
  6.0.101 [/usr/local/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.30 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.30 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.22 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.13 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.1 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.30 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.22 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.13 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.1 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
angularsen commented 2 years ago

I was just about to file this exact same issue, dumping my report here instead.

This was reported in #19510 and fixed in #30367, included in .NET 6.0-preview3.

I can still reproduce this issue, tested on:

Minimal repro here: https://github.com/angularsen/repro-aspnetcore-19510

Repro details

✅ Test 1 - Problem() without [Produces]

Returns application/problem+json as expected.

curl -i http://localhost:5106/test/1
HTTP/1.1 500 Internal Server Error
Content-Type: application/problem+json; charset=utf-8
Date: Thu, 27 Jan 2022 07:53:16 GMT
Server: Kestrel
Transfer-Encoding: chunked

{"type":"https://tools.ietf.org/html/rfc7231#section-6.6.1","title":"An error occurred while processing your request.","status":500,"detail":"Test1 ✅ - Problem(string) without ProducesAttribute => returns application/problem+json. Assembly: C:\\Program Files\\dotnet\\shared\\Microsoft.AspNetCore.App\\6.0.1\\Microsoft.AspNetCore.Mvc.Core.dll","traceId":"00-f837e61b4f3fe38e1326d4b736ce4b1b-5f42df452447a45b-00"}

❌ Test 2 - Problem() with [Produces("application/json")]

Returns application/json instead of expected application/problem+json.

curl -i http://localhost:5106/test/2
HTTP/1.1 500 Internal Server Error
Content-Type: application/json; charset=utf-8
Date: Thu, 27 Jan 2022 07:54:47 GMT
Server: Kestrel
Transfer-Encoding: chunked

{"type":"https://tools.ietf.org/html/rfc7231#section-6.6.1","title":"An error occurred while processing your request.","status":500,"detail":"Test2 ❌ - Problem(string) with [Produces(\"application/json\")] => incorrectly returns application/json instead of application/problem+json. Assembly: C:\\Program Files\\dotnet\\shared\\Microsoft.AspNetCore.App\\6.0.1\\Microsoft.AspNetCore.Mvc.Core.dll","traceId":"00-7898b5db7c72df8b1589c0776c857321-1145d480210ea6e5-00"}

❌ Test 3 - Problem() with [Produces("application/json", "application/problem+json")]

Returns application/json instead of expected application/problem+json.

curl -i http://localhost:5106/test/3
HTTP/1.1 500 Internal Server Error
Content-Type: application/json; charset=utf-8
Date: Thu, 27 Jan 2022 08:50:00 GMT
Server: Kestrel
Transfer-Encoding: chunked

{"type":"https://tools.ietf.org/html/rfc7231#section-6.6.1","title":"An error occurred while processing your request.","status":500,"detail":"Test3 ❌ - Problem(string) with [Produces(\"application/json\", \"application/problem+json\")] => incorrectly returns application/json instead of application/problem+json. Assembly: C:\\Program Files\\dotnet\\shared\\Microsoft.AspNetCore.App\\6.0.1\\Microsoft.AspNetCore.Mvc.Core.dll","traceId":"00-4e825df690029941d19792a19b6e1346-1e417b4f47ca76cf-00"}

❌ Test 4 - Problem() with [Produces("application/problem+json", "application/json")]

Returns application/problem+json, but then Ok(object) also returns application/problem+json instead of expected application/json.

curl -i http://localhost:5106/test/4
HTTP/1.1 500 Internal Server Error
Content-Type: application/problem+json; charset=utf-8
Date: Thu, 27 Jan 2022 07:55:05 GMT
Server: Kestrel
Transfer-Encoding: chunked

{"type":"https://tools.ietf.org/html/rfc7231#section-6.6.1","title":"An error occurred while processing your request.","status":500,"detail":"Test4 ❌ - Problem(string) with [Produces(\"application/problem+json\", \"application/json\")] => returns content-type application/problem+json, but then Ok(string) also returns application/problem+json. Assembly: C:\\Program Files\\dotnet\\shared\\Microsoft.AspNetCore.App\\6.0.1\\Microsoft.AspNetCore.Mvc.Core.dll","traceId":"00-cb867abbe8d88fbced809ff7764199fe-bb8a5378a54e6789-00"}
dotnet --info ``` dotnet --info .NET SDK (reflecting any global.json): Version: 6.0.101 Commit: ef49f6213a Runtime Environment: OS Name: Windows OS Version: 10.0.22000 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\6.0.101\ Host (useful for support): Version: 6.0.1 Commit: 3a25a7f1cc .NET SDKs installed: 3.1.416 [C:\Program Files\dotnet\sdk] 5.0.301 [C:\Program Files\dotnet\sdk] 5.0.302 [C:\Program Files\dotnet\sdk] 6.0.100-rc.2.21505.57 [C:\Program Files\dotnet\sdk] 6.0.100 [C:\Program Files\dotnet\sdk] 6.0.101 [C:\Program Files\dotnet\sdk] .NET runtimes installed: Microsoft.AspNetCore.App 3.1.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.22 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.0-rc.2.21480.10 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.22 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0-rc.2.21480.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.16 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.22 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.0-rc.2.21501.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] ```
captainsafia commented 2 years ago

❌ Test 4 - Problem() with [Produces("application/problem+json", "application/json")]

So, the ProducesAttribute has an OnResultExecuting hook that runs before the logic in the ObjectResultExecutor that was modified in the referenced PR runs. This OnResultExecuting will clear out result.ContentTypes and replace it with the content types that are defined in the ProducesAttribute.

https://github.com/dotnet/aspnetcore/blob/b21596afeb7412844ee5e19f6e5bef832c533c0f/src/Mvc/Mvc.Core/src/ProducesAttribute.cs#L91

https://github.com/dotnet/aspnetcore/blob/b21596afeb7412844ee5e19f6e5bef832c533c0f/src/Mvc/Mvc.Core/src/ProducesAttribute.cs#L101-L108

So, this is going to set result.ContentTypes to ["application/problem+json", "application/json"]. Now the InferContentType method modified in the referenced PR effectively no-ops because result.ContentTypes is not empty and the result.Value is not ProblemDetails.

https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs#L130-L142

I believe that the tests introduced in that PR didn't catch the issue because they don't account for the fact that the ProducesAttribute modifies the content types in this way.

I think the fundamental right choice here is to modify the ProducesAttribute such that the OnResultExecuting invocation no-ops under certain scenarios since that's the root cause here.

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

captainsafia commented 2 years ago

@brunolins16 Not sure if you want to consider this under the bucket of ProblemDetails-related items.

Not sure about relative priority compared to the cost of fix here, so might not be worth doing...

agilenut commented 2 years ago

Fingers crossed this gets fixed. Right now, I have to create an extra operation filter in every API I create.

angularsen commented 2 years ago

Just stumbled on this: JSON output is the default negotiated content format.

For our JSON APIs, this means [Produces("application/json")] is largely redundant and we can remove it from our API controllers to get the correct response content type header application/problem+json when returning Problem(), ValidationProblem() and other object results producing a ProblemDetails response.

We have generally transitioned from ActionResult to exception flow to control error responses with https://github.com/khellang/Middleware, but many old endpoints still return ActionResult.

dnperfors commented 2 years ago

One reason we use [Produces("application/json"] is because we want our swagger documentation to show application/json for our normal content. We would be glad if this would be fixed properly (and I would even consider helping with the PR for this). For now I would probably go for a custom ProducesAttribute to "fix" the problem myself.

--- edit --- The workaround I have chosen for now is to use [SwaggerResponse(StatusCodes.Status200OK, null, typeof(...), MediaTypes.Application.Json)] without [Produces(MediaTypes.Application.Json)]. This will return the correct Content types in our swagger documentation and the correct content type in all responses (that I have tested)

agilenut commented 2 years ago

One reason we use [Produces("application/json"] is because we want our swagger documentation to show application/json for our normal content.

Same here. We need to be able to use both and get the correct media types.

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

agilenut commented 2 years ago

I know this sounds silly but this was actually my most anticipated "feature" of .Net 7.

RenegadeVile commented 2 years ago

I know this sounds silly but this was actually my most anticipated "feature" of .Net 7.

Same here. I came here hoping to see it fixed.

captainsafia commented 2 years ago

I think the fundamental right choice here is to modify the ProducesAttribute such that the OnResultExecuting invocation no-ops under certain scenarios since that's the root cause here.

@brunolins16 What do you think about the proposed solution here?

niemyjski commented 1 year ago

Is this going to get fixed?

7amou3 commented 5 months ago

still waiting the fix

tylerohlsen commented 4 months ago

I found a workaround. If I remove the [Produces(...)] attribute and replace it with:

[ProducesResponseType(typeof(MyResponseType), statusCode: 200, "application/json")]
[ProducesResponseType(typeof(ProblemDetails), statusCode: 400, "application/problem+json")]
[ProducesResponseType(typeof(ProblemDetails), statusCode: 500, "application/problem+json")]

then the response has the correct content-type header (and swagger documentation is correct).

Or if you want to make it automatic on every endpoint:

.AddControllers(options => {
    options.Filters.Add(new ProducesResponseTypeAttribute(typeof(ProblemDetails), statusCode: 400, "application/problem+json"));
    options.Filters.Add(new ProducesResponseTypeAttribute(typeof(ProblemDetails), statusCode: 500, "application/problem+json"));
})

and then I only need to add the [ProducesResponseType(...)] attribute for the success case (200/201/204) on each controller action

captainsafia commented 4 months ago

Assigning myself to this to see if I can land on a solution for this as part of the recent wave of OpenAPI-work. I'm particularly eyeing the change in behavior between [Produces("application/json", "application/problem+json")] and [Produces("application/problem+json", "application/json)].

I have to revisit the analysis I did in https://github.com/dotnet/aspnetcore/issues/39802#issuecomment-1036748549. Hopefully, there is a straightforward way to apply this fix without having to considerably break into the content-neg layer in MVC.