Commit ceba9fb4 authored by maksim.sisov's avatar maksim.sisov Committed by Commit bot

Add: check exploded time is properly converted.

This cl introduces time checking in posix-like and mac systems.
base::Time::FromUTCExploded() and
base::Time::FromLocalExplode() can fail without returning
a proper error.

This fix does the following:
1) After calculations are done, create UTC or local time.
2) Convert UTC or local time back to exploded
3) Compare original exploded with converted one
4) If times are not equal, then return Time(0) indicating
an error.

Windows implementation already returns Time(0) on error.

BUG=601900

Review-Url: https://codereview.chromium.org/1988663002
Cr-Commit-Position: refs/heads/master@{#396638}
parent 7c2861c6
......@@ -263,6 +263,14 @@ bool Time::FromStringInternal(const char* time_string,
return true;
}
// static
bool Time::ExplodedMostlyEquals(const Exploded& lhs, const Exploded& rhs) {
return lhs.year == rhs.year && lhs.month == rhs.month &&
lhs.day_of_month == rhs.day_of_month && lhs.hour == rhs.hour &&
lhs.minute == rhs.minute && lhs.second == rhs.second &&
lhs.millisecond == rhs.millisecond;
}
std::ostream& operator<<(std::ostream& os, Time time) {
Time::Exploded exploded;
time.UTCExplode(&exploded);
......
......@@ -56,6 +56,7 @@
#include <limits>
#include "base/base_export.h"
#include "base/compiler_specific.h"
#include "base/numerics/safe_math.h"
#include "build/build_config.h"
......@@ -519,11 +520,29 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
// Converts an exploded structure representing either the local time or UTC
// into a Time class.
// TODO(maksims): Get rid of these in favor of the methods below when
// all the callers stop using these ones.
static Time FromUTCExploded(const Exploded& exploded) {
return FromExploded(false, exploded);
base::Time time;
ignore_result(FromUTCExploded(exploded, &time));
return time;
}
static Time FromLocalExploded(const Exploded& exploded) {
return FromExploded(true, exploded);
base::Time time;
ignore_result(FromLocalExploded(exploded, &time));
return time;
}
// Converts an exploded structure representing either the local time or UTC
// into a Time class. Returns false on a failure when, for example, a day of
// month is set to 31 on a 28-30 day month.
static bool FromUTCExploded(const Exploded& exploded,
Time* time) WARN_UNUSED_RESULT {
return FromExploded(false, exploded, time);
}
static bool FromLocalExploded(const Exploded& exploded,
Time* time) WARN_UNUSED_RESULT {
return FromExploded(true, exploded, time);
}
// Converts a string representation of time to a Time object.
......@@ -564,8 +583,12 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
void Explode(bool is_local, Exploded* exploded) const;
// Unexplodes a given time assuming the source is either local time
// |is_local = true| or UTC |is_local = false|.
static Time FromExploded(bool is_local, const Exploded& exploded);
// |is_local = true| or UTC |is_local = false|. Function returns false on
// failure and sets |time| to Time(0). Otherwise returns true and sets |time|
// to non-exploded time.
static bool FromExploded(bool is_local,
const Exploded& exploded,
Time* time) WARN_UNUSED_RESULT;
// Converts a string representation of time to a Time object.
// An example of a time string which is converted is as below:-
......@@ -577,6 +600,9 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
static bool FromStringInternal(const char* time_string,
bool is_local,
Time* parsed_time);
// Comparison does not consider |day_of_week| when doing the operation.
static bool ExplodedMostlyEquals(const Exploded& lhs, const Exploded& rhs);
};
// static
......
......@@ -34,7 +34,7 @@ int64_t ComputeCurrentTicks() {
struct timeval boottime;
int mib[2] = {CTL_KERN, KERN_BOOTTIME};
size_t size = sizeof(boottime);
int kr = sysctl(mib, arraysize(mib), &boottime, &size, NULL, 0);
int kr = sysctl(mib, arraysize(mib), &boottime, &size, nullptr, 0);
DCHECK_EQ(KERN_SUCCESS, kr);
base::TimeDelta time_difference = base::Time::Now() -
(base::Time::FromTimeT(boottime.tv_sec) +
......@@ -168,7 +168,7 @@ Time Time::NowFromSystemTime() {
}
// static
Time Time::FromExploded(bool is_local, const Exploded& exploded) {
bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
base::ScopedCFTypeRef<CFTimeZoneRef> time_zone(
is_local
? CFTimeZoneCopySystem()
......@@ -184,8 +184,28 @@ Time Time::FromExploded(bool is_local, const Exploded& exploded) {
exploded.day_of_month, exploded.hour, exploded.minute, exploded.second,
exploded.millisecond);
CFAbsoluteTime seconds = absolute_time + kCFAbsoluteTimeIntervalSince1970;
return Time(static_cast<int64_t>(seconds * kMicrosecondsPerSecond) +
kWindowsEpochDeltaMicroseconds);
base::Time converted_time =
Time(static_cast<int64_t>(seconds * kMicrosecondsPerSecond) +
kWindowsEpochDeltaMicroseconds);
// If |exploded.day_of_month| is set to 31
// on a 28-30 day month, it will return the first day of the next month.
// Thus round-trip the time and compare the initial |exploded| with
// |utc_to_exploded| time.
base::Time::Exploded to_exploded;
if (!is_local)
converted_time.UTCExplode(&to_exploded);
else
converted_time.LocalExplode(&to_exploded);
if (ExplodedMostlyEquals(to_exploded, exploded)) {
*time = converted_time;
return true;
}
*time = Time(0);
return false;
}
void Time::Explode(bool is_local, Exploded* exploded) const {
......
......@@ -211,7 +211,7 @@ void Time::Explode(bool is_local, Exploded* exploded) const {
}
// static
Time Time::FromExploded(bool is_local, const Exploded& exploded) {
bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
struct tm timestruct;
timestruct.tm_sec = exploded.second;
timestruct.tm_min = exploded.minute;
......@@ -301,8 +301,26 @@ Time Time::FromExploded(bool is_local, const Exploded& exploded) {
}
// Adjust from Unix (1970) to Windows (1601) epoch.
return Time((milliseconds * kMicrosecondsPerMillisecond) +
kWindowsEpochDeltaMicroseconds);
base::Time converted_time =
Time((milliseconds * kMicrosecondsPerMillisecond) +
kWindowsEpochDeltaMicroseconds);
// If |exploded.day_of_month| is set to 31 on a 28-30 day month, it will
// return the first day of the next month. Thus round-trip the time and
// compare the initial |exploded| with |utc_to_exploded| time.
base::Time::Exploded to_exploded;
if (!is_local)
converted_time.UTCExplode(&to_exploded);
else
converted_time.LocalExplode(&to_exploded);
if (ExplodedMostlyEquals(to_exploded, exploded)) {
*time = converted_time;
return true;
}
*time = Time(0);
return false;
}
// TimeTicks ------------------------------------------------------------------
......
......@@ -21,6 +21,52 @@ namespace base {
namespace {
TEST(TimeTestOutOfBounds, FromExplodedOutOfBoundsTime) {
// FromUTCExploded must set time to Time(0) and failure, if the day is set to
// 31 on a 28-30 day month. Test |exploded| returns Time(0) on 31st of
// February and 31st of April. New implementation handles this.
const struct DateTestData {
Time::Exploded explode;
bool is_valid;
} kDateTestData[] = {
// 31st of February
{{2016, 2, 0, 31, 12, 30, 0, 0}, true},
// 31st of April
{{2016, 4, 0, 31, 8, 43, 0, 0}, true},
// Negative month
{{2016, -5, 0, 2, 4, 10, 0, 0}, false},
// Negative date of month
{{2016, 6, 0, -15, 2, 50, 0, 0}, false},
// Negative hours
{{2016, 7, 0, 10, -11, 29, 0, 0}, false},
// Negative minutes
{{2016, 3, 0, 14, 10, -29, 0, 0}, false},
// Negative seconds
{{2016, 10, 0, 25, 7, 47, -30, 0}, false},
// Negative milliseconds
{{2016, 10, 0, 25, 7, 47, 20, -500}, false},
// Hours are too large
{{2016, 7, 0, 10, 26, 29, 0, 0}, false},
// Minutes are too large
{{2016, 3, 0, 14, 10, 78, 0, 0}, false},
// Seconds are too large
{{2016, 10, 0, 25, 7, 47, 234, 0}, false},
// Milliseconds are too large
{{2016, 10, 0, 25, 6, 31, 23, 1643}, false},
};
for (const auto& test : kDateTestData) {
EXPECT_EQ(test.explode.HasValidValues(), test.is_valid);
base::Time result;
EXPECT_FALSE(base::Time::FromUTCExploded(test.explode, &result));
EXPECT_TRUE(result.is_null());
EXPECT_FALSE(base::Time::FromLocalExploded(test.explode, &result));
EXPECT_TRUE(result.is_null());
}
}
// Specialized test fixture allowing time strings without timezones to be
// tested by comparing them to a known time in the local zone.
// See also pr_time_unittests.cc
......
......@@ -235,7 +235,7 @@ bool Time::IsHighResolutionTimerInUse() {
}
// static
Time Time::FromExploded(bool is_local, const Exploded& exploded) {
bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
// Create the system struct representing our exploded time. It will either be
// in local time or UTC.
SYSTEMTIME st;
......@@ -253,17 +253,19 @@ Time Time::FromExploded(bool is_local, const Exploded& exploded) {
// Ensure that it's in UTC.
if (is_local) {
SYSTEMTIME utc_st;
success = TzSpecificLocalTimeToSystemTime(NULL, &st, &utc_st) &&
success = TzSpecificLocalTimeToSystemTime(nullptr, &st, &utc_st) &&
SystemTimeToFileTime(&utc_st, &ft);
} else {
success = !!SystemTimeToFileTime(&st, &ft);
}
if (!success) {
NOTREACHED() << "Unable to convert time";
return Time(0);
*time = Time(0);
return false;
}
return Time(FileTimeToMicroseconds(ft));
*time = Time(FileTimeToMicroseconds(ft));
return true;
}
void Time::Explode(bool is_local, Exploded* exploded) const {
......@@ -288,7 +290,7 @@ void Time::Explode(bool is_local, Exploded* exploded) const {
// daylight saving time, it will take daylight saving time into account,
// even if the time you are converting is in standard time.
success = FileTimeToSystemTime(&utc_ft, &utc_st) &&
SystemTimeToTzSpecificLocalTime(NULL, &utc_st, &st);
SystemTimeToTzSpecificLocalTime(nullptr, &utc_st, &st);
} else {
success = !!FileTimeToSystemTime(&utc_ft, &st);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment