From debbb91e7819d0146491a477145e569b889fb644 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Juli=C3=A1n=20Espina?= Date: Fri, 5 May 2023 06:05:40 +0000 Subject: [PATCH] Fix `Date` for dynamic timezones (#2877) * Fix `Date` for dynamic timezones * Fix test --- boa_engine/src/builtins/date/mod.rs | 245 ++++++++++++-------------- boa_engine/src/builtins/date/tests.rs | 11 +- boa_engine/src/builtins/date/utils.rs | 18 +- boa_engine/src/context/hooks.rs | 23 ++- 4 files changed, 152 insertions(+), 145 deletions(-) diff --git a/boa_engine/src/builtins/date/mod.rs b/boa_engine/src/builtins/date/mod.rs index e9360e1dff..f6ce1d798e 100644 --- a/boa_engine/src/builtins/date/mod.rs +++ b/boa_engine/src/builtins/date/mod.rs @@ -206,12 +206,11 @@ impl BuiltInConstructor for Date { // a. Let now be the time value (UTC) identifying the current time. // b. Return ToDateString(now). return Ok(JsValue::new( - DateTime::::from_utc( - context.host_hooks().utc_now(), - context.host_hooks().tz_offset(), - ) - .format("%a %b %d %Y %H:%M:%S GMT%:z") - .to_string(), + context + .host_hooks() + .local_from_utc(context.host_hooks().utc_now()) + .format("%a %b %d %Y %H:%M:%S GMT%:z") + .to_string(), )); } // 2. Let numberOfArgs be the number of elements in values. @@ -269,14 +268,7 @@ impl BuiltInConstructor for Date { // Separating this into its own function to simplify the logic. let dt = Self::construct_date(args, context)? - .and_then(|dt| { - context - .host_hooks() - .tz_offset() - .from_local_datetime(&dt) - .earliest() - }) - .map(|dt| dt.naive_utc()); + .and_then(|dt| context.host_hooks().local_from_naive_local(dt).earliest()); Self(dt.map(|dt| dt.timestamp_millis())) } @@ -469,14 +461,13 @@ impl Date { let t = this_time_value(this)?; // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); - if LOCAL { - t = context - .host_hooks() - .tz_offset() - .from_utc_datetime(&t) - .naive_local(); - } + let t = some_or_nan!(t + .and_then(NaiveDateTime::from_timestamp_millis) + .map(|dt| if LOCAL { + context.host_hooks().local_from_utc(dt).naive_local() + } else { + dt + })); // 3. Return DateFromTime(LocalTime(t)). Ok(JsValue::new(t.day())) @@ -499,14 +490,13 @@ impl Date { let t = this_time_value(this)?; // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); - if LOCAL { - t = context - .host_hooks() - .tz_offset() - .from_utc_datetime(&t) - .naive_local(); - } + let t = some_or_nan!(t + .and_then(NaiveDateTime::from_timestamp_millis) + .map(|dt| if LOCAL { + context.host_hooks().local_from_utc(dt).naive_local() + } else { + dt + })); // 3. Return WeekDay(LocalTime(t)). Ok(JsValue::new(t.weekday().num_days_from_sunday())) @@ -535,7 +525,7 @@ impl Date { let t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); // 3. Return YearFromTime(LocalTime(t)) - 1900𝔽. - let local = context.host_hooks().tz_offset().from_utc_datetime(&t); + let local = context.host_hooks().local_from_utc(t); Ok(JsValue::from(local.year() - 1900)) } @@ -555,14 +545,13 @@ impl Date { let t = this_time_value(this)?; // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); - if LOCAL { - t = context - .host_hooks() - .tz_offset() - .from_utc_datetime(&t) - .naive_local(); - } + let t = some_or_nan!(t + .and_then(NaiveDateTime::from_timestamp_millis) + .map(|dt| if LOCAL { + context.host_hooks().local_from_utc(dt).naive_local() + } else { + dt + })); // 3. Return YearFromTime(LocalTime(t)). Ok(JsValue::new(t.year())) @@ -584,14 +573,13 @@ impl Date { let t = this_time_value(this)?; // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); - if LOCAL { - t = context - .host_hooks() - .tz_offset() - .from_utc_datetime(&t) - .naive_local(); - } + let t = some_or_nan!(t + .and_then(NaiveDateTime::from_timestamp_millis) + .map(|dt| if LOCAL { + context.host_hooks().local_from_utc(dt).naive_local() + } else { + dt + })); // 3. Return HourFromTime(LocalTime(t)). Ok(JsValue::new(t.hour())) @@ -613,14 +601,13 @@ impl Date { let t = this_time_value(this)?; // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); - if LOCAL { - t = context - .host_hooks() - .tz_offset() - .from_utc_datetime(&t) - .naive_local(); - } + let t = some_or_nan!(t + .and_then(NaiveDateTime::from_timestamp_millis) + .map(|dt| if LOCAL { + context.host_hooks().local_from_utc(dt).naive_local() + } else { + dt + })); // 3. Return msFromTime(LocalTime(t)). Ok(JsValue::new(t.timestamp_subsec_millis())) @@ -642,14 +629,13 @@ impl Date { let t = this_time_value(this)?; // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); - if LOCAL { - t = context - .host_hooks() - .tz_offset() - .from_utc_datetime(&t) - .naive_local(); - } + let t = some_or_nan!(t + .and_then(NaiveDateTime::from_timestamp_millis) + .map(|dt| if LOCAL { + context.host_hooks().local_from_utc(dt).naive_local() + } else { + dt + })); // 3. Return MinFromTime(LocalTime(t)). Ok(JsValue::new(t.minute())) @@ -672,14 +658,13 @@ impl Date { let t = this_time_value(this)?; // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); - if LOCAL { - t = context - .host_hooks() - .tz_offset() - .from_utc_datetime(&t) - .naive_local(); - } + let t = some_or_nan!(t + .and_then(NaiveDateTime::from_timestamp_millis) + .map(|dt| if LOCAL { + context.host_hooks().local_from_utc(dt).naive_local() + } else { + dt + })); // 3. Return MonthFromTime(LocalTime(t)). Ok(JsValue::new(t.month0())) @@ -701,14 +686,13 @@ impl Date { let t = this_time_value(this)?; // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); - if LOCAL { - t = context - .host_hooks() - .tz_offset() - .from_utc_datetime(&t) - .naive_local(); - } + let t = some_or_nan!(t + .and_then(NaiveDateTime::from_timestamp_millis) + .map(|dt| if LOCAL { + context.host_hooks().local_from_utc(dt).naive_local() + } else { + dt + })); // 3. Return SecFromTime(LocalTime(t)) Ok(JsValue::new(t.second())) @@ -751,11 +735,16 @@ impl Date { ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). // 2. If t is NaN, return NaN. - some_or_nan!(this_time_value(this)?); + let t = some_or_nan!(this_time_value(this)?.and_then(NaiveDateTime::from_timestamp_millis)); // 3. Return (t - LocalTime(t)) / msPerMinute. Ok(JsValue::from( - -context.host_hooks().tz_offset().local_minus_utc() / 60, + -context + .host_hooks() + .local_from_utc(t) + .offset() + .local_minus_utc() + / 60, )) } @@ -784,13 +773,13 @@ impl Date { // 4. Set t to LocalTime(t). // 5. Let newDate be MakeDate(MakeDay(YearFromTime(t), MonthFromTime(t), dt), TimeWithinDay(t)). // 6. Let u be TimeClip(UTC(newDate)). - let datetime = replace_params( + let datetime = replace_params::( datetime, DateParameters { date: Some(date), ..Default::default() }, - LOCAL.then(|| context.host_hooks().tz_offset()), + &*context.host_hooks(), ); // 7. Set the [[DateValue]] internal slot of this Date object to u. @@ -817,21 +806,20 @@ impl Date { get_mut_date!(let t = this); // 2. If t is NaN, set t to +0𝔽; otherwise, set t to LocalTime(t). - let datetime = match t.0.and_then(NaiveDateTime::from_timestamp_millis) { - Some(dt) => dt, - None if LOCAL => { - let Some(datetime) = context.host_hooks().tz_offset() - .from_local_datetime(&NaiveDateTime::default()) - .earliest() - .as_ref() - .map(DateTime::naive_utc) else { - *t = Self::new(None); - return Ok(t.as_value()) - }; - datetime - } - None => NaiveDateTime::default(), - }; + let datetime = + t.0.and_then(NaiveDateTime::from_timestamp_millis) + .or_else(|| { + if LOCAL { + context + .host_hooks() + .local_from_naive_local(NaiveDateTime::default()) + .earliest() + .map(|dt| dt.naive_utc()) + } else { + Some(NaiveDateTime::default()) + } + }); + let datetime = some_or_nan!(datetime); // 3. Let y be ? ToNumber(year). let year = args.get_or_undefined(0).to_integer_or_nan(context)?; @@ -850,7 +838,7 @@ impl Date { // 6. Let newDate be MakeDate(MakeDay(y, m, dt), TimeWithinDay(t)). // 7. Let u be TimeClip(UTC(newDate)). - let datetime = replace_params( + let datetime = replace_params::( datetime.timestamp_millis(), DateParameters { year: Some(year), @@ -858,7 +846,7 @@ impl Date { date, ..Default::default() }, - LOCAL.then(|| context.host_hooks().tz_offset()), + &*context.host_hooks(), ); // 8. Set the [[DateValue]] internal slot of this Date object to u. @@ -915,7 +903,7 @@ impl Date { // 10. If ms is not present, let milli be msFromTime(t). // 11. Let date be MakeDate(Day(t), MakeTime(h, m, s, milli)). // 12. Let u be TimeClip(UTC(date)). - let datetime = replace_params( + let datetime = replace_params::( datetime, DateParameters { hour: Some(hour), @@ -924,7 +912,7 @@ impl Date { millisecond, ..Default::default() }, - LOCAL.then(|| context.host_hooks().tz_offset()), + &*context.host_hooks(), ); // 13. Set the [[DateValue]] internal slot of this Date object to u. @@ -959,13 +947,13 @@ impl Date { // 4. Set t to LocalTime(t). // 5. Let time be MakeTime(HourFromTime(t), MinFromTime(t), SecFromTime(t), ms). // 6. Let u be TimeClip(UTC(MakeDate(Day(t), time))). - let datetime = replace_params( + let datetime = replace_params::( datetime, DateParameters { millisecond: Some(ms), ..Default::default() }, - LOCAL.then(|| context.host_hooks().tz_offset()), + &*context.host_hooks(), ); // 7. Set the [[DateValue]] internal slot of this Date object to u. @@ -1013,7 +1001,7 @@ impl Date { // 8. If ms is not present, let milli be msFromTime(t). // 9. Let date be MakeDate(Day(t), MakeTime(HourFromTime(t), m, s, milli)). // 10. Let u be TimeClip(UTC(date)). - let datetime = replace_params( + let datetime = replace_params::( datetime, DateParameters { minute: Some(minute), @@ -1021,7 +1009,7 @@ impl Date { millisecond, ..Default::default() }, - LOCAL.then(|| context.host_hooks().tz_offset()), + &*context.host_hooks(), ); // 11. Set the [[DateValue]] internal slot of this Date object to u. @@ -1063,14 +1051,14 @@ impl Date { // 6. If date is not present, let dt be DateFromTime(t). // 7. Let newDate be MakeDate(MakeDay(YearFromTime(t), m, dt), TimeWithinDay(t)). // 8. Let u be TimeClip(UTC(newDate)). - let datetime = replace_params( + let datetime = replace_params::( datetime, DateParameters { month: Some(month), date, ..Default::default() }, - LOCAL.then(|| context.host_hooks().tz_offset()), + &*context.host_hooks(), ); // 9. Set the [[DateValue]] internal slot of this Date object to u. @@ -1111,14 +1099,14 @@ impl Date { // 6. If ms is not present, let milli be msFromTime(t). // 7. Let date be MakeDate(Day(t), MakeTime(HourFromTime(t), MinFromTime(t), s, milli)). // 8. Let u be TimeClip(UTC(date)). - let datetime = replace_params( + let datetime = replace_params::( datetime, DateParameters { second: Some(second), millisecond, ..Default::default() }, - LOCAL.then(|| context.host_hooks().tz_offset()), + &*context.host_hooks(), ); // 9. Set the [[DateValue]] internal slot of this Date object to u. @@ -1155,16 +1143,16 @@ impl Date { let year = args.get_or_undefined(0).to_integer_or_nan(context)?; // 3. If t is NaN, set t to +0𝔽; otherwise, set t to LocalTime(t). - let Some(datetime) = t.0.and_then(NaiveDateTime::from_timestamp_millis).or_else(|| { - context.host_hooks().tz_offset() - .from_local_datetime(&NaiveDateTime::default()) - .earliest() - .as_ref() - .map(DateTime::naive_utc) - }) else { - *t = Self::new(None); - return Ok(t.as_value()); - }; + let datetime = + t.0.and_then(NaiveDateTime::from_timestamp_millis) + .or_else(|| { + context + .host_hooks() + .local_from_naive_local(NaiveDateTime::default()) + .earliest() + .map(|dt| dt.naive_utc()) + }); + let datetime = some_or_nan!(datetime); // 4. If y is NaN, then let Some(mut year) = year.as_integer() else { @@ -1183,13 +1171,13 @@ impl Date { // 8. Let d be MakeDay(yyyy, MonthFromTime(t), DateFromTime(t)). // 9. Let date be UTC(MakeDate(d, TimeWithinDay(t))). - let datetime = replace_params( + let datetime = replace_params::( datetime.timestamp_millis(), DateParameters { year: Some(IntegerOrNan::Integer(year)), ..Default::default() }, - Some(context.host_hooks().tz_offset()), + &*context.host_hooks(), ); // 10. Set the [[DateValue]] internal slot of this Date object to TimeClip(date). @@ -1257,8 +1245,7 @@ impl Date { // 5. Return DateString(t). Ok(context .host_hooks() - .tz_offset() - .from_utc_datetime(&tv) + .local_from_utc(tv) .format("%a %b %d %Y") .to_string() .into()) @@ -1395,13 +1382,12 @@ impl Date { ) -> JsResult { // 1. Let tv be ? thisTimeValue(this value). // 2. Return ToDateString(tv). - let Some(t) = this_time_value(this)?.and_then(NaiveDateTime::from_timestamp_millis) else { + let Some(tv) = this_time_value(this)?.and_then(NaiveDateTime::from_timestamp_millis) else { return Ok(js_string!("Invalid Date").into()); }; Ok(context .host_hooks() - .tz_offset() - .from_utc_datetime(&t) + .local_from_utc(tv) .format("%a %b %d %Y %H:%M:%S GMT%z") .to_string() .into()) @@ -1424,7 +1410,7 @@ impl Date { ) -> JsResult { // 1. Let O be this Date object. // 2. Let tv be ? thisTimeValue(O). - let Some(t) = this_time_value(this)?.and_then(NaiveDateTime::from_timestamp_millis) else { + let Some(tv) = this_time_value(this)?.and_then(NaiveDateTime::from_timestamp_millis) else { // 3. If tv is NaN, return "Invalid Date". return Ok(js_string!("Invalid Date").into()); }; @@ -1433,8 +1419,7 @@ impl Date { // 5. Return the string-concatenation of TimeString(t) and TimeZoneString(tv). Ok(context .host_hooks() - .tz_offset() - .from_utc_datetime(&t) + .local_from_utc(tv) .format("%H:%M:%S GMT%z") .to_string() .into()) diff --git a/boa_engine/src/builtins/date/tests.rs b/boa_engine/src/builtins/date/tests.rs index 3650caf0dc..6d337be732 100644 --- a/boa_engine/src/builtins/date/tests.rs +++ b/boa_engine/src/builtins/date/tests.rs @@ -258,7 +258,16 @@ fn date_proto_get_timezone_offset() { "new Date('1975-08-19T23:15:30+07:00').getTimezoneOffset()", { // The value of now().offset() depends on the host machine, so we have to replicate the method code here. - let offset_seconds = chrono::Local::now().offset().local_minus_utc(); + let dt = Local + .from_local_datetime( + &NaiveDate::from_ymd_opt(1975, 8, 19) + .unwrap() + .and_hms_opt(23, 15, 30) + .unwrap(), + ) + .earliest() + .unwrap(); + let offset_seconds = dt.offset().local_minus_utc(); -offset_seconds / 60 }, ), diff --git a/boa_engine/src/builtins/date/utils.rs b/boa_engine/src/builtins/date/utils.rs index db547a34c4..c0f39cc9c7 100644 --- a/boa_engine/src/builtins/date/utils.rs +++ b/boa_engine/src/builtins/date/utils.rs @@ -1,6 +1,6 @@ -use chrono::{Datelike, FixedOffset, NaiveDateTime, TimeZone, Timelike}; +use chrono::{Datelike, NaiveDateTime, Timelike}; -use crate::value::IntegerOrNan; +use crate::{context::HostHooks, value::IntegerOrNan}; /// The absolute maximum value of a timestamp pub(super) const MAX_TIMESTAMP: i64 = 864 * 10i64.pow(13); @@ -133,10 +133,10 @@ pub(super) struct DateParameters { } /// Replaces some (or all) parameters of `date` with the specified parameters -pub(super) fn replace_params( +pub(super) fn replace_params( datetime: i64, params: DateParameters, - offset: Option, + hooks: &dyn HostHooks, ) -> Option { let datetime = NaiveDateTime::from_timestamp_millis(datetime)?; let DateParameters { @@ -149,8 +149,8 @@ pub(super) fn replace_params( millisecond, } = params; - let datetime = if let Some(offset) = offset { - offset.from_utc_datetime(&datetime).naive_local() + let datetime = if LOCAL { + hooks.local_from_utc(datetime).naive_local() } else { datetime }; @@ -188,9 +188,9 @@ pub(super) fn replace_params( let new_time = make_time(hour, minute, second, millisecond)?; let mut ts = make_date(new_day, new_time)?; - if let Some(offset) = offset { - ts = offset - .from_local_datetime(&NaiveDateTime::from_timestamp_millis(ts)?) + if LOCAL { + ts = hooks + .local_from_naive_local(NaiveDateTime::from_timestamp_millis(ts)?) .earliest()? .naive_utc() .timestamp_millis(); diff --git a/boa_engine/src/context/hooks.rs b/boa_engine/src/context/hooks.rs index d3479bba6e..34024854de 100644 --- a/boa_engine/src/context/hooks.rs +++ b/boa_engine/src/context/hooks.rs @@ -5,7 +5,7 @@ use crate::{ realm::Realm, Context, JsResult, JsValue, }; -use chrono::{FixedOffset, Local, NaiveDateTime, Utc}; +use chrono::{DateTime, FixedOffset, Local, LocalResult, NaiveDateTime, TimeZone, Utc}; use super::intrinsics::Intrinsics; @@ -182,14 +182,27 @@ pub trait HostHooks { Utc::now().naive_utc() } - /// Gets the current timezone as a fixed offset from UTC. + /// Converts the naive datetime `utc` to the corresponding local datetime. /// - /// Defaults to using [`Local::now`] on all targets, which can cause panics if your platform + /// Defaults to using [`Local`] on all targets, which can cause panics if your platform /// doesn't support [`SystemTime::now`][time]. /// /// [time]: std::time::SystemTime::now - fn tz_offset(&self) -> FixedOffset { - *Local::now().offset() + fn local_from_utc(&self, utc: NaiveDateTime) -> DateTime { + let offset = Local.offset_from_utc_datetime(&utc); + DateTime::from_utc(utc, offset) + } + + /// Converts the naive local datetime `local` to a local timezone datetime. + /// + /// Defaults to using [`Local`] on all targets, which can cause panics if your platform + /// doesn't support [`SystemTime::now`][time]. + /// + /// [time]: std::time::SystemTime::now + fn local_from_naive_local(&self, local: NaiveDateTime) -> LocalResult> { + Local + .offset_from_local_datetime(&local) + .map(|offset| DateTime::from_local(local, offset)) } }