From eed7e57813c70e90a66fd386187d11fc0a551a5f Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Wed, 27 Mar 2013 19:46:44 +0900 Subject: [PATCH] Use the cookie creation time as base time for Max-Age. Now #expire returns created_at + max_age when expires is nil. Cookie.parse: the :date keyword is renamed to :created_at, and the value is set to in parsed cookies via #created_at. In YAML serialization, #max_age is stored. --- lib/http/cookie.rb | 46 +++++++++++++------------ lib/http/cookie_jar/cookiestxt_saver.rb | 2 +- test/test_http_cookie.rb | 44 +++++++++++++++++++---- test/test_http_cookie_jar.rb | 15 +++++--- 4 files changed, 74 insertions(+), 33 deletions(-) diff --git a/lib/http/cookie.rb b/lib/http/cookie.rb index d6584e2..6e4be4e 100644 --- a/lib/http/cookie.rb +++ b/lib/http/cookie.rb @@ -38,7 +38,8 @@ class HTTP::Cookie name value domain for_domain path secure httponly - expires created_at accessed_at + expires max_age + created_at accessed_at ] if String.respond_to?(:try_convert) @@ -110,8 +111,8 @@ class HTTP::Cookie # The setter method accepts a Time object, a string representation # of date/time, or `nil`. # - # Note that #max_age and #expires are mutually exclusive. Setting - # \#max_age resets #expires to nil, and vice versa. + # Setting this value resets #max_age to nil. When #max_age is + # non-nil, #expires returns `created_at + max_age`. # # :attr_accessor: expires @@ -122,8 +123,7 @@ class HTTP::Cookie # represents an integer which will be stringified and then # integerized using #to_i. # - # Note that #max_age and #expires are mutually exclusive. Setting - # \#max_age resets #expires to nil, and vice versa. + # This value is reset to nil when #expires= is called. # # :attr_accessor: max_age @@ -232,10 +232,6 @@ class HTTP::Cookie # given origin is not allowed to issue is not included in the # resulted array. # - # Any Max-Age attribute value found is converted to an expires - # value computing from the current time so that expiration check - # (#expired?) can be performed. - # # If a block is given, each cookie object is passed to the block. # # Available option keywords are below: @@ -243,9 +239,8 @@ class HTTP::Cookie # :origin # : The cookie's origin URI/URL # - # :date - # : The base date used for interpreting Max-Age attribute values - # instead of the current time + # :created_at + # : The creation time of the cookies parsed. # # :logger # : Logger object useful for debugging @@ -271,9 +266,8 @@ class HTTP::Cookie if options logger = options[:logger] origin = options[:origin] and origin = URI(origin) - date = options[:date] + created_at = options[:created_at] end - date ||= Time.now [].tap { |cookies| s = Scanner.new(set_cookie, logger) @@ -282,6 +276,7 @@ class HTTP::Cookie break if name.nil? || name.empty? cookie = new(name, value) + cookie.created_at = created_at if created_at attrs.each { |aname, avalue| begin case aname @@ -311,10 +306,6 @@ class HTTP::Cookie end } - # Have `expires` set instead of `max_age`, so that - # expiration check (`expired?`) can be performed. - cookie.expires = date + cookie.max_age if cookie.max_age - if origin begin cookie.origin = origin @@ -445,7 +436,9 @@ class HTTP::Cookie attr_reader :session alias session? session - attr_reader :expires + def expires + @expires or @created_at && @max_age ? @created_at + @max_age : nil + end # See #expires. def expires=(t) @@ -483,8 +476,11 @@ class HTTP::Cookie # Tests if this cookie is expired by now, or by a given time. def expired?(time = Time.now) - return false unless @expires - time > @expires + if expires = self.expires + expires <= time + else + false + end end # Expires this cookie by setting the expires attribute value to a @@ -501,7 +497,8 @@ class HTTP::Cookie # The comment attribute. attr_accessor :comment - # The time this cookie was created at. + # The time this cookie was created at. This value is used as a base + # date for interpreting the Max-Age attribute value. See #expires. attr_accessor :created_at # The time this cookie was last accessed at. @@ -618,11 +615,16 @@ class HTTP::Cookie # YAML deserialization helper for Psych. def yaml_initialize(tag, map) + expires = nil map.each { |key, value| case key + when 'expires' + # avoid clobbering max_age + expires = value when *PERSISTENT_PROPERTIES __send__(:"#{key}=", value) end } + self.expires = expires if self.max_age.nil? end end diff --git a/lib/http/cookie_jar/cookiestxt_saver.rb b/lib/http/cookie_jar/cookiestxt_saver.rb index dc9b0e3..9f22ca1 100644 --- a/lib/http/cookie_jar/cookiestxt_saver.rb +++ b/lib/http/cookie_jar/cookiestxt_saver.rb @@ -39,7 +39,7 @@ class HTTP::CookieJar::CookiestxtSaver < HTTP::CookieJar::AbstractSaver @for_domain ? True : False, @path, @secure ? True : False, - @expires.to_i, + expires.to_i, @name, @value ] diff --git a/test/test_http_cookie.rb b/test/test_http_cookie.rb index fd00076..a46e89e 100644 --- a/test/test_http_cookie.rb +++ b/test/test_http_cookie.rb @@ -200,18 +200,18 @@ class TestHTTPCookie < Test::Unit::TestCase cookie = HTTP::Cookie.parse('name=Akinori; max-age=3600', :origin => url).first assert_in_delta Time.now + 3600, cookie.expires, 1 - cookie = HTTP::Cookie.parse('name=Akinori; max-age=3600', :origin => url, :date => base).first + cookie = HTTP::Cookie.parse('name=Akinori; max-age=3600', :origin => url, :created_at => base).first assert_equal base + 3600, cookie.expires # Max-Age has precedence over Expires cookie = HTTP::Cookie.parse("name=Akinori; max-age=3600; expires=#{date}", :origin => url).first assert_in_delta Time.now + 3600, cookie.expires, 1 - cookie = HTTP::Cookie.parse("name=Akinori; max-age=3600; expires=#{date}", :origin => url, :date => base).first + cookie = HTTP::Cookie.parse("name=Akinori; max-age=3600; expires=#{date}", :origin => url, :created_at => base).first assert_equal base + 3600, cookie.expires cookie = HTTP::Cookie.parse("name=Akinori; expires=#{date}; max-age=3600", :origin => url).first assert_in_delta Time.now + 3600, cookie.expires, 1 - cookie = HTTP::Cookie.parse("name=Akinori; expires=#{date}; max-age=3600", :origin => url, :date => base).first + cookie = HTTP::Cookie.parse("name=Akinori; expires=#{date}; max-age=3600", :origin => url, :created_at => base).first assert_equal base + 3600, cookie.expires end @@ -382,8 +382,8 @@ class TestHTTPCookie < Test::Unit::TestCase date = Time.at(Time.now.to_i) cookie_params.keys.combine.each do |keys| cookie_text = [cookie_value, *keys.map { |key| cookie_params[key] }].join('; ') - cookie, = HTTP::Cookie.parse(cookie_text, :origin => url, :date => date) - cookie2, = HTTP::Cookie.parse(cookie.set_cookie_value, :origin => url, :date => date) + cookie, = HTTP::Cookie.parse(cookie_text, :origin => url, :created_at => date) + cookie2, = HTTP::Cookie.parse(cookie.set_cookie_value, :origin => url, :created_at => date) assert_equal(cookie.name, cookie2.name) assert_equal(cookie.value, cookie2.value) @@ -539,7 +539,7 @@ class TestHTTPCookie < Test::Unit::TestCase cookie.max_age = 3600 assert_equal false, cookie.session? - assert_equal nil, cookie.expires + assert_equal cookie.created_at + 3600, cookie.expires cookie.max_age = nil assert_equal true, cookie.session? @@ -775,6 +775,38 @@ class TestHTTPCookie < Test::Unit::TestCase } end + def test_yaml_expires + require 'yaml' + cookie = HTTP::Cookie.new(cookie_values) + + assert_equal false, cookie.session? + assert_equal nil, cookie.max_age + + ycookie = YAML.load(cookie.to_yaml) + assert_equal false, ycookie.session? + assert_equal nil, ycookie.max_age + + cookie.expires = nil + ycookie = YAML.load(cookie.to_yaml) + assert_equal true, ycookie.session? + assert_equal nil, ycookie.max_age + + cookie.expires = Time.now + 3600 + ycookie = YAML.load(cookie.to_yaml) + assert_equal false, ycookie.session? + assert_equal nil, ycookie.max_age + + cookie.max_age = 3600 + ycookie = YAML.load(cookie.to_yaml) + assert_equal false, ycookie.session? + assert_equal cookie.created_at + 3600, ycookie.expires + + cookie.max_age = nil + ycookie = YAML.load(cookie.to_yaml) + assert_equal true, ycookie.session? + assert_equal nil, ycookie.expires + end + def test_s_path_match? assert_equal true, HTTP::Cookie.path_match?('/admin/', '/admin/index') assert_equal false, HTTP::Cookie.path_match?('/admin/', '/Admin/index') diff --git a/test/test_http_cookie_jar.rb b/test/test_http_cookie_jar.rb index 827a5b2..b972429 100644 --- a/test/test_http_cookie_jar.rb +++ b/test/test_http_cookie_jar.rb @@ -363,13 +363,16 @@ class TestHTTPCookieJar < Test::Unit::TestCase h_cookie = HTTP::Cookie.new(cookie_values(:name => 'Quux', :value => 'Foo#Quux', :httponly => true)) - + ma_cookie = HTTP::Cookie.new(cookie_values(:name => 'Maxage', + :value => 'Foo#Maxage', + :max_age => 15000)) @jar.add(cookie) @jar.add(s_cookie) @jar.add(cookie2) @jar.add(h_cookie) + @jar.add(ma_cookie) - assert_equal(4, @jar.cookies(url).length) + assert_equal(5, @jar.cookies(url).length) Dir.mktmpdir do |dir| filename = File.join(dir, "cookies.txt") @@ -384,7 +387,7 @@ class TestHTTPCookieJar < Test::Unit::TestCase jar = HTTP::CookieJar.new jar.load(filename, :cookiestxt) # HACK test the format cookies = jar.cookies(url) - assert_equal(3, cookies.length) + assert_equal(4, cookies.length) cookies.each { |cookie| case cookie.name when 'Foo' @@ -407,13 +410,17 @@ class TestHTTPCookieJar < Test::Unit::TestCase assert_equal true, cookie.for_domain assert_equal '/', cookie.path assert_equal true, cookie.httponly? + when 'Maxage' + assert_equal 'Foo#Maxage', cookie.value + assert_equal nil, cookie.max_age + assert_in_delta ma_cookie.expires, cookie.expires, 1 else raise end } end - assert_equal(4, @jar.cookies(url).length) + assert_equal(5, @jar.cookies(url).length) end def test_expire_cookies