Proper handling of max-age and expires for cookies (#10564)

Co-authored-by: Blacksmoke16 <[email protected]>
Co-authored-by: Chris Watson <[email protected]>
Co-authored-by: Chris <[email protected]>

spec/std/http/cookie_spec.cr

@@ -304,42 +304,75 @@ module HTTP
         parse_set_cookie("a=1; domain=127.0.0.1; 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,6 +24,8 @@ 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, samesite, extension
 
@@ -33,11 +35,13 @@ module HTTP
     def initialize(name : String, value : String, @path : String? = nil,
                    @expires : Time? = nil, @domain : String? = nil,
                    @secure : Bool = false, @http_only : Bool = false,
-                   @samesite : SameSite? = nil, @extension : String? = nil)
+                   @samesite : SameSite? = nil, @extension : String? = nil,
+                   @max_age : Time::Span? = nil, @creation_time = Time.utc)
       validate_name(name)
       @name = name
       validate_value(value)
       @value = value
+      raise IO::Error.new("Invalid max_age") if @max_age.try { |max_age| max_age < Time::Span.zero }
     end
 
     # Sets the name of this cookie.
@@ -81,13 +85,15 @@ module HTTP
     def to_set_cookie_header : String
       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
@@ -107,9 +113,24 @@ module HTTP
       io << @value
     end
 
-    def expired? : Bool
-      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
@@ -171,11 +192,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(
           match["name"], match["value"],
@@ -185,7 +203,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