Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

When a Controller operation returns byte[] the content should automatically be set to application/octet-stream #7926

Closed
ghost opened this issue Jun 16, 2018 · 14 comments

Comments

@ghost
Copy link

ghost commented Jun 16, 2018

Is this a Bug or Feature request?:

From my standpoint it's a bug, a design one at least, but I could understand it perceived as a feature.

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

This post from @davidfowl demonstrates that returning a byte[] doesn't work as one should expect: setting automatically the content as application/octet-stream and having MVC returning the data properly.

From the example below, the first method doesn't work and the two next ones do.

public class HomeController : Controller
{
    [HttpGet("/bin")]
    [Produces("application/octet-stream")]
    public byte[] GetBin()
    {
        return Encoding.UTF8.GetBytes("Hello World");
    }

    [HttpGet("/bin2")]
    public Task GetBinRaw()
    {
        Response.ContentType = "application/octet-stream";
        var buffer = Encoding.UTF8.GetBytes("Hello World");
        return Response.Body.WriteAsync(buffer, 0, buffer.Length);
    }

    [HttpGet("/bin3")]
    [Produces("application/octet-stream")]
    public Stream GetBinStream()
    {
        return new MemoryStream(Encoding.UTF8.GetBytes("Hello World"));
    }
}

Description of the problem:

When one wants to return binary data in the HTTP body of the Response, right now the only way is to rely on Stream, returning byte[] won't work but it is however a very natural way to do it imho. The content type should be automatically set as byte[] could only be perceived as binary data.

Right now while returning this type the requester always gets a StatusCode of 406 (not acceptable) and no content.

Version of Microsoft.AspNetCore.Mvc:

2.1

@mkArtakMSFT
Copy link
Member

Thanks for contacting us, @nockawa.
@glennc. @pranavkm thoughts?

@pranavkm
Copy link
Contributor

pranavkm commented Jun 18, 2018

There is a set of FileContentResult based helper methods on ControllerBase that accept a byte[] and a content type: https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.mvc.controllerbase.file?view=aspnetcore-2.1#Microsoft_AspNetCore_Mvc_ControllerBase_File_System_Byte___System_String_ that should do what you need.

@davidfowl
Copy link
Member

Why doesn't returning byte[] do that?

@pranavkm
Copy link
Contributor

The Json \ Xml formatters convert it to a base-64 encoded string. So it does work today, just not what @nockawa is expecting it to do.

@davidfowl
Copy link
Member

That's interesting, because returning a Stream is treated differently to returning a byte[]. I would expect they would act similarly.

@rynowak
Copy link
Member

rynowak commented Jun 19, 2018

I'd suggest implementing something like this with a formatter. If you want to send a PR, we can put something like this in the box for others to use (off by default).

I don't think there's much reason why we would consider changing the defaults, as mentioned here, we already have a defined behavior here that most users will see (JSON + Base64 encoding)

@davidfowl
Copy link
Member

I don't think there's much reason why we would consider changing the defaults, as mentioned here, we already have a defined behavior here that most users will see (JSON + Base64 encoding)

Right, this would be a breaking change otherwise.

@ghost
Copy link
Author

ghost commented Jun 19, 2018

@pranavkm @rynowak Tell me if I understand this correctly:
If we don't set the ContentType and return byte[] then it will encode the result in Base64 and return JSON content type.

Other questions:

  1. What about if I manually set the content type to application/octet-stream ? On would expect the data to be interpreted as binary?
  2. Why would this require a specific formatter as I've chose the binary type through the ContentType?

Thanks,
Loïc

@rynowak
Copy link
Member

rynowak commented Jun 19, 2018

Formatters (IOutputFormatter specifically) are the thing that is responsible for writing a method's return value into the response body. This usually involves looking at the request (or response) content type and making a decision based on the content type + .NET return type.

Here's an example of a simple one: https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/Formatters/StreamOutputFormatter.cs

The answers to your questions:

  1. That's only true if there's a formatter that wants to write byte[] In this case the JSON formatter understands how to write byte[] so it's handling it. If you set the content type to application/octet-stream I would expect the JSON formatter to skip it. Since no other formatter handles it, it becomes a 406.
  2. Formatters are the thing that associates a content type + .NET type and writes them to the output

