Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Versioning accept header strict/cascade settings appears to not work in this case #1082

Closed
elliotlarson opened this issue Jul 30, 2015 · 3 comments
Labels

Comments

@elliotlarson
Copy link
Contributor

If I have this config for versioning...

version 'v1', using: :header, vendor: 'acme', strict: true, cascade: false

then this should require that I pass the appropriate Accept request header to get a valid 200 response.

This request should work:

$ curl -H 'Accept:application/vnd.acme-v1+json' http://localhost:3000/api/projects
=> succeeds √ 

... and it does, returning a successful 200 response.

But, not passing the correct Accept header should result in an error. For example, this request:

$ curl -H 'Accept:application/vnd.acme-nonexistentversion+json' http://localhost:3000/api/projects
=> fails √ 

... fails with a 406 response as expected.

However, this request returns a successful 200 API response, which is unexpected:

$ curl -H 'Accept:application/xml' http://localhost:3000/api/projects
=> succeeds ?

I didn't make the request with the correct version Accept header, but I'm getting a successful 200 response. Is this a bug, or have I misunderstood how the strict: true, cascade: false config settings are supposed to work?

@elliotlarson elliotlarson changed the title Versioning accept header strict setting appears to not work in this case Versioning accept header strict/cascade settings appears to not work in this case Jul 30, 2015
@dblock
Copy link
Member

dblock commented Jul 31, 2015

I believe your logic is correct. Without a valid Accept header you have no matching version and it should fail with a 406 as long as strict: true. Ot at least the documentation says so. Lets get to the bottom of this, try writing a test that reproduces it on Grape's HEAD?

@dblock dblock added the bug? label Jul 31, 2015
@elliotlarson
Copy link
Contributor Author

Okay, I created a pull request for this: #1101 At least, this seemed fairly straight forward to test and fix. It looks like if the accept header was a media type, it was failing to catch the missing version and return a 406.

@dm1try
Copy link
Member

dm1try commented Aug 20, 2015

Fixed in #1101.

@dm1try dm1try closed this as completed Aug 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants