From aae5f30e554d71d042f4b1a1659580a1d14210bb Mon Sep 17 00:00:00 2001 From: Brandon McGinty-Carroll Date: Tue, 19 Jun 2018 00:42:22 -0400 Subject: [PATCH] setup propper handling of max-age and expires for cookies --- spec/std/http/cookie_spec.cr | 76 ++++++++++++++++++++++++++++-------- src/http/cookie.cr | 36 ++++++++++++----- 2 files changed, 87 insertions(+), 25 deletions(-) diff --git a/spec/std/http/cookie_spec.cr b/spec/std/http/cookie_spec.cr index f659429a2d41..689ae852f7ef 100644 --- a/spec/std/http/cookie_spec.cr +++ b/spec/std/http/cookie_spec.cr @@ -152,36 +152,80 @@ module HTTP it "parse max-age as seconds from Time.now" do cookie = parse_set_cookie("a=1; max-age=10") - delta = cookie.expires.not_nil! - Time.now - delta.should be > 9.seconds - delta.should be < 11.seconds + cookie.max_age.should eq 10.seconds cookie = parse_set_cookie("a=1; max-age=0") - delta = Time.now - cookie.expires.not_nil! - delta.should be > 0.seconds - delta.should be < 1.seconds + cookie.max_age.should eq 0.seconds + end + end + + describe "expiration_time" do + it "sets expiration_time to be current when max-age=0" do + cookie = parse_set_cookie("bla=1; max-age=0") + (Time.now - cookie.expiration_time.not_nil!).should be <= 1.seconds + end + + it "sets expiration_time with old date" do + cookie = parse_set_cookie("bla=1; expires=Thu, 01 Jan 1970 00:00:00 -0000") + cookie.expiration_time.should eq Time.utc(1970, 1, 1, 0, 0, 0) + end + + it "sets future expiration_time with max-age" do + cookie = parse_set_cookie("bla=1; max-age=1") + (cookie.expiration_time.not_nil!).should be > Time.now + end + + it "sets future expiration_time with max-age and future cookie creation time" do + cookie = parse_set_cookie("bla=1; max-age=1") + cookie_expiration = cookie.expiration_time(time_reference: Time.now + 20.seconds).not_nil! + (Time.now + 20.seconds).should be < cookie_expiration + (Time.now + 21.seconds).should be >= cookie_expiration + end + + it "sets future expiration_time with expires" do + cookie = parse_set_cookie("bla=1; expires=Thu, 01 Jan 2020 00:00:00 -0000") + (cookie.expiration_time.not_nil! >= Time.utc(2020, 1, 1, 0, 0, 0)).should be_true + end + + it "returns nil expiration_time when expires and max-age are not set" do + cookie = parse_set_cookie("bla=1") + cookie.expiration_time.should be_nil end end describe "expired?" do - it "by max-age=0" do - parse_set_cookie("bla=1; max-age=0").expired?.should eq true + it "expired when max-age=0" do + cookie = parse_set_cookie("bla=1; max-age=0") + cookie.expired?.should be_true + end + + it "expired with old expires date" do + cookie = parse_set_cookie("bla=1; expires=Thu, 01 Jan 1970 00:00:00 -0000") + cookie.expired?.should be_true end - it "by old date" do - parse_set_cookie("bla=1; expires=Thu, 01 Jan 1970 00:00:00 -0000").expired?.should eq true + it "not expired with future max-age" do + cookie = parse_set_cookie("bla=1; max-age=1") + cookie.expired?.should be_false end - it "not expired" do - parse_set_cookie("bla=1; max-age=1").expired?.should eq false + it "not expired with future expires" do + cookie = parse_set_cookie("bla=1; expires=Thu, 01 Jan 2020 00:00:00 -0000") + cookie.expired?.should be_false end - it "not expired" do - parse_set_cookie("bla=1; expires=Thu, 01 Jan 2020 00:00:00 -0000").expired?.should eq false + it "sets past expiration_time with max-age and future time reference" do + cookie = parse_set_cookie("bla=1; max-age=1") + cookie_expiration = cookie.expiration_time(time_reference: Time.now - 20.seconds).not_nil! + (Time.now - 20.seconds).should be < cookie_expiration + (Time.now - 18.seconds).should be > cookie_expiration + cookie_expired = cookie.expired?(time_reference: Time.now - 20.seconds) + cookie_expired.should be_true end - it "not expired" do - parse_set_cookie("bla=1").expired?.should eq false + it "not expired when max-age and expires are not provided" do + cookie = parse_set_cookie("bla=1") + cookie.expired?.should be_false end end end diff --git a/src/http/cookie.cr b/src/http/cookie.cr index 21e71f7cd248..4ed941781763 100644 --- a/src/http/cookie.cr +++ b/src/http/cookie.cr @@ -8,17 +8,20 @@ module HTTP property value : String property path : String property expires : Time? + property max_age : Time::Span? property domain : String? property secure : Bool property http_only : Bool property extension : String? + @creation_time : Time def_equals_and_hash name, value, path, expires, domain, secure, http_only def initialize(@name : String, value : String, @path : String = "/", - @expires : Time? = nil, @domain : String? = nil, + @expires : Time? = nil, @max_age : Time::Span? = nil, @domain : String? = nil, @secure : Bool = false, @http_only : Bool = false, @extension : String? = nil) + @creation_time = Time.now @name = URI.unescape name @value = URI.unescape value end @@ -26,12 +29,14 @@ module HTTP def to_set_cookie_header path = @path expires = @expires + max_age = @max_age domain = @domain String.build do |header| header << "#{URI.escape @name}=#{URI.escape value}" header << "; domain=#{domain}" if domain header << "; path=#{path}" if path header << "; expires=#{HTTP.format_time(expires)}" if expires + header << "; max-age=#{max_age.total_seconds}" if max_age header << "; Secure" if @secure header << "; HttpOnly" if @http_only header << "; #{@extension}" if @extension @@ -42,9 +47,24 @@ module HTTP "#{@name}=#{URI.escape value}" end - def expired? - if e = expires - e < Time.now + # Returns the `Time` at which this cookie will expire, or `nil` if it will not expire. + # Uses *max-age* and *expires* values to calculate the time. + # By default, this function uses the creation time of this cookie as the offset for max-age, if max-age is set. + # To use a different offset, provide a `Time` object to *time_reference*. + def expiration_time(time_reference = @creation_time) + if max_age = @max_age + time_reference + max_age + else + @expires + end + end + + # returns the expiration status of this cookie as a `Bool` given a creation time + def expired?(time_reference = @creation_time) + if @max_age == 0.seconds + true + elsif (time = expiration_time(time_reference)) && time < Time.now + true else false end @@ -99,16 +119,14 @@ module HTTP match = header.match(SetCookieString) return unless match - expires = if max_age = match["max_age"]? - Time.now + max_age.to_i.seconds - else - parse_time(match["expires"]?) - end + expires = parse_time(match["expires"]?) + max_age = match["max_age"]? ? match["max_age"].to_i.seconds : nil Cookie.new( match["name"], match["value"], path: match["path"]? || "/", expires: expires, + max_age: max_age, domain: match["domain"]?, secure: match["secure"]? != nil, http_only: match["http_only"]? != nil,