Preserve application/problem+json and application/problem+xml when using ProducesAttribute#66461
Preserve application/problem+json and application/problem+xml when using ProducesAttribute#66461Youssef1313 wants to merge 1 commit intomainfrom
Conversation
…ing ProducesAttribute
There was a problem hiding this comment.
Pull request overview
Adjusts MVC’s ObjectResultExecutor content-type inference so application/problem+json / application/problem+xml are preferred for ProblemDetails, even when [Produces] has already populated ObjectResult.ContentTypes, fixing incorrect application/json selection during formatter negotiation (Issue #39802).
Changes:
- Update
InferContentTypesto insert problem media types at the start ofObjectResult.ContentTypesforProblemDetails. - Expand/parameterize the existing XML negotiation test to catch the
[Produces("application/xml")]regression. - Add a new test ensuring
Response.ContentTypeis still considered as a fallback when no JSON/XML formatter is available.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs | Reorders inferred ProblemDetails content types to prefer application/problem+json/+xml during formatter selection. |
| src/Mvc/Mvc.Core/test/Infrastructure/ObjectResultExecutorTest.cs | Strengthens coverage for ProblemDetails content negotiation and adds fallback test case. |
|
Donot know why checks failed. |
|
@Aijazali777 I restarted it. |
|
This fix addresses the MVC controller path via The minimal API path is actually simpler to fix. All generic typed results flow through A similar fix could be applied in if (contentType is null && value is ProblemDetails)
{
contentType = ContentTypeConstants.ProblemDetailsContentType; // "application/problem+json"
}This would ensure that This would partially address #58574 (the runtime content-type half — the OpenAPI metadata half would still need a separate change). |
Fixes #39802
What happens in this case is
InferContentTypesis called withresult.ContentTypesalready having some content type (which might beapplication/json, for example, in case of[Produces(MediaTypeNames.Application.Json)]).Previously we then added the
application/problem+jsonandapplication/problem+xmlto that list. However,DefaultOutputFormatterSelector.SelectFormatterwill preferapplication/jsonbecause it encounters it first.This PR changes
InferContentTypesto ensure thatapplication/problem+jsonandapplication/problem+xmlare added first to the list.The test
ExecuteAsync_ForProblemDetailsValue_UsesProblemDetailsXMLContentType_BasedOnAcceptHeaderdidn't originally capture the bug because:ObjectResult.ContentTypestotext/plainInferContentTypesthen addsapplication/problem+jsonandapplication/problem+xmlto the end of the list.application/xml, the test had the correct behavior and didn't capture the bug. Because the only content type that is compatible with Accept header is the correctapplication/problem+xml. The test was modified to parameterize the ObjectResult.ContentTypes. When the test input isapplication/xml, the bug would be captured by the test.Response.ContentTypewhen we are dealing with ProblemDetails, in case no formatter can handle json or xml. This test would capture a bug that might happen ifwasEmptyis moved after we added the ProblemJson and ProblemXml.