Inconsistent "invalid header value" response codes #464
Labels
Comments
|
Do you think you could build an RSpec test that reproduces this? Should definitely not return a 500. |
|
This reproduces the problem. I'll look into whether I can fix it. context 'version headers' do
before do
subject.version 'v1', using: :header, vendor: 'ohanapi'
subject.get '/test' do
"Hello!"
end
end
it 'should result in a 406 response if they are invalid' do
get '/test', {}, 'HTTP_ACCEPT' => 'application/vnd.ohanapi.v1+json'
last_response.status.should == 406
end
it 'should result in a 406 response if they are invalid (2)' do
get '/test', {}, 'HTTP_ACCEPT' => 'application/vnd.ohanapi.v1+json; version=1'
last_response.status.should == 406
end
end |
|
Fixing the '500' problem is trivial enough, but also results in a 404. there's a comment in the relevant code mentioning X-Cascade so that Rack::Mount attempts other routes. Not sure if that's what's resulting in the 404 instead of the intended 406. --- i/lib/grape/middleware/versioner/header.rb
+++ w/lib/grape/middleware/versioner/header.rb
@@ -23,7 +23,11 @@ module Grape
# route.
class Header < Base
def before
- header = Rack::Accept::MediaType.new env['HTTP_ACCEPT']
+ begin
+ header = Rack::Accept::MediaType.new env['HTTP_ACCEPT']
+ rescue Exception => e
+ throw :error, status: 406, headers: error_headers, message: e.to_s
+ end
if strict?
# If no Accept header: |
|
Indeed - setting the |
bwalex
added a commit
to bwalex/grape
that referenced
this issue
Jan 3, 2014
* Catch the exception(s) thrown by rack-accept and return a 406 if one is caught. * Add a spec to cover this case.
bwalex
added a commit
to bwalex/grape
that referenced
this issue
Jan 3, 2014
* Catch the exception(s) thrown by rack-accept and return a 406 if one is caught. * Add a spec to cover this case.
bwalex
added a commit
to bwalex/grape
that referenced
this issue
Jan 3, 2014
* Catch the exception(s) thrown by rack-accept and return a 406 if one is caught. * Add a spec to cover this case.
dblock
added a commit
that referenced
this issue
Jan 3, 2014
Fix for #464: gracefully handle invalid version headers
|
Closing, fixed in 0.7.0. |
salimane
added a commit
to salimane/grape
that referenced
this issue
Jan 8, 2014
* upstream/master: add posibility to define reusable named params Added ability to restrict declared(params) to the local endpoint with include_parent_namespaces: false. Fix for ruby-grape#464: gracefully handle invalid version headers Rename exception classes to match README. Define errors in context/before blocks. Updated/renamed UPGRADING. README.md and UPGRADE.md update for ruby-grape#543, ruby-grape#545 Address ruby-grape#543 - raise proper validation errors on array/hash types Remove unnecessary test case. Rename children --> subclasses to match the name used in the code. Fix typo in rescue_from unit test (Communication --> Communications). Rescue subclasses in error middleware. Fix for travis-ci/travis-ci#1800. Added Ruby 2.1 support. Lock RSpec version, failing with rspec-expectations 3.x beta.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi,
After searching through some older issues, it looks like it was agreed that an invalid header value would return a 406 error. Is that still the case? I'm getting mixed responses. Sometimes I get a 500, sometimes a 404.
404:
500:
Here's the full backtrace:
The text was updated successfully, but these errors were encountered: