diff --git a/CHANGELOG.md b/CHANGELOG.md index bd1ed4cda5..e76cafac92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ * [#1052](https://github.com/ruby-grape/grape/pull/1052): Reset `description[:params]` when resetting validations - [@marshall-lee](https://github.com/marshall-lee). * [#1088](https://github.com/ruby-grape/grape/pull/1088): Support ActiveSupport 3.x by explicitly requiring `Hash#except` - [@wagenet](https://github.com/wagenet). * [#1096](https://github.com/ruby-grape/grape/pull/1096): Fix coercion on booleans - [@towanda](https://github.com/towanda). +* [#1101](https://github.com/intridea/grape/pull/1101): With strict version header, 406 response when using media type accept header - [@elliotlarson](https://github.com/elliotlarson). 0.12.0 (6/18/2015) ================== diff --git a/lib/grape/middleware/versioner/header.rb b/lib/grape/middleware/versioner/header.rb index 1da5675ea9..535b146fc3 100644 --- a/lib/grape/middleware/versioner/header.rb +++ b/lib/grape/middleware/versioner/header.rb @@ -22,54 +22,82 @@ module Versioner # X-Cascade header to alert Rack::Mount to attempt the next matched # route. class Header < Base - def before - header = rack_accept_header + VENDOR_VERSION_HEADER_REGEX = + /\Avnd\.([a-z0-9*.]+)(?:-([a-z0-9*\-.]+))?(?:\+([a-z0-9*\-.+]+))?\z/ - if strict? - # If no Accept header: - if header.qvalues.empty? - 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? - fail Grape::Exceptions::InvalidAcceptHeader.new('Accept header must not contain ranges ("*").', - error_headers) - end - end - - media_type = header.best_of available_media_types + def before + strict_header_checks if strict? if media_type - type, subtype = Rack::Accept::Header.parse_media_type media_type - env['api.type'] = type - env['api.subtype'] = subtype - - if /\Avnd\.([a-z0-9*.]+)(?:-([a-z0-9*\-.]+))?(?:\+([a-z0-9*\-.+]+))?\z/ =~ subtype - env['api.vendor'] = Regexp.last_match[1] - env['api.version'] = Regexp.last_match[2] - env['api.format'] = Regexp.last_match[3] # weird that Grape::Middleware::Formatter also does this - end + media_type_header_handler # If none of the available content types are acceptable: elsif strict? - 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) } - fail Grape::Exceptions::InvalidAcceptHeader.new('API vendor or version not found.', error_headers) + fail_with_invalid_accept_header!('406 Not Acceptable') + elsif headers_contain_wrong_vendor_or_version? + fail_with_invalid_accept_header!('API vendor or version not found.') end end private + def strict_header_checks + # If no Accept header: + if header.qvalues.empty? + fail_with_invalid_accept_header!('Accept header must be set.') + end + # Remove any acceptable content types with ranges. + header.qvalues.reject! do |media_type, _| + Rack::Accept::Header.parse_media_type(media_type).find do |s| + s == '*' + end + end + # If all Accept headers included a range: + if header.qvalues.empty? + fail_with_invalid_accept_header!( + 'Accept header must not contain ranges ("*").' + ) + end + end + + def header + @header ||= rack_accept_header + end + + def media_type + @media_type ||= header.best_of(available_media_types) + end + + def media_type_header_handler + type, subtype = Rack::Accept::Header.parse_media_type media_type + env['api.type'] = type + env['api.subtype'] = subtype + + if VENDOR_VERSION_HEADER_REGEX =~ subtype + env['api.vendor'] = Regexp.last_match[1] + env['api.version'] = Regexp.last_match[2] + # weird that Grape::Middleware::Formatter also does this + env['api.format'] = Regexp.last_match[3] + elsif versions.size > 0 + fail_with_invalid_accept_header!( + 'API vendor or version not found.' + ) + end + end + + def fail_with_invalid_accept_header!(message) + fail Grape::Exceptions::InvalidAcceptHeader + .new(message, error_headers) + end + def available_media_types available_media_types = [] content_types.each do |extension, _media_type| versions.reverse_each do |version| - available_media_types += ["application/vnd.#{vendor}-#{version}+#{extension}", "application/vnd.#{vendor}-#{version}"] + available_media_types += [ + "application/vnd.#{vendor}-#{version}+#{extension}", + "application/vnd.#{vendor}-#{version}" + ] end available_media_types << "application/vnd.#{vendor}+#{extension}" end @@ -83,10 +111,16 @@ def available_media_types available_media_types.flatten end + def headers_contain_wrong_vendor_or_version? + header.values.all? do |header_value| + has_vendor?(header_value) || version?(header_value) + end + end + def rack_accept_header Rack::Accept::MediaType.new env[Grape::Http::Headers::HTTP_ACCEPT] rescue RuntimeError => e - raise Grape::Exceptions::InvalidAcceptHeader.new(e.message, error_headers) + fail_with_invalid_accept_header!(e.message) end def versions @@ -94,19 +128,25 @@ def versions end def vendor - options[:version_options] && options[:version_options][:vendor] + version_options && version_options[:vendor] end def strict? - options[:version_options] && options[:version_options][:strict] + version_options && version_options[:strict] + end + + def version_options + options[:version_options] end - # By default those errors contain an `X-Cascade` header set to `pass`, which allows nesting and stacking - # of routes (see [Rack::Mount](https://github.com/josh/rack-mount) for more information). To prevent - # this behavior, and not add the `X-Cascade` header, one can set the `:cascade` option to `false`. + # By default those errors contain an `X-Cascade` header set to `pass`, + # which allows nesting and stacking of routes + # (see [Rack::Mount](https://github.com/josh/rack-mount) for more + # information). To prevent # this behavior, and not add the `X-Cascade` + # header, one can set the `:cascade` option to `false`. def cascade? - if options[:version_options] && options[:version_options].key?(:cascade) - !!options[:version_options][:cascade] + if version_options && version_options.key?(:cascade) + !!version_options[:cascade] else true end @@ -119,14 +159,14 @@ def error_headers # @param [String] media_type a content type # @return [Boolean] whether the content type sets a vendor def has_vendor?(media_type) - _, subtype = Rack::Accept::Header.parse_media_type media_type + _, subtype = Rack::Accept::Header.parse_media_type(media_type) subtype[/\Avnd\.[a-z0-9*.]+/] end # @param [String] media_type a content type # @return [Boolean] whether the content type sets an API version def version?(media_type) - _, subtype = Rack::Accept::Header.parse_media_type media_type + _, subtype = Rack::Accept::Header.parse_media_type(media_type) subtype[/\Avnd\.[a-z0-9*.]+-[a-z0-9*\-.]+/] end end diff --git a/spec/grape/middleware/versioner/header_spec.rb b/spec/grape/middleware/versioner/header_spec.rb index c3a23c6f40..d047c79f31 100644 --- a/spec/grape/middleware/versioner/header_spec.rb +++ b/spec/grape/middleware/versioner/header_spec.rb @@ -223,6 +223,15 @@ end end + it 'fails with 406 Not Acceptable if header is application/xml' do + expect { subject.call('HTTP_ACCEPT' => 'application/xml').last }.to raise_exception do |exception| + expect(exception).to be_a(Grape::Exceptions::InvalidAcceptHeader) + expect(exception.headers).to eql({}) + expect(exception.status).to eql 406 + expect(exception.message).to include('API vendor or version not found.') + end + end + it 'fails with 406 Not Acceptable if header is empty' do expect { subject.call('HTTP_ACCEPT' => '').last }.to raise_exception do |exception| expect(exception).to be_a(Grape::Exceptions::InvalidAcceptHeader)