Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper handling of max-age and expires for cookies #7925

Closed
wants to merge 9 commits into from
76 changes: 60 additions & 16 deletions spec/std/http/cookie_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -194,36 +194,80 @@ module HTTP

it "parse max-age as seconds from current time" do
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
cookie = parse_set_cookie("a=1; max-age=10")
delta = cookie.expires.not_nil! - Time.utc
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.utc - 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
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
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
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
(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
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
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
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
36 changes: 25 additions & 11 deletions src/http/cookie.cr
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ 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
Expand All @@ -25,23 +26,27 @@ module HTTP
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,
@secure : Bool = false, @http_only : Bool = false,
@samesite : SameSite? = nil, @extension : String? = nil)
@expires : Time? = nil, @max_age : Time::Span? = nil,
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
@domain : String? = nil, @secure : Bool = false,
@http_only : Bool = false, @samesite : SameSite? = nil,
@extension : String? = nil)
@creation_time = Time.now
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
@name = URI.unescape name
@value = URI.unescape value
end

def to_set_cookie_header
path = @path
expires = @expires
max_age = @max_age
domain = @domain
samesite = @samesite
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
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
header << "; Secure" if @secure
header << "; HttpOnly" if @http_only
header << "; SameSite=#{samesite}" if samesite
Expand All @@ -53,9 +58,20 @@ module HTTP
"#{@name}=#{URI.escape value}"
end

def expired?
if e = expires
e < Time.utc
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
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
def expired?(time_reference = @creation_time)
if @max_age == 0.seconds
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
true
elsif (time = expiration_time(time_reference)) && time < Time.now
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
true
else
false
end
Expand Down Expand Up @@ -112,16 +128,14 @@ module HTTP
match = header.match(SetCookieString)
return unless match

expires = if max_age = match["max_age"]?
Time.utc + max_age.to_i.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"],
path: match["path"]? || "/",
expires: expires,
max_age: max_age,
domain: match["domain"]?,
secure: match["secure"]? != nil,
http_only: match["http_only"]? != nil,
Expand Down