From 8e9cff7523e40a1b29ae11aa663473859203425d Mon Sep 17 00:00:00 2001 From: Jason Desrosiers Date: Wed, 28 Jun 2017 22:57:12 -0700 Subject: [PATCH] Calculate the Allow header from routes The Allow header is also used to determine the Access-Control-Allow-Methods header value. --- README.md | 1 - lib/sinatra/cors.rb | 35 +++++++++++++++++++++++------------ sinatra-cors.gemspec | 2 +- spec/cors_spec.rb | 31 +++++++++++++++++++++---------- spec/fixture.rb | 10 +++++++++- 5 files changed, 54 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index f1c1442..e01773f 100644 --- a/README.md +++ b/README.md @@ -42,5 +42,4 @@ Settings Comming Soon ------------ -* Automatically determine `Allow` headers based on existing routes * Route specific settings diff --git a/lib/sinatra/cors.rb b/lib/sinatra/cors.rb index c3f968f..8234fa3 100644 --- a/lib/sinatra/cors.rb +++ b/lib/sinatra/cors.rb @@ -10,6 +10,7 @@ def cors logger.warn bad_method_message return end + unless headers_are_allowed? logger.warn bad_headers_message return @@ -41,16 +42,15 @@ def is_preflight_request? end def method_is_allowed? - allow_methods = settings.allow_methods || response.headers["Allow"] || "" - request_method = request.env["HTTP_ACCESS_CONTROL_REQUEST_METHOD"] || "" - allow_methods.split.include? request_method + allow_methods = settings.allow_methods.split & response.headers["Allow"].split + request_method = request.env["HTTP_ACCESS_CONTROL_REQUEST_METHOD"] + allow_methods.include? request_method end def headers_are_allowed? - allow_headers = settings.allow_headers || "" + allow_headers = settings.allow_headers request_headers = request.env["HTTP_ACCESS_CONTROL_REQUEST_HEADERS"] || "" - diff = request_headers.split - allow_headers.split - diff.size == 0 + (request_headers.split - allow_headers.split).empty? end def origin_is_allowed? @@ -81,19 +81,30 @@ def bad_origin_message def self.registered(app) app.helpers Cors::Helpers - app.disable :allow_origin - app.disable :allow_methods - app.disable :allow_headers + app.set :allow_origin, "" + app.set :allow_methods, "" + app.set :allow_headers, "" app.disable :max_age app.disable :expose_headers app.disable :allow_credentials - app.set(:is_cors_preflight) { |bool| + app.set(:is_cors_preflight) do |bool| condition { is_cors_request? && is_preflight_request? == bool } - } + end app.options "*", is_cors_preflight: true do - response.headers["Allow"] = settings.allow_methods || "" + matches = [] + settings.routes.each do |method, routes| + routes.each do |route| + process_route(route[0], route[1], route[2]) do |application, pattern| + matches << method + end + end + end + + pass if matches.size == 1 + + response.headers["Allow"] = matches.join " " end app.after do diff --git a/sinatra-cors.gemspec b/sinatra-cors.gemspec index a1c3c78..60ed508 100644 --- a/sinatra-cors.gemspec +++ b/sinatra-cors.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = "sinatra-cors" - s.version = "0.1.1" + s.version = "0.2.0" s.date = "2017-06-28" s.summary = "CORS support for Sinatra applications" s.description = <<-EOT diff --git a/spec/cors_spec.rb b/spec/cors_spec.rb index 1e5095d..432d275 100644 --- a/spec/cors_spec.rb +++ b/spec/cors_spec.rb @@ -13,7 +13,18 @@ def app describe "A non-CORS OPTIONS request" do it "should not be handled" do - options "/foo" + options "/foo/1" + expect(last_response).to be_not_found + end + end + + describe "A CORS preflight request for a route that doesn't exist" do + it "should 404" do + rack_env = { + "HTTP_ORIGIN" => "http://example.com", + "HTTP_ACCESS_CONTROL_REQUEST_METHOD" => "GET", + } + options "/baz/1", {}, rack_env expect(last_response).to be_not_found end end @@ -24,7 +35,7 @@ def app "HTTP_ORIGIN" => "http://example.com", "HTTP_ACCESS_CONTROL_REQUEST_METHOD" => "DELETE", } - options "/foo", {}, rack_env + options "/foo/1", {}, rack_env end it "should not return an Access-Control-Allow-Origin header" do @@ -43,15 +54,15 @@ def app "HTTP_ACCESS_CONTROL_REQUEST_METHOD" => "GET", "HTTP_ACCESS_CONTROL_REQUEST_HEADERS" => "if-modified-since" } - options "/foo", {}, rack_env + options "/foo/1", {}, rack_env end it "should be handled for all routes" do expect(last_response).to be_ok end - it "should have an allow header that matches the :allow_methods setting" do - expect(last_response["Allow"]).to eq("GET HEAD POST") + it "should have an Allow header build from existing routes" do + expect(last_response["Allow"]).to eq("GET HEAD DELETE OPTIONS") end it "should have an Access-Control-Allow-Methods header that includes only the method requested" do @@ -79,7 +90,7 @@ def app "HTTP_ORIGIN" => "http://example.com", "HTTP_ACCESS_CONTROL_REQUEST_METHOD" => "GET", } - options "/foo", {}, rack_env + options "/foo/1", {}, rack_env expect(last_response["Access-Control-Max-Age"]).to eq("600") end @@ -90,7 +101,7 @@ def app rack_env = { "HTTP_ORIGIN" => "http://example.com", } - get "/foo", {}, rack_env + get "/foo/1", {}, rack_env end it "should have an Access-Control-Allow-Origin header that includes only the origin of the request" do @@ -111,7 +122,7 @@ def make_request(allow_origin) rack_env = { "HTTP_ORIGIN" => "http://example.com", } - get "/foo", {}, rack_env + get "/foo/1", {}, rack_env end it "should be 'null' if the origin is not allowed" do @@ -140,7 +151,7 @@ def make_request(allow_origin) rack_env = { "HTTP_ORIGIN" => "http://example.com", } - get "/foo", {}, rack_env + get "/foo/1", {}, rack_env expect(last_response["Access-Control-Allow-Credentials"]).to eq("true") end @@ -156,7 +167,7 @@ def make_request(allow_origin) rack_env = { "HTTP_ORIGIN" => "http://example.com", } - get "/foo", {}, rack_env + get "/foo/1", {}, rack_env expect(last_response["Access-Control-Expose-Headers"]).to eq("location link") end diff --git a/spec/fixture.rb b/spec/fixture.rb index f2a0b24..f2e39d9 100644 --- a/spec/fixture.rb +++ b/spec/fixture.rb @@ -1,10 +1,18 @@ require "sinatra" require "./lib/sinatra/cors" -get "/foo" do +get "/foo/:id" do "foo" end +delete "/foo/:id" do + "foo" +end + +post "/bar/:id" do + "bar" +end + register Sinatra::Cors set :allow_origin, "http://example.com http://foo.com"