@ghost
Copy link
Author

ghost commented Jun 19, 2018

Sorry, but I still don't understand why when I set the content type to aplication/octet-stream it could hurt in any kind of ways to allow returning a byte[] as anybody would expect: sending binary data!
I'm porting a service from Go to .net core and my colleague just laughed about this lack of feature and simplicity...

@mkArtakMSFT
Copy link
Member

In general, MVC delegates how .NET objects are formatted into responses through Output formatters.
The output formatters take precedence over modifications made directly to the HttpContext.
We have few special cases, where based on the type of the response we define content types. Specifically String and Stream. We do not currently special case byte[] and doing so would be a breaking change. As you've mentioned above, there are easy workarounds available like returning a Stream. That said, we don't plan to change this behavior as the impact is going to be too big.

@davidfowl
Copy link
Member

@mkArtakMSFT @rynowak I think @nockawa is right here. It actually wouldn't be a breaking change to introduce this behavior and it would meet customer expectations. Basically we would have a BinaryOutputFormatter that could handle the application/octet-stream content type and handle byte[] return types. It seems odd that Stream works but byte[] does not.

@rynowak
Copy link
Member

rynowak commented Jun 20, 2018

I agree that it's not a breaking change - if it's limited to cases where conneg decides that application/octet-stream is the appropriate content type and byte[] is returned.

For context, I'm officially on the record as opposed to putting in more on by default output formatters for specialized scenarios. I was also against Stream and I don't think that feature gets of ton of usage. We get feedback from all possible sides of every default behavior the framework implements. What's obvious to one person is not always obvious or easier to understand to another - in fact users tell us that their obvious desires are often in conflict. Now in this case I can't imagine another obvious behavior that you'd want (other than base64, which is already a thing) - but if we do this I'm sure we'll find out.

What would change my mind about this is understanding how this is much better than both what we already have (return File(someBytes, "application/octet-stream")) - and also much better than the alternatives (suppose we added return Bytes(someBytes)).

Would we tell everyone to 'do it this way'? Would we change our documentation to say that this is the correct way to return bytes vs File(...)?

Keep in mind we're talking about the case where user code sets both the content type and returns bytes 😆 - and we're talking about it in the context of adding a new thing to an existing set of features - not the world of green-field, ideal design. We haven't yet built any features that are targeted only at cases where application code sets a content type.

So your code either looks like:

public async Task<ActionResult<byte[]>> MyAction() 
{
    ...
    response.ContentType = "application/octet-stream"
    return someBytes;
}

OR

[Produces("application/octet-stream")]
public async Task<ActionResult<byte[]>> MyAction() 
{
    ...
    return someBytes;
}

OR

public async Task<ActionResult> MyAction() 
{
    ...
    return new ObjectResult() {  ContentType = "application/octet-stream", Value = someBytes };
}

@ghost
Copy link
Author

ghost commented Jun 20, 2018

In my case I'm looking for the fastest way for my WebApi method to operate. I'm returning a raw bytes of a serialized Protobuf object that is being cached to avoid multiple serializations. (which has nothing to do with File...)

So the object I have on the server site in a byte[], the most efficient one, with the lowest footprint.
If I understand correctly, down the road if I set the content type and return a byte[] the internal implementation will write its content in the Response.Body which is basically what I'm doing now thanks to @davidfowl's answer.

But for the unadvised user (and sorry, but considering the state of the documentation so far, it will happen), to set the content type and returning a byte[] and ending up with WebAPI telling the caller the request was "not acceptable" (406), that is unacceptable to me.

I don't understand why such natural behavior is the center such debate:

  • There's no regression: because I have to set the ContentType myself
  • The behavior can only be natural to people: byte[] is a chunk of raw data in many languages.
  • I'm sure the implementation wouldn't be such a big of a deal.

I've convinced people in my company to switch to .net core because asp.net core is fast and simple to code...The web is not all about returning json data...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants