Header versioning allows jibberish

[mariovisic]

Hi Guys

Currently when header versioning is enabled (even with the strict option) sending a request without a valid vendor string will result in the first version being matched.

On line #37 here the loop will not run if the Accept header doesn't match the string.
https://github.com/intridea/grape/blob/master/lib/grape/middleware/versioner/header.rb#L37

This doesn't seem like the correct behaviour to me. Would it be better to render a 404 instead? If so then i'll see about adding a failing test + patch to solve the problem.

[mbleigh]

The problem is that the Accept header should be allowed to be set to a non-version. For example, if Accept is set to application/json then I should still serve the "default" version of the API regardless. That being said, there are a couple things we could do here:

1. Require an explicit "default" version via a provided option, otherwise render a 404 if none is provided.
2. Set something like X-API-Requested-Version header if the version being provided is an explicit match.

Any other ideas?

[mariovisic]

I think perhaps if the strict option is enabled then it should not serve the default version. If the strict option is not enabled then serving the default version I think should be fine.

Currently enabling the strict option and passing a blank accept header renders a 404. I think this should be extended as most of the time a client will send application/json or similar and get the default version.

[jwkoelewijn]

@mariovisic

I think perhaps if the strict option is enabled then it should not serve the default version. If the strict option is not enabled then serving the default version I think should be fine.

I think this is already taken care of in the Frontier branch:
https://github.com/intridea/grape/blob/frontier/lib/grape/middleware/versioner/header.rb

Specs have been added for it to if I am not mistaken
I hope this solves your problem :)

[jwkoelewijn]

@mariovisic I looked at it, and it seems I was partially wrong, the strict option is used to return a 404 if no accept header is given at all, and in the master branch only the version part is checked. In the frontier branch this behavior is still the same, with the exception that when the strict option is enabled and a vendor is created, a 404 is returned whenever the vendor is incorrect or the version can not be found.

I for one think this is what I would expect when someone uses the accept header to select a version, especially when the subtype is already determined to be 'vnd'.

just my opinion though :)

[dblock]

Pull request #144 fixed some issues around this.

The RFC says this:

If no Accept header field is present, then it is assumed that the client accepts all media types. If an Accept header field is present, and if the server cannot send a response which is acceptable according to the combined Accept field value, then the server SHOULD send a 406 (not acceptable) response.

So that's taken care of, a well formed, but an unacceptable Accept header will cause a 406.

So what's left for this item? For gibberish Accept header I think Grape should return a 400 when used with :strict versionning. Thoughts?

[dblock]

Bump? Comments?

[jpaas]

I would like to see strict mean that the header must match one of the mounted versions, otherwise 406.

[paulnicholson]

I agree with @jpaas, our version header includes a format key and I would like it to return a 406 unless the accept header include the current vendor and format.

[dentarg]

I would like to see strict mean that the header must match one of the mounted versions, otherwise 406.

This is what I expected after implementing versioning according to the README of grape.

[dblock]

I am down with the above, PR someone please?

[dm1try]

Fixed in #1101