From ab9b8109d94a6768460ce5a6c03cd9b713dbfb07 Mon Sep 17 00:00:00 2001 From: Pavlos Rontidis Date: Tue, 6 Aug 2024 12:52:52 -0400 Subject: [PATCH 1/3] fix(timestamp conversion): use timestamp_nanos_opt to avoid panics when out of range --- src/compiler/value/error.rs | 8 ++++++-- src/stdlib/to_float.rs | 10 +++++++--- src/stdlib/to_unix_timestamp.rs | 12 +++++++++++- src/stdlib/uuid_v7.rs | 17 ++++++++++------- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/compiler/value/error.rs b/src/compiler/value/error.rs index eab3bf0047..5f287d1a5d 100644 --- a/src/compiler/value/error.rs +++ b/src/compiler/value/error.rs @@ -1,7 +1,7 @@ +use super::Kind; use crate::compiler::ExpressionError; use crate::diagnostic::DiagnosticMessage; - -use super::Kind; +use crate::prelude::ValueError::OutOfRange; #[derive(thiserror::Error, Debug, PartialEq, Eq)] pub enum ValueError { @@ -55,6 +55,9 @@ pub enum ValueError { #[error("can't merge type {1} into {0}")] Merge(Kind, Kind), + + #[error("can't convert {0}")] + OutOfRange(Kind), } impl DiagnosticMessage for ValueError { @@ -81,6 +84,7 @@ impl DiagnosticMessage for ValueError { Lt(..) => 313, Le(..) => 314, Merge(..) => 315, + OutOfRange(..) => 316, } } } diff --git a/src/stdlib/to_float.rs b/src/stdlib/to_float.rs index cd1148bb55..49a24eeaaf 100644 --- a/src/stdlib/to_float.rs +++ b/src/stdlib/to_float.rs @@ -8,9 +8,13 @@ fn to_float(value: Value) -> Resolved { Integer(v) => Ok(Value::from_f64_or_zero(v as f64)), Boolean(v) => Ok(NotNan::new(if v { 1.0 } else { 0.0 }).unwrap().into()), Null => Ok(NotNan::new(0.0).unwrap().into()), - Timestamp(v) => Ok(Value::from_f64_or_zero( - v.timestamp_nanos() as f64 / 1_000_000_000_f64, - )), + Timestamp(v) => { + let nanos = match v.timestamp_nanos_opt() { + Some(nanos) => nanos as f64, + None => return Err(ValueError::OutOfRange(Kind::timestamp()).into()), + }; + Ok(Value::from_f64_or_zero(nanos / 1_000_000_000_f64)) + } Bytes(v) => Conversion::Float .convert(v) .map_err(|e| e.to_string().into()), diff --git a/src/stdlib/to_unix_timestamp.rs b/src/stdlib/to_unix_timestamp.rs index a75ddba8c4..74c59382b4 100644 --- a/src/stdlib/to_unix_timestamp.rs +++ b/src/stdlib/to_unix_timestamp.rs @@ -7,7 +7,10 @@ fn to_unix_timestamp(value: Value, unit: Unit) -> Resolved { Unit::Seconds => ts.timestamp(), Unit::Milliseconds => ts.timestamp_millis(), Unit::Microseconds => ts.timestamp_micros(), - Unit::Nanoseconds => ts.timestamp_nanos(), + Unit::Nanoseconds => match ts.timestamp_nanos_opt() { + None => return Err(ValueError::OutOfRange(Kind::timestamp()).into()), + Some(nanos) => nanos, + }, }; Ok(time.into()) } @@ -186,5 +189,12 @@ mod test { want: Ok(1_609_459_200_000_000_000_i64), tdef: TypeDef::integer().infallible(), } + crash { + args: func_args![value: chrono::Utc.ymd(0, 1, 1).and_hms_milli(0, 0, 0, 0), + unit: "nanoseconds" + ], + want: Err("can't convert timestamp"), + tdef: TypeDef::integer().infallible(), + } ]; } diff --git a/src/stdlib/uuid_v7.rs b/src/stdlib/uuid_v7.rs index 6c02f0aee3..871d5dbade 100644 --- a/src/stdlib/uuid_v7.rs +++ b/src/stdlib/uuid_v7.rs @@ -3,22 +3,25 @@ use bytes::Bytes; use chrono::{DateTime, Utc}; use uuid::{timestamp::Timestamp, NoContext}; -fn uuid_v7(timestamp: Option) -> Value { - let timestamp: DateTime = if let Some(timestamp) = timestamp { - timestamp.try_timestamp().unwrap() +fn uuid_v7(timestamp: Option) -> Resolved { + let utc_timestamp: DateTime = if let Some(timestamp) = timestamp { + timestamp.try_timestamp()? } else { Utc::now() }; - let seconds = timestamp.timestamp() as u64; - let nanoseconds = timestamp.timestamp_nanos() as u32; + let seconds = utc_timestamp.timestamp() as u64; + let nanoseconds = match utc_timestamp.timestamp_nanos_opt() { + Some(nanos) => nanos as u32, + None => return Err(ValueError::OutOfRange(Kind::timestamp()).into()), + }; let timestamp = Timestamp::from_unix(NoContext, seconds, nanoseconds); let mut buffer = [0; 36]; let uuid = uuid::Uuid::new_v7(timestamp) .hyphenated() .encode_lower(&mut buffer); - Bytes::copy_from_slice(uuid.as_bytes()).into() + Ok(Bytes::copy_from_slice(uuid.as_bytes()).into()) } #[derive(Clone, Copy, Debug)] @@ -77,7 +80,7 @@ impl FunctionExpression for UuidV7Fn { .map(|m| m.resolve(ctx)) .transpose()?; - Ok(uuid_v7(timestamp)) + uuid_v7(timestamp) } fn type_def(&self, _: &TypeState) -> TypeDef { From f253ce8cbb41b7f2be8862eb12c02ba352ae48c6 Mon Sep 17 00:00:00 2001 From: Pavlos Rontidis Date: Tue, 6 Aug 2024 16:42:59 -0400 Subject: [PATCH 2/3] changelog --- changelog.d/979.fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/979.fix.md diff --git a/changelog.d/979.fix.md b/changelog.d/979.fix.md new file mode 100644 index 0000000000..b327020126 --- /dev/null +++ b/changelog.d/979.fix.md @@ -0,0 +1 @@ +Replaced all usages of `timestamp_nanos` with `timestamp_nanos_opt` to avoid panics when the timestamp is out of range. From e71bd86efb98a5ddfbae73a3b42806c17956337b Mon Sep 17 00:00:00 2001 From: Pavlos Rontidis Date: Tue, 6 Aug 2024 16:47:11 -0400 Subject: [PATCH 3/3] cleanup --- src/compiler/value/error.rs | 2 +- src/stdlib/to_float.rs | 4 ++-- src/stdlib/to_unix_timestamp.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/compiler/value/error.rs b/src/compiler/value/error.rs index 5f287d1a5d..6d4021dbec 100644 --- a/src/compiler/value/error.rs +++ b/src/compiler/value/error.rs @@ -56,7 +56,7 @@ pub enum ValueError { #[error("can't merge type {1} into {0}")] Merge(Kind, Kind), - #[error("can't convert {0}")] + #[error("can't convert out of range {0}")] OutOfRange(Kind), } diff --git a/src/stdlib/to_float.rs b/src/stdlib/to_float.rs index 49a24eeaaf..19819d6565 100644 --- a/src/stdlib/to_float.rs +++ b/src/stdlib/to_float.rs @@ -9,11 +9,11 @@ fn to_float(value: Value) -> Resolved { Boolean(v) => Ok(NotNan::new(if v { 1.0 } else { 0.0 }).unwrap().into()), Null => Ok(NotNan::new(0.0).unwrap().into()), Timestamp(v) => { - let nanos = match v.timestamp_nanos_opt() { + let nanoseconds = match v.timestamp_nanos_opt() { Some(nanos) => nanos as f64, None => return Err(ValueError::OutOfRange(Kind::timestamp()).into()), }; - Ok(Value::from_f64_or_zero(nanos / 1_000_000_000_f64)) + Ok(Value::from_f64_or_zero(nanoseconds / 1_000_000_000_f64)) } Bytes(v) => Conversion::Float .convert(v) diff --git a/src/stdlib/to_unix_timestamp.rs b/src/stdlib/to_unix_timestamp.rs index 74c59382b4..8f46d5a2a6 100644 --- a/src/stdlib/to_unix_timestamp.rs +++ b/src/stdlib/to_unix_timestamp.rs @@ -189,11 +189,11 @@ mod test { want: Ok(1_609_459_200_000_000_000_i64), tdef: TypeDef::integer().infallible(), } - crash { + out_of_range { args: func_args![value: chrono::Utc.ymd(0, 1, 1).and_hms_milli(0, 0, 0, 0), unit: "nanoseconds" ], - want: Err("can't convert timestamp"), + want: Err("can't convert out of range timestamp"), tdef: TypeDef::integer().infallible(), } ];