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

Simple string returned by controller action is not a valid JSON! #4945

Closed
ryanelian opened this issue Jul 1, 2016 · 10 comments
Closed

Simple string returned by controller action is not a valid JSON! #4945

ryanelian opened this issue Jul 1, 2016 · 10 comments

Comments

@ryanelian
Copy link

ryanelian commented Jul 1, 2016

[HttpGet]
public string Get()
{
    return "Hello World!";
}

ASP.NET MVC Core 1.0.0 does this:

200 OK

Transfer-Encoding: chunked
Content-Type: application/json; charset=utf-8
Server: Kestrel
X-SourceFiles: =?UTF-8?B?RDpcVlNcQWNjZWxpc3QuQUlTXEFjY2VsaXN0LkFJUy5XZWJBcHBcYXBpXHZhbHVlcw==?=
X-Powered-By: ASP.NET
Date: Fri, 01 Jul 2016 10:18:31 GMT

Hello World!

Meanwhile in ASP.NET Web API 5.2.3 does this:

200 OK

Cache-Control: no-cache
Pragma: no-cache
Content-Type: application/json; charset=utf-8
Expires: -1
Server: Microsoft-IIS/10.0
X-AspNet-Version: 4.0.30319
X-SourceFiles: =?UTF-8?B?RDpcVlNcQWNjZWxpc3QuTURNXEFjY2VsaXN0Lk1ETS5TZXJ2ZXJcYXBpXHRlc3Q=?=
X-Powered-By: ASP.NET
Date: Fri, 01 Jul 2016 10:22:10 GMT
Content-Length: 13

"Hello World"

ASP.NET MVC 5.2.3, when returningJson("Hello World", JsonRequestBehavior.AllowGet), does this:

200 OK

Cache-Control: private
Content-Type: application/json; charset=utf-8
Server: Microsoft-IIS/10.0
X-AspNetMvc-Version: 5.2
X-AspNet-Version: 4.0.30319
X-SourceFiles: =?UTF-8?B?RDpcVlNcQWNjZWxpc3QuTURNXEFjY2VsaXN0Lk1ETS5TZXJ2ZXJcaGVsbG8=?=
X-Powered-By: ASP.NET
Date: Fri, 01 Jul 2016 10:23:57 GMT
Content-Length: 13

"Hello World"

This is an unexpected behavior for me. Is this a bug or feature? I believe "Hello World!" is a valid JSON response, not Hello World!. Javascript JSON.parse() function can parse the former but not the latter.

@rynowak rynowak added this to the 2.0.0 milestone Jul 5, 2016
@rynowak
Copy link
Member

rynowak commented Jul 5, 2016

I had a chance to discuss this with @Eilon this AM, and I think that while the current behavior is what was intended, it's not super useful. It's also not something we'd be willing to change for a 1.1.0 - hence the 2.0.0 label.

Some Background

The intention here was to make it easy and possible to return plaintext from an action, the "Hello, World" scenario. This has long been MVC's behavior - but, as you pointed out this conflicts with WebAPI's historical behavior which would be to content-negotiate and choose XML or JSON based on your configuration.

The compromise for this was to introduce StringOutputFormatter - a formatter that is only active for string objects and will do the following:

  • If there is no 'best' content type then return the text verbatim with text/plain
  • If there is a 'best' content type then return the text verbatim with the 'best' content type

I'm glossing over a bit what we mean exactly by 'best' content type - what you should know is that 'best' typically means what's in the accept header or [Produces(...)] OR most importantly if your request is coming from a browser, the accept header is ignored and there is no 'best' unless you are using [Produces(...)].

So we can update the table from above to describe what usually will happen:

  • If the request comes from a browser, then return the text verbatim with text/plain
  • If the request came from an API client, there is a 'best' content type and so return the text verbatim with the 'best' content type

The Mistake

The mistake here is in this case:

If the request came from an API client, there is a 'best' content type and so return the text verbatim with the 'best' content type

The assumptions here on our part are wrong. You can't write an action that returns text that's meant to be written verbatim in multiple formats. If you had done that, it's because you did conneg already in the action, and don't need us to do it for you. If you manually formatted a string as JSON in your action and returned it, we wouldn't be able to somehow make it XML (or any other format).

So clearly this behavior isn't useful, and it's what you're running into as there's a Content-Type: application/json; charset=utf-8 in the response.

The Fix

Our future fix for this will likely be to drop case 2. The StringOutputFormatter will behave like this:

  • If the request comes from a browser, then return the text verbatim with text/plain
  • If the request came from an API client requesting text/plain return the text verbatim

This is nice because you can control the default experience by using [Produces(...)] as a global filter.

Unfortunately, this is subtle change in behavior that we're not comfortable making in a 1.1.0 release. Someone out there has probably taken a dependency on this even though it's not optimal.

The Workaround

My suggestion if you don't like how this works today is just to remove the string formatter.

services.AddMvc(options => options.OutputFormatters.RemoveType<StringOutputFormatter>());

If you're writing an API, it's not likely that this would enable anything you need anyway.

@ryanelian
Copy link
Author

ryanelian commented Jul 6, 2016

Actually, I find the Core MVC 1.0.0 behavior to be useful because it lets my client consume the text immediately without deserializing.

But I was surprised when I discovered this 'feature' when I was coaching about MVC / EF Core. I thought it was a bug, hence the issue.

Thanks for the heads-up and the workaround. :D

EDIT:

Just tested. Curiously this 'bug' doesn't break AngularJS 1.5.7 $http. Yay?

@leppie
Copy link

leppie commented Jul 11, 2016

Workaround should be (as there is no Formatters property):

services.AddMvc(options => options.OutputFormatters.RemoveType<StringOutputFormatter>());

@rynowak
Copy link
Member

rynowak commented Jul 11, 2016

@leppie - thanks, updated my post.

@jods4
Copy link

jods4 commented Jul 20, 2016

@rynowak

If the request comes from a browser...

Do I understand correctly? Does this include a Fetch request from a JS application? If the answer is yes, I don't understand why you ignore the client Accept header. If my app asks for application/json you SHOULD return that and not text/plain.

@rynowak
Copy link
Member

rynowak commented Jul 20, 2016

Does this include a Fetch request from a JS application?

No.

What I mean by this is that we (by default) ignore the Accept header when it contains the the wildcard */*. This generally means that the client is something that's going to just display the text verbatim rather than trying to understand the data. In that case we want the ordering of formatters to take over rather than relying on what's in the Accept header.

For instance, consider headers like this:

Firefox -   Accept:text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Chrome - Accept: application/xml,application/xhtml+xml,text/html;q=0.9, text/plain;q=0.8,image/png,*/*;q=0.5

We'd ignore these, give each formatter in sequence a chance to respond. API clients tend to be specific about what they want and won't claim to Accept: */*.

If this doesn't do what you want for some reason you can turn it off.

services.AddMvc(options => options.RespectBrowserAcceptHeader = true);

@jods4
Copy link

jods4 commented Jul 20, 2016

@rynowak ok then!

@Eilon Eilon modified the milestones: 2.0.0, 1.2.0 Dec 19, 2016
@Eilon
Copy link
Member

Eilon commented Jan 5, 2017

Let's take a stab at implementing the solution that @rynowak suggested.

@leppie
Copy link

leppie commented Jan 25, 2017

@kichalla thanks!

@TylerBrinkley
Copy link

Just bit by this in ASP.NET Core 2.0 when there is no accept header.

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

7 participants