Proper handling of max-age and expires for cookies

spec/std/http/cookie_spec.cr

@@ -193,42 +193,75 @@ module HTTP
         parse_set_cookie("a=1; domain=127.0.0.1; path=/; HttpOnly").domain.should eq "127.0.0.1"
       end
 
-      it "parse max-age as seconds from current time" do
+      it "parse max-age as Time::Span" do
         cookie = parse_set_cookie("a=1; max-age=10")
-        delta = cookie.expires.not_nil! - Time.utc
-        delta.should be_close(10.seconds, 1.second)
+        cookie.max_age.should eq 10.seconds
 
         cookie = parse_set_cookie("a=1; max-age=0")
-        delta = Time.utc - cookie.expires.not_nil!
-        delta.should be_close(0.seconds, 1.second)
+        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")
+        expiration_time = cookie.expiration_time.should_not be_nil
+        expiration_time.should be_close(Time.utc, 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.utc
+      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.should_not be_nil
+        cookie_expiration.should be_close(Time.utc, 1.seconds)
+
+        cookie.expired?(time_reference: cookie.creation_time + 1.second).should be_true
+      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.should eq Time.utc(2020, 1, 1, 0, 0, 0)
       end
 
-      it "parses large max-age (#8744)" do
-        cookie = parse_set_cookie("a=1; max-age=3153600000")
-        delta = cookie.expires.not_nil! - Time.utc
-        delta.should be_close(3153600000.seconds, 1.second)
+      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 "by old date" do
-        parse_set_cookie("bla=1; expires=Thu, 01 Jan 1970 00:00:00 -0000").expired?.should eq true
+      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 "not expired" do
-        parse_set_cookie("bla=1; max-age=1").expired?.should eq false
+      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; expires=Thu, 01 Jan #{Time.utc.year + 2} 00:00:00 -0000").expired?.should eq false
+      it "not expired with future expires" do
+        cookie = parse_set_cookie("bla=1; expires=Thu, 01 Jan #{Time.utc.year + 2} 00:00:00 -0000")
+        cookie.expired?.should be_false
       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

src/http/cookie.cr

@@ -24,27 +24,49 @@ module HTTP
     property http_only : Bool
     property samesite : SameSite?
     property extension : String?
+    property max_age : Time::Span?
+    getter creation_time : Time
 
     def_equals_and_hash name, value, path, expires, domain, secure, http_only
 
-    def initialize(@name : String, value : String, @path : String = "/",
+    @[Deprecated("Use named arguments instead.")]
+    def self.new(_name : String, _value : String, _path : String = "/",
+                 _expires : Time? = nil, _domain : String? = nil,
+                 _secure : Bool = false, _http_only : Bool = false,
+                 _samesite : SameSite? = nil, _extension : String? = nil) : self
+      new(
+        _name, _value,
+        path: _path, expires: _expires, domain: _domain, secure: _secure,
+        http_only: _http_only, samesite: _samesite, extension: _extension
+      )
+    end
+
+    def self.new(_name : String, _value : String) : self
+      new(name: _name, value: _value)
+    end
+
+    def initialize(@name : String, @value : String,
+                   *,
+                   @path : String = "/",
                    @expires : Time? = nil, @domain : String? = nil,
                    @secure : Bool = false, @http_only : Bool = false,
-                   @samesite : SameSite? = nil, @extension : String? = nil)
-      @name = name
-      @value = value
+                   @samesite : SameSite? = nil, @extension : String? = nil,
+                   @max_age : Time::Span? = nil, @creation_time = Time.utc)
+      raise "Invalid max_age" if @max_age.try { |max_age| max_age < Time::Span.zero }
     end
 
     def to_set_cookie_header
       path = @path
       expires = @expires
+      max_age = @max_age
       domain = @domain
       samesite = @samesite
       String.build do |header|
         to_cookie_header(header)
         header << "; domain=#{domain}" if domain
         header << "; path=#{path}" if path
         header << "; expires=#{HTTP.format_time(expires)}" if expires
+        header << "; max-age=#{max_age.to_i}" if max_age
         header << "; Secure" if @secure
         header << "; HttpOnly" if @http_only
         header << "; SameSite=#{samesite}" if samesite
@@ -64,9 +86,24 @@ module HTTP
       URI.encode_www_form(value, io)
     end
 
-    def expired?
-      if e = expires
-        e <= Time.utc
+    # Returns the expiration time of this cookie.
+    def expiration_time : Time?
+      if max_age = @max_age
+        @creation_time + max_age
+      else
+        @expires
+      end
+    end
+
+    # Returns the expiration status of this cookie as a `Bool`.
+    #
+    # *time_reference* can be passed to use a different reference time for
+    # comparison. Default is the current time (`Time.utc`).
+    def expired?(time_reference = Time.utc) : Bool
+      if @max_age.try &.zero?
+        true
+      elsif expiration_time = self.expiration_time
+        expiration_time <= time_reference
       else
         false
       end
@@ -123,11 +160,8 @@ module HTTP
         match = header.match(SetCookieString)
         return unless match
 
-        expires = if max_age = match["max_age"]?
-                    Time.utc + max_age.to_i64.seconds
-                  else
-                    parse_time(match["expires"]?)
-                  end
+        expires = parse_time(match["expires"]?)
+        max_age = match["max_age"]?.try(&.to_i64.seconds)
 
         Cookie.new(
           URI.decode_www_form(match["name"]), URI.decode_www_form(match["value"]),
@@ -137,7 +171,8 @@ module HTTP
           secure: match["secure"]? != nil,
           http_only: match["http_only"]? != nil,
           samesite: match["samesite"]?.try { |v| SameSite.parse? v },
-          extension: match["extension"]?
+          extension: match["extension"]?,
+          max_age: max_age,
         )
       end