Skip to content

Commit

Permalink
Add boundary check for seconds in Time#initialize (crystal-lang#5786)
Browse files Browse the repository at this point in the history
Previously, `Time#add_span` did not handle times at the min or max range with positive or
negative offsets correctly because `@seconds` can legitimately be `< 0` or `> MAX_SECONDS`
when the offset is taken into account.
The boundary check was moved to the constructor to prevent manually
creating an invalid date.
  • Loading branch information
straight-shoota authored and chris-huxtable committed Apr 6, 2018
1 parent 47ecd47 commit 4ffc372
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 4 deletions.
53 changes: 53 additions & 0 deletions spec/std/time/time_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,28 @@ describe Time do
end
end

it "#initialize checks boundary at time min" do
{-5 * 3600, -1, 0, 1, 5 * 3600}.each do |offset|
seconds = -offset.to_i64
Time.new(seconds: seconds + 1, nanoseconds: 0, location: Time::Location.fixed(offset))
Time.new(seconds: seconds, nanoseconds: 0, location: Time::Location.fixed(offset))
Time.expect_invalid do
Time.new(seconds: seconds - 1, nanoseconds: 0, location: Time::Location.fixed(offset))
end
end
end

it "#initialize checks boundary at time max" do
{-5 * 3600, -1, 0, 1, 5 * 3600}.each do |offset|
seconds = Time::MAX_SECONDS - offset.to_i64
Time.new(seconds: seconds - 1, nanoseconds: 0, location: Time::Location.fixed(offset))
Time.new(seconds: seconds, nanoseconds: 0, location: Time::Location.fixed(offset))
Time.expect_invalid do
Time.new(seconds: seconds + 1, nanoseconds: 0, location: Time::Location.fixed(offset))
end
end
end

it "initialize with .epoch" do
seconds = 1439404155
time = Time.epoch(seconds)
Expand Down Expand Up @@ -121,6 +143,37 @@ describe Time do
end
end

it "#add_span checks boundary at time min" do
{5 * 3600, 1, 0, -1, -5 * 3600}.each do |offset|
location = Time::Location.fixed(offset)

time = Time.new(1, 1, 1, location: location)
time.add_span(0, 1).should eq Time.new(1, 1, 1, nanosecond: 1, location: location)
time.add_span(0, 0).should eq time
expect_raises(ArgumentError) do
time.add_span(0, -1)
end
end
end

it "#add_span checks boundary at time max" do
{5 * 3600, 1, 0, -1, -5 * 3600}.each do |offset|
location = Time::Location.fixed(offset)

time = Time.new(9999, 12, 31, 23, 59, 59, nanosecond: 999_999_999, location: location)
time.add_span(0, -1).should eq Time.new(9999, 12, 31, 23, 59, 59, nanosecond: 999_999_998, location: location)
time.add_span(0, 0).should eq time
expect_raises(ArgumentError) do
time.add_span(0, 1)
end
end
end

it "adds zero span" do
time = Time.now
time.add_span(0, 0).should eq time
end

it "add days" do
t1 = Time.utc(2002, 2, 25, 15, 25, 13)
t1 = t1 + 3.days
Expand Down
8 changes: 4 additions & 4 deletions src/time.cr
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ struct Time
{% end %}

def initialize(*, @seconds : Int64, @nanoseconds : Int32, @location : Location)
unless 0 <= offset_seconds <= MAX_SECONDS
raise ArgumentError.new "Invalid time: seconds out of range"
end

unless 0 <= @nanoseconds < NANOSECONDS_PER_SECOND
raise ArgumentError.new "Invalid time: nanoseconds out of range"
end
Expand Down Expand Up @@ -303,10 +307,6 @@ struct Time
nanoseconds += NANOSECONDS_PER_SECOND
end

unless 0 <= seconds <= MAX_SECONDS
raise ArgumentError.new "Invalid time"
end

Time.new(seconds: seconds, nanoseconds: nanoseconds.to_i, location: location)
end

Expand Down

0 comments on commit 4ffc372

Please sign in to comment.