From ad15b615e0a4bed657302f4382dc56d92dcdf98c Mon Sep 17 00:00:00 2001 From: Karl Freeman Date: Mon, 4 Nov 2013 16:48:59 +0000 Subject: [PATCH] Dry up options and headers logic, allow headers to be passed to OPTIONS requests - fixes quite a few edge cases surrounding middleware not being run --- CHANGELOG.md | 1 + lib/grape/api.rb | 36 ++++++++++++++++++------------------ spec/grape/api_spec.rb | 8 ++++++-- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2693ea97b2..95a1f95e1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Next Release * [#492](/~https://github.com/intridea/grape/pull/492): Don't allow to have nil value when a param is required and has a list of allowed values. - [@Antti](/~https://github.com/Antti) * [#495](/~https://github.com/intridea/grape/pull/495): Fix `ParamsScope#params` for parameters nested inside arrays - [@asross](/~https://github.com/asross). +* [#498](/~https://github.com/intridea/grape/pull/498): Dry up options and headers logic, allow headers to be passed to OPTIONS requests - [@karlfreeman](/~https://github.com/karlfreeman). 0.6.1 ===== diff --git a/lib/grape/api.rb b/lib/grape/api.rb index 343dd045f5..819ea89cbe 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -514,10 +514,10 @@ def inherit_settings(other_stack) def initialize @route_set = Rack::Mount::RouteSet.new + add_head_not_allowed_methods_and_options_methods self.class.endpoints.each do |endpoint| endpoint.mount_in(@route_set) end - add_head_not_allowed_methods @route_set.freeze end @@ -549,7 +549,7 @@ def cascade? # with a list of HTTP methods that can be called. Also add a route that # will return an HTTP 405 response for any HTTP method that the resource # cannot handle. - def add_head_not_allowed_methods + def add_head_not_allowed_methods_and_options_methods allowed_methods = Hash.new { |h, k| h[k] = [] } resources = self.class.endpoints.map do |endpoint| if endpoint.options[:app] && endpoint.options[:app].respond_to?(:endpoints) @@ -559,27 +559,27 @@ def add_head_not_allowed_methods end end resources.flatten.each do |route| - allowed_methods[route.route_compiled] << route.route_method + allowed_methods[route.route_path] << route.route_method end - allowed_methods.each do |path_info, methods| - if methods.include?('GET') && !methods.include?("HEAD") && !self.class.settings[:do_not_route_head] + allowed_methods.each do |path, methods| + if methods.include?('GET') && !methods.include?('HEAD') && !self.class.settings[:do_not_route_head] methods = methods | ['HEAD'] end - allow_header = (["OPTIONS"] | methods).join(", ") - unless methods.include?("OPTIONS") || self.class.settings[:do_not_route_options] - @route_set.add_route(proc { [204, { 'Allow' => allow_header }, []] }, { - path_info: path_info, - request_method: "OPTIONS" - }) + allow_header = (['OPTIONS'] | methods).join(', ') + if methods.include?('OPTIONS') || !self.class.settings[:do_not_route_options] + self.class.options(path, {}) { + header 'Allow', allow_header + status 204 + '' + } end not_allowed_methods = %w(GET PUT POST DELETE PATCH HEAD) - methods - not_allowed_methods << "OPTIONS" if self.class.settings[:do_not_route_options] - not_allowed_methods.each do |bad_method| - @route_set.add_route(proc { [405, { 'Allow' => allow_header, 'Content-Type' => 'text/plain' }, []] }, { - path_info: path_info, - request_method: bad_method - }) - end + not_allowed_methods << 'OPTIONS' if self.class.settings[:do_not_route_options] + self.class.route(not_allowed_methods, path) { + header 'Allow', allow_header + status 405 + '' + } end end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index e2f6edb475..a18a19e4fe 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -473,13 +473,15 @@ def subject.enable_root_route! last_response.body.should eql 'Created' end - it 'returns a 405 for an unsupported method' do + it 'returns a 405 for an unsupported method with an X-Custom-Header' do + subject.before { header 'X-Custom-Header', 'foo' } subject.get 'example' do "example" end put '/example' last_response.status.should eql 405 last_response.body.should eql '' + last_response.headers['X-Custom-Header'].should eql 'foo' end specify '405 responses includes an Allow header specifying supported methods' do @@ -504,7 +506,8 @@ def subject.enable_root_route! last_response.headers['Content-Type'].should eql 'text/plain' end - it 'adds an OPTIONS route that returns a 204 and an Allow header' do + it 'adds an OPTIONS route that returns a 204, an Allow header and a X-Custom-Header' do + subject.before { header 'X-Custom-Header', 'foo' } subject.get 'example' do "example" end @@ -512,6 +515,7 @@ def subject.enable_root_route! last_response.status.should eql 204 last_response.body.should eql '' last_response.headers['Allow'].should eql 'OPTIONS, GET, HEAD' + last_response.headers['X-Custom-Header'].should eql 'foo' end it 'allows HEAD on a GET request' do