Skip to content

Commit

Permalink
Fixed that invalid accept headers are (1) not processed by rescue han…
Browse files Browse the repository at this point in the history
…dlers and (2) result in status 500 (no method) when http_codes are defined.
  • Loading branch information
croeck committed Jan 30, 2015
1 parent 4704cc5 commit f5ea8a0
Show file tree
Hide file tree
Showing 8 changed files with 432 additions and 106 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Next Release
* [#901](https://github.com/intridea/grape/pull/901): Fix: callbacks defined in a version block are only called for the routes defined in that block - [@kushkella](https://github.com/kushkella).
* [#886](https://github.com/intridea/grape/pull/886): Group of parameters made to require an explicit type of Hash or Array - [@jrichter1](https://github.com/jrichter1).
* [#912](https://github.com/intridea/grape/pull/912): Extended the `:using` feature for param documentation to `optional` fields - [@croeck](https://github.com/croeck).
* [#913](https://github.com/intridea/grape/pull/913): Fix: Invalid accept headers are (1) not processed by rescue handlers and (2) result in 500 when http_codes are defines - [@croeck](https://github.com/croeck).
* Your contribution here.

0.10.1 (12/28/2014)
Expand Down
1 change: 1 addition & 0 deletions lib/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ module Exceptions
autoload :IncompatibleOptionValues, 'grape/exceptions/incompatible_option_values'
autoload :MissingGroupTypeError, 'grape/exceptions/missing_group_type'
autoload :UnsupportedGroupTypeError, 'grape/exceptions/unsupported_group_type'
autoload :InvalidAcceptHeader, 'grape/exceptions/invalid_accept_header'
end

module ErrorFormatter
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/error_formatter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def present(message, env)
http_codes = env['rack.routing_args'][:route_info].route_http_codes || []
found_code = http_codes.find do |http_code|
(http_code[0].to_i == env['api.endpoint'].status) && http_code[2].respond_to?(:represent)
end
end unless env['api.endpoint'].request.nil?

presenter = found_code[2] if found_code
end
Expand Down
10 changes: 10 additions & 0 deletions lib/grape/exceptions/invalid_accept_header.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# encoding: utf-8
module Grape
module Exceptions
class InvalidAcceptHeader < Base
def initialize(message, headers)
super(message: compose_message('invalid_accept_header', message: message), status: 406, headers: headers)
end
end
end
end
3 changes: 3 additions & 0 deletions lib/grape/locale/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,7 @@ en:
all_or_none: 'provide all or none of parameters'
missing_group_type: 'group type is required'
unsupported_group_type: 'group type must be Array or Hash'
invalid_accept_header:
problem: 'Invalid accept header'
resolution: '%{message}'

21 changes: 12 additions & 9 deletions lib/grape/middleware/versioner/header.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,21 @@ module Versioner
# route.
class Header < Base
def before
begin
header = Rack::Accept::MediaType.new env['HTTP_ACCEPT']
rescue RuntimeError => e
throw :error, status: 406, headers: error_headers, message: e.message
end
header = rack_accept_header

if strict?
# If no Accept header:
if header.qvalues.empty?
throw :error, status: 406, headers: error_headers, message: 'Accept header must be set.'
fail Grape::Exceptions::InvalidAcceptHeader.new('Accept header must be set.', error_headers)
end
# Remove any acceptable content types with ranges.
header.qvalues.reject! do |media_type, _|
Rack::Accept::Header.parse_media_type(media_type).find { |s| s == '*' }
end
# If all Accept headers included a range:
if header.qvalues.empty?
throw :error, status: 406, headers: error_headers, message: 'Accept header must not contain ranges ("*").'
fail Grape::Exceptions::InvalidAcceptHeader.new('Accept header must not contain ranges ("*").',
error_headers)
end
end

Expand All @@ -58,10 +55,10 @@ def before
end
# If none of the available content types are acceptable:
elsif strict?
throw :error, status: 406, headers: error_headers, message: '406 Not Acceptable'
fail Grape::Exceptions::InvalidAcceptHeader.new('406 Not Acceptable', error_headers)
# If all acceptable content types specify a vendor or version that doesn't exist:
elsif header.values.all? { |header_value| has_vendor?(header_value) || version?(header_value) }
throw :error, status: 406, headers: error_headers, message: 'API vendor or version not found.'
fail Grape::Exceptions::InvalidAcceptHeader.new('API vendor or version not found.', error_headers)
end
end

Expand All @@ -86,6 +83,12 @@ def available_media_types
available_media_types = available_media_types.flatten
end

def rack_accept_header
Rack::Accept::MediaType.new env['HTTP_ACCEPT']
rescue RuntimeError => e
raise Grape::Exceptions::InvalidAcceptHeader.new(e.message, error_headers)
end

def versions
options[:versions] || []
end
Expand Down
Loading

0 comments on commit f5ea8a0

Please sign in to comment.