diff --git a/lib/http/cookie.rb b/lib/http/cookie.rb index a688530..8be45be 100644 --- a/lib/http/cookie.rb +++ b/lib/http/cookie.rb @@ -313,14 +313,9 @@ class HTTP::Cookie end } - if origin - begin - cookie.origin = origin - rescue => e - logger.warn("Invalid cookie for the origin: #{origin} (#{e})") if logger - next - end - end + cookie.origin = origin + + cookie.acceptable? or next yield cookie if block_given? @@ -422,10 +417,10 @@ class HTTP::Cookie @origin.nil? or raise ArgumentError, "origin cannot be changed once it is set" origin = URI(origin) - self.domain ||= origin.host - self.path ||= (origin + './').path - acceptable_from_uri?(origin) or - raise ArgumentError, "unacceptable cookie sent from URI #{origin}" + if URI::HTTP === origin + self.domain ||= origin.host + self.path ||= (origin + './').path + end @origin = origin end @@ -534,6 +529,22 @@ class HTTP::Cookie end end + # Tests if it is OK to accept this cookie considering its origin. + # If either domain or path is missing, raises ArgumentError. If + # origin is missing, returns true. + def acceptable? + case + when @domain.nil? + raise ArgumentError, "domain is missing" + when @path.nil? + raise ArgumentError, "path is missing" + when @origin.nil? + true + else + acceptable_from_uri?(@origin) + end + end + # Tests if it is OK to send this cookie to a given `uri`. A runtime # error is raised if the cookie's domain is unknown. def valid_for_uri?(uri) diff --git a/lib/http/cookie_jar.rb b/lib/http/cookie_jar.rb index 471b7fd..d2ea438 100644 --- a/lib/http/cookie_jar.rb +++ b/lib/http/cookie_jar.rb @@ -45,8 +45,9 @@ class HTTP::CookieJar @store = other.instance_eval { @store.dup } end - # Adds a cookie to the jar and returns self. A given cookie must - # have domain and path attributes set, or ArgumentError is raised. + # Adds a cookie to the jar if it is acceptable, and returns self in + # any case. A given cookie must have domain and path attributes + # set, or ArgumentError is raised. # # ### Compatibility Note for Mechanize::Cookie users # @@ -63,14 +64,10 @@ class HTTP::CookieJar # jar.add!(cookie) # no acceptance check is performed # # # HTTP::Cookie - # jar.origin = origin # acceptance check is performed - # jar.add(cookie) + # jar.origin = origin + # jar.add(cookie) # acceptance check is performed def add(cookie) - if cookie.domain.nil? || cookie.path.nil? - raise ArgumentError, "a cookie with unknown domain or path cannot be added" - end - - @store.add(cookie) + @store.add(cookie) if cookie.acceptable? self end alias << add diff --git a/test/test_http_cookie.rb b/test/test_http_cookie.rb index c5450ac..0f86837 100644 --- a/test/test_http_cookie.rb +++ b/test/test_http_cookie.rb @@ -450,6 +450,9 @@ class TestHTTPCookie < Test::Unit::TestCase assert_equal 'key', cookie.name assert_equal 'value', cookie.value assert_equal nil, cookie.expires + assert_raises(ArgumentError) { + cookie.acceptable? + } # Minimum unit for the expires attribute is second expires = Time.at((Time.now + 3600).to_i) @@ -458,24 +461,39 @@ class TestHTTPCookie < Test::Unit::TestCase assert_equal 'key', cookie.name assert_equal 'value', cookie.value assert_equal expires, cookie.expires + assert_raises(ArgumentError) { + cookie.acceptable? + } cookie = HTTP::Cookie.new(:value => 'value', :name => 'key', :expires => expires.dup) assert_equal 'key', cookie.name assert_equal 'value', cookie.value assert_equal expires, cookie.expires assert_equal false, cookie.for_domain? + assert_raises(ArgumentError) { + # domain and path are missing + cookie.acceptable? + } cookie = HTTP::Cookie.new(:value => 'value', :name => 'key', :expires => expires.dup, :domain => '.example.com') assert_equal 'key', cookie.name assert_equal 'value', cookie.value assert_equal expires, cookie.expires assert_equal true, cookie.for_domain? + assert_raises(ArgumentError) { + # path is missing + cookie.acceptable? + } cookie = HTTP::Cookie.new(:value => 'value', :name => 'key', :expires => expires.dup, :domain => 'example.com', :for_domain => false) assert_equal 'key', cookie.name assert_equal 'value', cookie.value assert_equal expires, cookie.expires assert_equal false, cookie.for_domain? + assert_raises(ArgumentError) { + # path is missing + cookie.acceptable? + } cookie = HTTP::Cookie.new(:value => 'value', :name => 'key', :expires => expires.dup, :domain => 'example.org', :for_domain? => true) assert_equal 'key', cookie.name @@ -483,6 +501,10 @@ class TestHTTPCookie < Test::Unit::TestCase assert_equal expires, cookie.expires assert_equal 'example.org', cookie.domain assert_equal true, cookie.for_domain? + assert_raises(ArgumentError) { + # path is missing + cookie.acceptable? + } assert_raises(ArgumentError) { HTTP::Cookie.new(:name => 'name') } assert_raises(ArgumentError) { HTTP::Cookie.new(:value => 'value') } @@ -591,30 +613,46 @@ class TestHTTPCookie < Test::Unit::TestCase def test_new_rejects_cookies_that_do_not_contain_an_embedded_dot url = URI 'http://rubyforge.org/' - assert_raises(ArgumentError) { - tld_cookie = HTTP::Cookie.new(cookie_values(:domain => '.org', :origin => url)) - } - assert_raises(ArgumentError) { - single_dot_cookie = HTTP::Cookie.new(cookie_values(:domain => '.', :origin => url)) - } + tld_cookie1 = HTTP::Cookie.new(cookie_values(:domain => 'org', :origin => url)) + assert_equal false, tld_cookie1.for_domain? + assert_equal 'org', tld_cookie1.domain + assert_equal false, tld_cookie1.acceptable? + + tld_cookie2 = HTTP::Cookie.new(cookie_values(:domain => '.org', :origin => url)) + assert_equal false, tld_cookie1.for_domain? + assert_equal 'org', tld_cookie2.domain + assert_equal false, tld_cookie2.acceptable? + end + + def test_new_tld_domain_from_tld + url = URI 'http://org/' + + tld_cookie1 = HTTP::Cookie.new(cookie_values(:domain => 'org', :origin => url)) + assert_equal false, tld_cookie1.for_domain? + assert_equal 'org', tld_cookie1.domain + assert_equal true, tld_cookie1.acceptable? + + tld_cookie2 = HTTP::Cookie.new(cookie_values(:domain => '.org', :origin => url)) + assert_equal false, tld_cookie1.for_domain? + assert_equal 'org', tld_cookie2.domain + assert_equal true, tld_cookie2.acceptable? end def test_fall_back_rules_for_local_domains url = URI 'http://www.example.local' - assert_raises(ArgumentError) { - tld_cookie = HTTP::Cookie.new(cookie_values(:domain => '.local', :origin => url)) - } + tld_cookie = HTTP::Cookie.new(cookie_values(:domain => '.local', :origin => url)) + assert_equal false, tld_cookie.acceptable? sld_cookie = HTTP::Cookie.new(cookie_values(:domain => '.example.local', :origin => url)) + assert_equal true, sld_cookie.acceptable? end def test_new_rejects_cookies_with_ipv4_address_subdomain url = URI 'http://192.168.0.1/' - assert_raises(ArgumentError) { - cookie = HTTP::Cookie.new(cookie_values(:domain => '.0.1', :origin => url)) - } + cookie = HTTP::Cookie.new(cookie_values(:domain => '.0.1', :origin => url)) + assert_equal false, cookie.acceptable? end def test_path @@ -675,11 +713,15 @@ class TestHTTPCookie < Test::Unit::TestCase url = URI.parse('http://example.com/path/') cookie = HTTP::Cookie.new('a', 'b') + assert_raises(ArgumentError) { + cookie.origin = 123 + } cookie.origin = url assert_equal '/path/', cookie.path assert_equal 'example.com', cookie.domain assert_equal false, cookie.for_domain assert_raises(ArgumentError) { + # cannot change the origin once set cookie.origin = URI.parse('http://www.example.com/') } @@ -689,13 +731,21 @@ class TestHTTPCookie < Test::Unit::TestCase assert_equal 'example.com', cookie.domain assert_equal true, cookie.for_domain assert_raises(ArgumentError) { + # cannot change the origin once set cookie.origin = URI.parse('http://www.example.com/') } cookie = HTTP::Cookie.new('a', 'b', :domain => '.example.com') - assert_raises(ArgumentError) { - cookie.origin = URI.parse('http://example.org/') - } + cookie.origin = URI.parse('http://example.org/') + assert_equal false, cookie.acceptable? + + cookie = HTTP::Cookie.new('a', 'b', :domain => '.example.com') + cookie.origin = 'file:///tmp/test.html' + assert_equal nil, cookie.path + + cookie = HTTP::Cookie.new('a', 'b', :domain => '.example.com', :path => '/') + cookie.origin = 'file:///tmp/test.html' + assert_equal false, cookie.acceptable? end def test_acceptable_from_uri? diff --git a/test/test_http_cookie_jar.rb b/test/test_http_cookie_jar.rb index d7c18ec..bd3f9dd 100644 --- a/test/test_http_cookie_jar.rb +++ b/test/test_http_cookie_jar.rb @@ -284,9 +284,7 @@ module TestHTTPCookieJar def test_cookies_no_host url = URI 'file:///path/' - assert_raises(ArgumentError) { - @jar.add(HTTP::Cookie.new(cookie_values(:origin => url))) - } + @jar.add(HTTP::Cookie.new(cookie_values(:origin => url))) assert_equal(0, @jar.cookies(url).length) end