From c983e7072f4d842b9dd26a28be7ae9b5810d94dc Mon Sep 17 00:00:00 2001 From: Nick Braga Date: Wed, 4 Apr 2018 19:12:11 -0400 Subject: [PATCH 1/4] Allow specifying redundant default port in origin When `http://EXAMPLE:80` is an allowed origin, requests are not allowed from `http://EXAMPLE`. Since port 80 is the default port for HTTP, browsers will strip it and thus rack-cors never receives a request from `http://EXAMPLE`. A similar problem is discussed here: https://github.com/request/request/issues/515 --- lib/rack/cors.rb | 2 +- test/unit/cors_test.rb | 7 +++++++ test/unit/test.ru | 4 +++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/rack/cors.rb b/lib/rack/cors.rb index 0a23106..131368a 100644 --- a/lib/rack/cors.rb +++ b/lib/rack/cors.rb @@ -278,8 +278,8 @@ def origins(*args, &blk) case n when Proc, Regexp, - /^https?:\/\//, 'file://' then n + when /^https?:\/\// then URI.parse(n).to_s when '*' then @public_resources = true; n else Regexp.compile("^[a-z][a-z0-9.+-]*:\\\/\\\/#{Regexp.quote(n)}$") end diff --git a/test/unit/cors_test.rb b/test/unit/cors_test.rb index f79c9fb..2e8987d 100644 --- a/test/unit/cors_test.rb +++ b/test/unit/cors_test.rb @@ -270,6 +270,13 @@ def load_app(name) cors_result.must_be :preflight end + it 'should allow HTTP/HTTPS origin without the default port' do + preflight_request('http://allow-the-default-port.io', '/') + last_response.must_render_cors_success + preflight_request('https://allow-the-default-port.io', '/') + last_response.must_render_cors_success + end + it 'should allow any header if headers = :any' do preflight_request('http://localhost:3000', '/', :headers => 'Fooey') last_response.must_render_cors_success diff --git a/test/unit/test.ru b/test/unit/test.ru index 5793de1..55b9377 100644 --- a/test/unit/test.ru +++ b/test/unit/test.ru @@ -8,7 +8,9 @@ use Rack::Cors do '127.0.0.1:3000', /http:\/\/192\.168\.0\.\d{1,3}(:\d+)?/, 'file://', - /http:\/\/(.*?)\.example\.com/ + /http:\/\/(.*?)\.example\.com/, + 'http://allow-the-default-port.io:80', + 'https://allow-the-default-port.io:443' resource '/get-only', :methods => :get resource '/', :headers => :any, :methods => :any From 4e536ffba15becf33b8f4390d932766f0e9e03db Mon Sep 17 00:00:00 2001 From: Nick Braga Date: Wed, 13 Jun 2018 13:52:07 -0400 Subject: [PATCH 2/4] Remove default port from http(s) without URI#parse --- lib/rack/cors.rb | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/rack/cors.rb b/lib/rack/cors.rb index 131368a..98f318c 100644 --- a/lib/rack/cors.rb +++ b/lib/rack/cors.rb @@ -8,6 +8,8 @@ class Cors HTTP_ACCESS_CONTROL_REQUEST_METHOD = 'HTTP_ACCESS_CONTROL_REQUEST_METHOD'.freeze HTTP_ACCESS_CONTROL_REQUEST_HEADERS = 'HTTP_ACCESS_CONTROL_REQUEST_HEADERS'.freeze + HTTP_REGEX = 'https?:\/\/[^:^\/]*(?:\d*)?.*?'.freeze + PATH_INFO = 'PATH_INFO'.freeze REQUEST_METHOD = 'REQUEST_METHOD'.freeze @@ -273,15 +275,24 @@ def initialize @public_resources = false end + def parse_http(uri) + match_data = /#{HTTP_REGEX}/.match(uri) + case match_data['port'] + when nil then uri + when ":#{URI.parse(uri).default_port}" then uri.sub(match_data['port'], '') + else uri + end + end + def origins(*args, &blk) @origins = args.flatten.reject{ |s| s == '' }.map do |n| case n when Proc, Regexp, - 'file://' then n - when /^https?:\/\// then URI.parse(n).to_s - when '*' then @public_resources = true; n - else Regexp.compile("^[a-z][a-z0-9.+-]*:\\\/\\\/#{Regexp.quote(n)}$") + 'file://' then n + when /#{HTTP_REGEX}/ then parse_http n + when '*' then @public_resources = true; n + else Regexp.compile("^[a-z][a-z0-9.+-]*:\\\/\\\/#{Regexp.quote(n)}$") end end.flatten @origins.push(blk) if blk From 40f3fed3aab086f7b9c0041a911e5223fa06d566 Mon Sep 17 00:00:00 2001 From: Nick Braga Date: Tue, 2 Aug 2022 13:26:28 -0400 Subject: [PATCH 3/4] revert code changes --- lib/rack/cors.rb | 19 ++++--------------- test/unit/cors_test.rb | 7 ------- test/unit/test.ru | 4 +--- 3 files changed, 5 insertions(+), 25 deletions(-) diff --git a/lib/rack/cors.rb b/lib/rack/cors.rb index 98f318c..0a23106 100644 --- a/lib/rack/cors.rb +++ b/lib/rack/cors.rb @@ -8,8 +8,6 @@ class Cors HTTP_ACCESS_CONTROL_REQUEST_METHOD = 'HTTP_ACCESS_CONTROL_REQUEST_METHOD'.freeze HTTP_ACCESS_CONTROL_REQUEST_HEADERS = 'HTTP_ACCESS_CONTROL_REQUEST_HEADERS'.freeze - HTTP_REGEX = 'https?:\/\/[^:^\/]*(?:\d*)?.*?'.freeze - PATH_INFO = 'PATH_INFO'.freeze REQUEST_METHOD = 'REQUEST_METHOD'.freeze @@ -275,24 +273,15 @@ def initialize @public_resources = false end - def parse_http(uri) - match_data = /#{HTTP_REGEX}/.match(uri) - case match_data['port'] - when nil then uri - when ":#{URI.parse(uri).default_port}" then uri.sub(match_data['port'], '') - else uri - end - end - def origins(*args, &blk) @origins = args.flatten.reject{ |s| s == '' }.map do |n| case n when Proc, Regexp, - 'file://' then n - when /#{HTTP_REGEX}/ then parse_http n - when '*' then @public_resources = true; n - else Regexp.compile("^[a-z][a-z0-9.+-]*:\\\/\\\/#{Regexp.quote(n)}$") + /^https?:\/\//, + 'file://' then n + when '*' then @public_resources = true; n + else Regexp.compile("^[a-z][a-z0-9.+-]*:\\\/\\\/#{Regexp.quote(n)}$") end end.flatten @origins.push(blk) if blk diff --git a/test/unit/cors_test.rb b/test/unit/cors_test.rb index 2e8987d..f79c9fb 100644 --- a/test/unit/cors_test.rb +++ b/test/unit/cors_test.rb @@ -270,13 +270,6 @@ def load_app(name) cors_result.must_be :preflight end - it 'should allow HTTP/HTTPS origin without the default port' do - preflight_request('http://allow-the-default-port.io', '/') - last_response.must_render_cors_success - preflight_request('https://allow-the-default-port.io', '/') - last_response.must_render_cors_success - end - it 'should allow any header if headers = :any' do preflight_request('http://localhost:3000', '/', :headers => 'Fooey') last_response.must_render_cors_success diff --git a/test/unit/test.ru b/test/unit/test.ru index 55b9377..5793de1 100644 --- a/test/unit/test.ru +++ b/test/unit/test.ru @@ -8,9 +8,7 @@ use Rack::Cors do '127.0.0.1:3000', /http:\/\/192\.168\.0\.\d{1,3}(:\d+)?/, 'file://', - /http:\/\/(.*?)\.example\.com/, - 'http://allow-the-default-port.io:80', - 'https://allow-the-default-port.io:443' + /http:\/\/(.*?)\.example\.com/ resource '/get-only', :methods => :get resource '/', :headers => :any, :methods => :any From 897cd5fe08b53c0b4574c65d376c461b1686c6e7 Mon Sep 17 00:00:00 2001 From: Nick Braga Date: Wed, 3 Aug 2022 14:35:03 -0400 Subject: [PATCH 4/4] add note on origins w/ default port to README.md --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 56b9204..7767e20 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,9 @@ A Resource path can be specified as exact string match (`/path/to/file.txt`) or ### Origin Matching -When specifying an origin, make sure that it does not have a trailing slash. +* When specifying an origin, make sure that it does not have a trailing slash. + +* When specifying an HTTP origin that uses the scheme's default port (e.g. `http://example.test:80`), some clients may not strip the port which could result in unexpected blocked requests (additional context [here](https://github.com/request/request/pull/2904)). ### Testing Postman and/or cURL