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

Cookie expiration #5042

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 60 additions & 16 deletions spec/std/http/cookie_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 27 additions & 9 deletions src/http/cookie.cr
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,35 @@ 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

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
Expand All @@ -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
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

max_age = match["max_age"]?.try(&.to_i.seconds)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggestion seems better than the current code.


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