From 72866e217e3927c2097854832b3c883dc402f9b6 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sun, 23 Apr 2023 17:27:03 +0000 Subject: [PATCH] Allow `Date` object to store invalid `NativeDateTime` (#2861) Part of ES5. This PR allows `Date` objects to store an invalid `NativeDateTime` as a `i64` and check when `getMinutes` (or other methods) call and check if it's valid, like in the spec This was failing some `Date` tests, when calling `new Date(8.64e15)` then calling `getTime()` it was returning `NaN` when it should return the passed value `8640000000000000` (as node does) --- boa_engine/src/builtins/date/mod.rs | 149 +++++++++++++---------- boa_engine/src/builtins/date/utils.rs | 7 +- boa_engine/src/object/builtins/jsdate.rs | 2 +- 3 files changed, 88 insertions(+), 70 deletions(-) diff --git a/boa_engine/src/builtins/date/mod.rs b/boa_engine/src/builtins/date/mod.rs index c083a0e1c9..764b5b83a4 100644 --- a/boa_engine/src/builtins/date/mod.rs +++ b/boa_engine/src/builtins/date/mod.rs @@ -61,7 +61,7 @@ macro_rules! get_mut_date { /// Abstract operation [`thisTimeValue`][spec]. /// /// [spec]: https://tc39.es/ecma262/#sec-thistimevalue -pub(super) fn this_time_value(value: &JsValue) -> JsResult> { +pub(super) fn this_time_value(value: &JsValue) -> JsResult> { Ok(value .as_object() .and_then(|obj| obj.borrow().as_date().copied()) @@ -71,24 +71,24 @@ pub(super) fn this_time_value(value: &JsValue) -> JsResult /// The internal representation of a `Date` object. #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct Date(Option); +pub struct Date(Option); impl Date { /// Creates a new `Date`. - pub(crate) const fn new(dt: Option) -> Self { + pub(crate) const fn new(dt: Option) -> Self { Self(dt) } /// Creates a new `Date` from the current UTC time of the host. pub(crate) fn utc_now(hooks: &dyn HostHooks) -> Self { - Self(Some(hooks.utc_now())) + let dt = hooks.utc_now(); + Self(Some(dt.timestamp_millis())) } /// Converts the `Date` into a `JsValue`, mapping `None` to `NaN` and `Some(datetime)` to /// `JsValue::from(datetime.timestamp_millis())`. fn as_value(&self) -> JsValue { - self.0 - .map_or_else(|| f64::NAN.into(), |dt| dt.timestamp_millis().into()) + self.0.map_or_else(|| f64::NAN.into(), Into::into) } } @@ -243,26 +243,25 @@ impl BuiltInConstructor for Date { // 1. Assert: The next step never returns an abrupt completion because v is a String. // 2. Let tv be the result of parsing v as a date, in exactly the same manner as for the // parse method (21.4.3.2). - Self( - str.to_std_string() - .ok() - .and_then(|s| { - chrono::DateTime::parse_from_rfc3339(s.as_str()).ok() - }) - .map(|dt| dt.naive_utc()), - ) + + let dt = str + .to_std_string() + .ok() + .and_then(|s| chrono::DateTime::parse_from_rfc3339(s.as_str()).ok()) + .map(|dt| dt.naive_utc()); + Self::new(dt.map(|dt| dt.timestamp_millis())) } // iii. Else, v => { // Directly convert to integer // 1. Let tv be ? ToNumber(v). - Self( - v.to_integer_or_nan(context)? - .as_integer() - // d. Let dv be TimeClip(tv). - .and_then(time_clip) - .and_then(NaiveDateTime::from_timestamp_millis), - ) + + let dt = v + .to_integer_or_nan(context)? + .as_integer() + // d. Let dv be TimeClip(tv). + .and_then(time_clip); + Self(dt) } } } @@ -270,17 +269,18 @@ impl BuiltInConstructor for Date { // 5. Else, _ => { // Separating this into its own function to simplify the logic. - Self( - Self::construct_date(args, context)? - .and_then(|dt| { - context - .host_hooks() - .tz_offset() - .from_local_datetime(&dt) - .earliest() - }) - .map(|dt| dt.naive_utc()), - ) + + 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()); + + Self(dt.map(|dt| dt.timestamp_millis())) } }; @@ -464,8 +464,10 @@ impl Date { context: &mut Context<'_>, ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). + let t = this_time_value(this)?; + // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(this_time_value(this)?); + let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); if LOCAL { t = context .host_hooks() @@ -492,8 +494,10 @@ impl Date { context: &mut Context<'_>, ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). + let t = this_time_value(this)?; + // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(this_time_value(this)?); + let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); if LOCAL { t = context .host_hooks() @@ -523,8 +527,10 @@ impl Date { context: &mut Context<'_>, ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). + let t = this_time_value(this)?; + // 2. If t is NaN, return NaN. - let t = some_or_nan!(this_time_value(this)?); + 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); @@ -544,8 +550,10 @@ impl Date { context: &mut Context<'_>, ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). + let t = this_time_value(this)?; + // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(this_time_value(this)?); + let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); if LOCAL { t = context .host_hooks() @@ -571,8 +579,10 @@ impl Date { context: &mut Context<'_>, ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). + let t = this_time_value(this)?; + // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(this_time_value(this)?); + let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); if LOCAL { t = context .host_hooks() @@ -598,8 +608,10 @@ impl Date { context: &mut Context<'_>, ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). + let t = this_time_value(this)?; + // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(this_time_value(this)?); + let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); if LOCAL { t = context .host_hooks() @@ -625,8 +637,10 @@ impl Date { context: &mut Context<'_>, ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). + let t = this_time_value(this)?; + // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(this_time_value(this)?); + let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); if LOCAL { t = context .host_hooks() @@ -653,8 +667,10 @@ impl Date { context: &mut Context<'_>, ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). + let t = this_time_value(this)?; + // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(this_time_value(this)?); + let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); if LOCAL { t = context .host_hooks() @@ -680,8 +696,10 @@ impl Date { context: &mut Context<'_>, ) -> JsResult { // 1. Let t be ? thisTimeValue(this value). + let t = this_time_value(this)?; + // 2. If t is NaN, return NaN. - let mut t = some_or_nan!(this_time_value(this)?); + let mut t = some_or_nan!(t.and_then(NaiveDateTime::from_timestamp_millis)); if LOCAL { t = context .host_hooks() @@ -710,8 +728,7 @@ impl Date { _context: &mut Context<'_>, ) -> JsResult { // 1. Return ? thisTimeValue(this value). - let t = some_or_nan!(this_time_value(this)?); - Ok(JsValue::from(t.timestamp_millis())) + Ok(this_time_value(this)?.map_or(JsValue::nan(), JsValue::from)) } /// `Date.prototype.getTimeZoneOffset()`. @@ -775,7 +792,7 @@ impl Date { ); // 7. Set the [[DateValue]] internal slot of this Date object to u. - *t = Self(datetime); + *t = Self::new(datetime); // 8. Return u. Ok(t.as_value()) @@ -798,7 +815,7 @@ 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 { + 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() @@ -806,7 +823,7 @@ impl Date { .earliest() .as_ref() .map(DateTime::naive_utc) else { - *t = Self(None); + *t = Self::new(None); return Ok(t.as_value()) }; datetime @@ -832,7 +849,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( - datetime, + datetime.timestamp_millis(), DateParameters { year: Some(year), month, @@ -843,7 +860,7 @@ impl Date { ); // 8. Set the [[DateValue]] internal slot of this Date object to u. - *t = Self(datetime); + *t = Self::new(datetime); // 9. Return u. Ok(t.as_value()) @@ -909,7 +926,7 @@ impl Date { ); // 13. Set the [[DateValue]] internal slot of this Date object to u. - *t = Self(datetime); + *t = Self::new(datetime); // 14. Return u. Ok(t.as_value()) @@ -950,7 +967,7 @@ impl Date { ); // 7. Set the [[DateValue]] internal slot of this Date object to u. - *t = Self(datetime); + *t = Self::new(datetime); // 8. Return u. Ok(t.as_value()) @@ -1006,7 +1023,7 @@ impl Date { ); // 11. Set the [[DateValue]] internal slot of this Date object to u. - *t = Self(datetime); + *t = Self::new(datetime); // 12. Return u. Ok(t.as_value()) @@ -1055,7 +1072,7 @@ impl Date { ); // 9. Set the [[DateValue]] internal slot of this Date object to u. - *t = Self(datetime); + *t = Self::new(datetime); // 10. Return u. Ok(t.as_value()) @@ -1103,7 +1120,7 @@ impl Date { ); // 9. Set the [[DateValue]] internal slot of this Date object to u. - *t = Self(datetime); + *t = Self::new(datetime); // 10. Return u. Ok(t.as_value()) @@ -1136,21 +1153,21 @@ 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.or_else(|| { + 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(None); + *t = Self::new(None); return Ok(t.as_value()); }; // 4. If y is NaN, then let Some(mut year) = year.as_integer() else { // a. Set the [[DateValue]] internal slot of this Date object to NaN. - *t = Self(None); + *t = Self::new(None); // b. Return NaN. return Ok(t.as_value()); @@ -1165,7 +1182,7 @@ 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( - datetime, + datetime.timestamp_millis(), DateParameters { year: Some(IntegerOrNan::Integer(year)), ..Default::default() @@ -1174,7 +1191,7 @@ impl Date { ); // 10. Set the [[DateValue]] internal slot of this Date object to TimeClip(date). - *t = Self(datetime); + *t = Self::new(datetime); // 11. Return the value of the [[DateValue]] internal slot of this Date object. Ok(t.as_value()) @@ -1204,11 +1221,10 @@ impl Date { .get_or_undefined(0) .to_integer_or_nan(context)? .as_integer() - .and_then(time_clip) - .and_then(NaiveDateTime::from_timestamp_millis); + .and_then(time_clip); // 4. Set the [[DateValue]] internal slot of this Date object to v. - *t = Self(timestamp); + *t = Self::new(timestamp); // 5. Return v. Ok(t.as_value()) @@ -1230,7 +1246,7 @@ impl Date { ) -> JsResult { // 1. Let O be this Date object. // 2. Let tv be ? thisTimeValue(O). - let Some(tv) = this_time_value(this)? 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()); }; @@ -1263,6 +1279,7 @@ impl Date { _: &mut Context<'_>, ) -> JsResult { let t = this_time_value(this)? + .and_then(NaiveDateTime::from_timestamp_millis) .ok_or_else(|| JsNativeError::range().with_message("Invalid time value"))?; Ok(Utc .from_utc_datetime(&t) @@ -1376,7 +1393,7 @@ impl Date { ) -> JsResult { // 1. Let tv be ? thisTimeValue(this value). // 2. Return ToDateString(tv). - let Some(t) = this_time_value(this)? else { + let Some(t) = this_time_value(this)?.and_then(NaiveDateTime::from_timestamp_millis) else { return Ok(js_string!("Invalid Date").into()); }; Ok(context @@ -1405,7 +1422,7 @@ impl Date { ) -> JsResult { // 1. Let O be this Date object. // 2. Let tv be ? thisTimeValue(O). - let Some(t) = this_time_value(this)? else { + let Some(t) = 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()); }; @@ -1436,7 +1453,7 @@ impl Date { _context: &mut Context<'_>, ) -> JsResult { // 1. Let O be this Date object. - let Some(t) = this_time_value(this)? else { + let Some(t) = 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()) }; @@ -1470,7 +1487,7 @@ impl Date { _context: &mut Context<'_>, ) -> JsResult { // 1. Return ?Ā thisTimeValue(this value). - Ok(Self(this_time_value(this)?).as_value()) + Ok(Self::new(this_time_value(this)?).as_value()) } /// [`Date.prototype [ @@toPrimitive ] ( hint )`][spec]. diff --git a/boa_engine/src/builtins/date/utils.rs b/boa_engine/src/builtins/date/utils.rs index 4803fcb788..db547a34c4 100644 --- a/boa_engine/src/builtins/date/utils.rs +++ b/boa_engine/src/builtins/date/utils.rs @@ -134,10 +134,11 @@ pub(super) struct DateParameters { /// Replaces some (or all) parameters of `date` with the specified parameters pub(super) fn replace_params( - datetime: NaiveDateTime, + datetime: i64, params: DateParameters, offset: Option, -) -> Option { +) -> Option { + let datetime = NaiveDateTime::from_timestamp_millis(datetime)?; let DateParameters { year, month, @@ -195,5 +196,5 @@ pub(super) fn replace_params( .timestamp_millis(); } - NaiveDateTime::from_timestamp_millis(time_clip(ts)?) + time_clip(ts) } diff --git a/boa_engine/src/object/builtins/jsdate.rs b/boa_engine/src/object/builtins/jsdate.rs index f6297cc308..e6551a0750 100644 --- a/boa_engine/src/object/builtins/jsdate.rs +++ b/boa_engine/src/object/builtins/jsdate.rs @@ -571,7 +571,7 @@ impl JsDate { .map_err(|_| JsNativeError::typ().with_message("unpaired surrogate on date string"))?; let date_time = DateTime::parse_from_rfc3339(&string) .map_err(|err| JsNativeError::typ().with_message(err.to_string()))?; - let date_time = Date::new(Some(date_time.naive_local())); + let date_time = Date::new(Some(date_time.naive_local().timestamp_millis())); Ok(Self { inner: JsObject::from_proto_and_data(prototype, ObjectData::date(date_time)),