From 4339137d2ae737febd819e232c58fabc212925ba Mon Sep 17 00:00:00 2001 From: Chris Waters Date: Tue, 6 Sep 2022 11:28:19 +0100 Subject: [PATCH 1/2] cookie paths not applicable when setting --- lib/rack/test/cookie_jar.rb | 31 ++++++++++++++++++---------- spec/fixtures/fake_app.rb | 4 ++++ spec/rack/test/cookie_object_spec.rb | 20 +++++++++++++----- spec/rack/test/cookie_spec.rb | 6 ++++++ 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/lib/rack/test/cookie_jar.rb b/lib/rack/test/cookie_jar.rb index 2730ddc..a857fc3 100644 --- a/lib/rack/test/cookie_jar.rb +++ b/lib/rack/test/cookie_jar.rb @@ -86,21 +86,19 @@ def expired? expires && expires < Time.now end - # Whether the cookie is valid for the given URI. - def valid?(uri) - uri ||= default_uri - - uri.host = @default_host if uri.host.nil? + # Whether the cookie is valid to set from the given URI. + def valid_set?(uri) + valid?(uri) + end - real_domain = domain =~ /^\./ ? domain[1..-1] : domain - !!((!secure? || (secure? && uri.scheme == 'https')) && - uri.host =~ Regexp.new("#{'^' if @exact_domain_match}#{Regexp.escape(real_domain)}$", Regexp::IGNORECASE) && - uri.path =~ Regexp.new("^#{Regexp.escape(path)}")) + # Whether the cookie is valid to send to the given URI. + def valid_send?(uri) + valid?(uri) && !!(uri.path =~ Regexp.new("^#{Regexp.escape(path)}")) end # Cookies that do not match the URI will not be sent in requests to the URI. def matches?(uri) - !expired? && valid?(uri) + !expired? && valid_send?(uri) end # Order cookies by name, path, and domain. @@ -120,6 +118,17 @@ def to_h private + # Whether the cookie is valid for the given URI. + def valid?(uri) + uri ||= default_uri + + uri.host = @default_host if uri.host.nil? + + real_domain = domain =~ /^\./ ? domain[1..-1] : domain + !!((!secure? || (secure? && uri.scheme == 'https')) && + uri.host =~ Regexp.new("#{'^' if @exact_domain_match}#{Regexp.escape(real_domain)}$", Regexp::IGNORECASE)) + end + # The default URI to use for the cookie, including just the host. def default_uri URI.parse('//' + @default_host + '/') @@ -190,7 +199,7 @@ def merge(raw_cookies, uri = nil) raw_cookies.each do |raw_cookie| cookie = Cookie.new(raw_cookie, uri, @default_host) - self << cookie if cookie.valid?(uri) + self << cookie if cookie.valid_set?(uri) end end diff --git a/spec/fixtures/fake_app.rb b/spec/fixtures/fake_app.rb index d8b986a..3cfeff2 100644 --- a/spec/fixtures/fake_app.rb +++ b/spec/fixtures/fake_app.rb @@ -60,6 +60,10 @@ def handle(env) end end + if path == '/redirect-with-cookie' && method == 'GET' + return [302, { 'set-cookie' => "value=1; path=/cookies;", 'location' => '/cookies/show' }, []] + end + if path == '/redirected' additional_info = if method == 'GET' ", session #{session.inspect} with options #{env['rack.session.options'].inspect}" diff --git a/spec/rack/test/cookie_object_spec.rb b/spec/rack/test/cookie_object_spec.rb index 98f5a54..7a226c0 100644 --- a/spec/rack/test/cookie_object_spec.rb +++ b/spec/rack/test/cookie_object_spec.rb @@ -6,7 +6,7 @@ describe Rack::Test::Cookie do value = 'the cookie value'.freeze domain = 'www.example.org'.freeze - path = '/'.freeze + path = '/foo'.freeze expires = 'Mon, 10 Aug 2015 14:40:57 0100'.freeze cookie_string = [ 'cookie_name=' + CGI.escape(value), @@ -39,10 +39,20 @@ Rack::Test::Cookie.new('value=').empty?.must_equal true end - it '#valid? should consider the given URI scheme for secure cookies' do - cookie('; secure').valid?(URI.parse('https://www.example.org/')).must_equal true - cookie('; secure').valid?(URI.parse('httpx://www.example.org/')).must_equal false - cookie('; secure').valid?(URI.parse('/')).must_equal false + it '#valid_set? should consider the given URI scheme for secure cookies' do + cookie('; secure').valid_set?(URI.parse('https://www.example.org/')).must_equal true + cookie('; secure').valid_set?(URI.parse('httpx://www.example.org/')).must_equal false + cookie('; secure').valid_set?(URI.parse('/')).must_equal false + end + + it '#valid_set? is indifferent to matching paths' do + cookie.valid_set?(URI.parse('https://www.example.org/foo')).must_equal true + cookie.valid_set?(URI.parse('https://www.example.org/bar')).must_equal true + end + + it '#valid_send? demands matching paths' do + cookie.valid_send?(URI.parse('https://www.example.org/foo')).must_equal true + cookie.valid_send?(URI.parse('https://www.example.org/bar')).must_equal false end it '#http_only? for a non HTTP only cookie returns false' do diff --git a/spec/rack/test/cookie_spec.rb b/spec/rack/test/cookie_spec.rb index 1595004..f88c431 100644 --- a/spec/rack/test/cookie_spec.rb +++ b/spec/rack/test/cookie_spec.rb @@ -258,4 +258,10 @@ def cookie.expired?; true end request '/cookies/show', cookie: 'value=1' last_request.cookies.must_equal 'value' => '1' end + + it 'sets and subsequently sends cookies when redirecting to the path of the cookie' do + get '/redirect-with-cookie' + follow_redirect! + last_request.cookies.must_equal 'value' => '1' + end end From 02fec341967e619dcb23081a254b6fdc4ee952eb Mon Sep 17 00:00:00 2001 From: Chris Waters Date: Wed, 7 Sep 2022 19:37:31 +0100 Subject: [PATCH 2/2] match path in #matches? rather than introduce new methods --- lib/rack/test/cookie_jar.rb | 31 ++++++++++++---------------- spec/rack/test/cookie_object_spec.rb | 22 ++++++++++---------- 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/lib/rack/test/cookie_jar.rb b/lib/rack/test/cookie_jar.rb index a857fc3..b3f53d2 100644 --- a/lib/rack/test/cookie_jar.rb +++ b/lib/rack/test/cookie_jar.rb @@ -86,19 +86,20 @@ def expired? expires && expires < Time.now end - # Whether the cookie is valid to set from the given URI. - def valid_set?(uri) - valid?(uri) - end + # Whether the cookie is valid for the given URI. + def valid?(uri) + uri ||= default_uri - # Whether the cookie is valid to send to the given URI. - def valid_send?(uri) - valid?(uri) && !!(uri.path =~ Regexp.new("^#{Regexp.escape(path)}")) + uri.host = @default_host if uri.host.nil? + + real_domain = domain =~ /^\./ ? domain[1..-1] : domain + !!((!secure? || (secure? && uri.scheme == 'https')) && + uri.host =~ Regexp.new("#{'^' if @exact_domain_match}#{Regexp.escape(real_domain)}$", Regexp::IGNORECASE)) end # Cookies that do not match the URI will not be sent in requests to the URI. def matches?(uri) - !expired? && valid_send?(uri) + !expired? && valid?(uri) && valid_path?(uri.path) end # Order cookies by name, path, and domain. @@ -118,15 +119,9 @@ def to_h private - # Whether the cookie is valid for the given URI. - def valid?(uri) - uri ||= default_uri - - uri.host = @default_host if uri.host.nil? - - real_domain = domain =~ /^\./ ? domain[1..-1] : domain - !!((!secure? || (secure? && uri.scheme == 'https')) && - uri.host =~ Regexp.new("#{'^' if @exact_domain_match}#{Regexp.escape(real_domain)}$", Regexp::IGNORECASE)) + # Whether the path of URI matches the Cookie path + def valid_path?(uri_path) + !!(uri_path =~ Regexp.new("^#{Regexp.escape(path)}")) end # The default URI to use for the cookie, including just the host. @@ -199,7 +194,7 @@ def merge(raw_cookies, uri = nil) raw_cookies.each do |raw_cookie| cookie = Cookie.new(raw_cookie, uri, @default_host) - self << cookie if cookie.valid_set?(uri) + self << cookie if cookie.valid?(uri) end end diff --git a/spec/rack/test/cookie_object_spec.rb b/spec/rack/test/cookie_object_spec.rb index 7a226c0..25852cf 100644 --- a/spec/rack/test/cookie_object_spec.rb +++ b/spec/rack/test/cookie_object_spec.rb @@ -7,7 +7,7 @@ value = 'the cookie value'.freeze domain = 'www.example.org'.freeze path = '/foo'.freeze - expires = 'Mon, 10 Aug 2015 14:40:57 0100'.freeze + expires = (Time.now + (24 * 60 * 60)).httpdate cookie_string = [ 'cookie_name=' + CGI.escape(value), 'domain=' + domain, @@ -39,20 +39,20 @@ Rack::Test::Cookie.new('value=').empty?.must_equal true end - it '#valid_set? should consider the given URI scheme for secure cookies' do - cookie('; secure').valid_set?(URI.parse('https://www.example.org/')).must_equal true - cookie('; secure').valid_set?(URI.parse('httpx://www.example.org/')).must_equal false - cookie('; secure').valid_set?(URI.parse('/')).must_equal false + it '#valid? should consider the given URI scheme for secure cookies' do + cookie('; secure').valid?(URI.parse('https://www.example.org/')).must_equal true + cookie('; secure').valid?(URI.parse('httpx://www.example.org/')).must_equal false + cookie('; secure').valid?(URI.parse('/')).must_equal false end - it '#valid_set? is indifferent to matching paths' do - cookie.valid_set?(URI.parse('https://www.example.org/foo')).must_equal true - cookie.valid_set?(URI.parse('https://www.example.org/bar')).must_equal true + it '#valid? is indifferent to matching paths' do + cookie.valid?(URI.parse('https://www.example.org/foo')).must_equal true + cookie.valid?(URI.parse('https://www.example.org/bar')).must_equal true end - it '#valid_send? demands matching paths' do - cookie.valid_send?(URI.parse('https://www.example.org/foo')).must_equal true - cookie.valid_send?(URI.parse('https://www.example.org/bar')).must_equal false + it '#matches? demands matching paths' do + cookie.matches?(URI.parse('https://www.example.org/foo')).must_equal true + cookie.matches?(URI.parse('https://www.example.org/bar')).must_equal false end it '#http_only? for a non HTTP only cookie returns false' do