Grape: JSON endpoints accept 'text/plain' in JSON format

[cmaitchison]

According to the Grape documentation, this endpoint should only accept JSON requests, and all other formats should result in a 406 response code.

The following API will only respond to the JSON content-type and will not parse any other input than application/json, application/x-www-form-urlencoded, multipart/form-data, multipart/related and multipart/mixed. All other requests will fail with an HTTP 406 error code.

class Twitter::API < Grape::API
  format :json
  default_format :json
end

I am finding that posting text/plain in JSON format does not result in a 406. Debugging within the endpoint shows that env['CONTENT_TYPE'] is indeed text/plain.

[dm1try]

The following API will only respond to the JSON content-type and will not parse any other input than application/json, application/x-www-form-urlencoded, multipart/form-data, multipart/related and multipart/mixed. All other requests will fail with an HTTP 406 error code.

class Twitter::API < Grape::API
  format :json
end

When the content-type is omitted, Grape will return a 406 error code unless default_format is specified. The following API will try to parse any data without a content-type using a JSON parser.

class Twitter::API < Grape::API
  format :json
  default_format :json
end

This is definitely a bug. Those two cases described in README are not valid anymore if the request content-type is omitted. Also if default_format is present the provided content type is ignored.

@cmaitchison Could you try to fix this or maybe write some specs? Thanks!

[cmaitchison]

Can do! Thanks for taking a look.

[dblock]

Bump @cmaitchison, would love some help.

[Morred]

Added a spec for the described issue. Seems like this only happens when default_format is set? When I comment it out, the spec will pass.
(Sorry for the double commit reference, the second one is relevant.)

[rodzyn]

@Morred this behaviour is by design I guess

It's described few paragraphs above in README

GET /hello.xls with an Accept: text/plain header has an unrecognized extension and an unrecognized Accept header, so it will respond with JSON (the default format).

So if we specified default_format, any unrecognized stuff will be treated as format defined by default_format (request/response headers).

The other thing is when we don't provide any Content-Type header at all and we didn't set default_format. Then, sadly we don't have any checks performed that could return 406. I've already made some changes that make it possible so PR is coming.

[Morred]

@rodzyn Thanks for clarifying, I assumed this part of the documentation means that default_format will only kick in if there is no content type specified and things with the wrong content type will still be rejected.

[dm1try]

Honestly I have the same feelings as @Morred about this part of the documentation mentioned above.

Also some things are ambiguous to me in the current implementation with default_format.
For example
curl localhost:9292 -d '{}' -H "Content-Type: invalid/type"

class API < Grape::API
  format :json
  default_format :json

  post do
    request.content_type # invalid/type
    params               # but the input is parsed as JSON already
  end
end

@cmaitchison mentioned that thing in the first comment

endpoint shows that env['CONTENT_TYPE'] is indeed text/plain.

@dblock any thoughts?

[dblock]

So there're two sides of the story: what you supply as content-type for POSTed content and what you have in an Accept header in return. So I think we want this:

• Content-type: invalid/type should fail with an error, default format or not.
• no content-type should use the default parser via default format.
• no Accept: ... should use the default formatter.
• Accept: some/type that is not supported by the API should fail instead of default formatting.
• Accept: some/type should decide which formatter to use by default.

This issue already is a confirmed bug (thx @dm1try), I'll take PRs in those lines with careful UPGRADING docs.

[richddr]

is there any update on this issue?

[dblock]

There's a spec in https://github.com/ruby-grape/grape/pull/877 and no fix. Someone please contribute.

[Morred]

Ah, I wrote that spec, forever ago. Will have a look when I get a bit of free time this week!

[inclooder]

Hi, I'd like contribute if no one else is working on that.

[dblock]

Please try HEAD from https://github.com/ruby-grape/grape/pull/1589, thanks @inclooder!