ObjectResultExecutor should hold the Accept header in higher regard

[tuespetre]

The problem behavior

Here is the rundown of the issue I'm seeing:

1. I have a controller action decorated with [Produces("application/vnd.my-app+json; version=1")]
2. I have a custom OutputFormatter that will say it CanWriteResult as long as the content type is application/vnd.my-app+json, regardless of media type parameter values
3. I have RespectBrowserAcceptHeader (why the 'browser' in the name?) set to true to make sure any Accept header is respected (Just in case, I guess? It looks like maybe it only has to do with */* wildcard media types...)
4. I send a fetch to the controller action from my browser's console with an Accept header value of application/vnd.derp and receive a 200 response with a Content-Type header value of application/vnd.my-app; version=1

The expected behavior

I would expect content negotiation to honor the Accept request header in all cases and respond with 406 whenever it cannot locate a formatter that applies to a media type specified in the Accept header, rather than responding with whatever it can muster up even when the client has explicitly declared what it will accept.

Possible workaround

In RC2, the OutputFormatterWriteContext exposes the HttpContext. A custom formatter could take advantage of that in order to properly respect the Accept header value, but this breaks the current negotiator/formatter pattern and causes the formatter to violate the Single Responsibility Principle.

In RC1, well, there's nothing that easy that I can see offhand.

[khellang]

Have you added the HttpNotAcceptableOutputFormatter?

[javiercn]

Hi, my guess is that there might be a formatter being chosen to serve the request. You can override https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectResultExecutor.cs#L401 call base and see if it returns a result to verify that is what is happening.

See https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectResultExecutor.cs#L180 for more details

And https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectResultExecutor.cs#L128 is where I expect to get a 406.

As far as I know, when you use Produces, the accept header is not taken into account. @rynowak can confirm

[tuespetre]

@khellang I have not, but glancing at the source, my scenario is not meeting the conditions necessary to hit the code that would trigger it. A ProducesAttribute with one media type specified ends up bypassing negotiation altogether. I think the HttpNotAcceptableOutputFormatter is a bit of a mess though; it's not really a formatter, and the FailedContentNegotiation seems to only exist for it to use, when no formatter really has any business making use of such information. 💰 💰

@javiercn you are right, and that is exactly what I have seen. My formatter is being chosen even though the request specifies an Accept header that is not compatible with my formatter's target media type. This is because the ObjectResultExecutor is not paying any mind to the Accept header.

As far as I know, when you use Produces, the accept header is not taken into account. @rynowak can confirm

This is true, except for cases where the ProducesAttribute has multiple media types specified -- then, the ObjectResultExecutor kinda-sorta takes the Accept header into account, but will ultimately still fall back to an unacceptable formatter.

[Eilon]

@dougbu assigning to you because PR #4623 is assigned to you.

[dougbu]

👍

[dougbu]

14f68b6