Repository: sharkdp/numbat (rust)

Issue:
Unexpected panic when using datetime formatter
While experimenting with datetime formatting, I accidentally entered a bad format and caused a panic. Here it is with standard backtrace:

```
>>> format_datetime("%Y-%m-%dT%H%:M", now())
thread 'main' panicked at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/alloc/src/string.rs:2611:14:
a Display implementation returned an error unexpectedly: Error
stack backtrace:
   0: rust_begin_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/result.rs:1649:5
   3: numbat::ffi::format_datetime
   4: numbat::vm::Vm::run_without_cleanup
   5: numbat::vm::Vm::run
   6: <numbat::bytecode_interpreter::BytecodeInterpreter as numbat::interpreter::Interpreter>::interpret_statements
   7: numbat::Context::interpret_with_settings
   8: numbat::Cli::parse_and_evaluate
   9: numbat::Cli::run
  10: numbat::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
```

This should probably not leak (and certainly shouldn't crash the program). I also saw some unrelated panics in WASM, which is more annoying to debug because you can't easily see the backtrace. 🙂 Unfortunately, I didn't note down what caused them.

Pull request:
Runtime error for wrong format specifiers
Unfortunately, I didn't find a way to get a more detailed error message out of chrono.

Closes #377 and fixes a panic when rendering large dates in RFC2822

Patch:
diff --git a/numbat/src/datetime.rs b/numbat/src/datetime.rs
index facd4a2..7775273 100644
--- a/numbat/src/datetime.rs
+++ b/numbat/src/datetime.rs
@@ -1,4 +1,4 @@
-use chrono::{DateTime, FixedOffset, LocalResult, Offset, ParseError, TimeZone};
+use chrono::{DateTime, Datelike, FixedOffset, LocalResult, Offset, TimeZone};
 use chrono_tz::Tz;
 
 pub fn get_local_timezone() -> Option<Tz> {
@@ -25,11 +25,11 @@ pub fn local_offset_for_datetime<O: TimeZone>(dt: &DateTime<O>) -> FixedOffset {
         .unwrap_or_else(|| dt.offset().fix())
 }
 
-pub fn parse_datetime(input: &str) -> Result<Option<DateTime<FixedOffset>>, ParseError> {
+pub fn parse_datetime(input: &str) -> Option<DateTime<FixedOffset>> {
     if let Ok(dt) = chrono::DateTime::parse_from_rfc3339(input) {
-        Ok(Some(dt))
+        Some(dt)
     } else if let Ok(dt) = chrono::DateTime::parse_from_rfc2822(input) {
-        Ok(Some(dt))
+        Some(dt)
     } else {
         const FORMATS: [&str; 8] = [
             // 24 hour formats:
@@ -47,7 +47,7 @@ pub fn parse_datetime(input: &str) -> Result<Option<DateTime<FixedOffset>>, Pars
         for format in FORMATS {
             // Try to match the given format plus an additional UTC offset (%z)
             if let Ok(dt) = chrono::DateTime::parse_from_str(input, &format!("{format} %z")) {
-                return Ok(Some(dt));
+                return Some(dt);
             }
 
             // Try to match the given format plus an additional timezone name (%Z).
@@ -62,7 +62,7 @@ pub fn parse_datetime(input: &str) -> Result<Option<DateTime<FixedOffset>>, Pars
                 if let Ok(tz) = potential_timezone_name.parse::<Tz>() {
                     if let Ok(ndt) = chrono::NaiveDateTime::parse_from_str(rest, format) {
                         if let LocalResult::Single(dt) = ndt.and_local_timezone(tz) {
-                            return Ok(Some(dt.with_timezone(&local_offset_for_datetime(&dt))));
+                            return Some(dt.with_timezone(&local_offset_for_datetime(&dt)));
                         }
                     }
                 }
@@ -73,11 +73,19 @@ pub fn parse_datetime(input: &str) -> Result<Option<DateTime<FixedOffset>>, Pars
                 if let LocalResult::Single(dt) =
                     ndt.and_local_timezone(get_local_timezone().unwrap_or(chrono_tz::UTC))
                 {
-                    return Ok(Some(dt.with_timezone(&local_offset_for_datetime(&dt))));
+                    return Some(dt.with_timezone(&local_offset_for_datetime(&dt)));
                 }
             }
         }
 
-        Ok(None)
+        None
+    }
+}
+
+pub fn to_rfc2822_save(dt: &DateTime<FixedOffset>) -> String {
+    if dt.year() < 0 || dt.year() > 9999 {
+        "<year out of range for displaying in RFC2822>".to_string()
+    } else {
+        dt.to_rfc2822()
     }
 }
diff --git a/numbat/src/ffi.rs b/numbat/src/ffi.rs
index d6a51bb..cc3436b 100644
--- a/numbat/src/ffi.rs
+++ b/numbat/src/ffi.rs
@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::fmt::Write;
 
 use std::sync::OnceLock;
 
@@ -844,7 +845,6 @@ fn datetime(args: &[Value]) -> Result<Value> {
     let input = args[0].unsafe_as_string();
 
     let output = datetime::parse_datetime(input)
-        .map_err(RuntimeError::DateParsingError)?
         .ok_or(RuntimeError::DateParsingErrorUnknown)?
         .fixed_offset();
 
@@ -857,7 +857,8 @@ fn format_datetime(args: &[Value]) -> Result<Value> {
     let format = args[0].unsafe_as_string();
     let dt = args[1].unsafe_as_datetime();
 
-    let output = dt.format(format).to_string();
+    let mut output = String::new();
+    write!(output, "{}", dt.format(format)).map_err(|_| RuntimeError::DateFormattingError)?;
 
     Ok(Value::String(output))
 }
diff --git a/numbat/src/interpreter.rs b/numbat/src/interpreter.rs
index 411064d..f04f6fa 100644
--- a/numbat/src/interpreter.rs
+++ b/numbat/src/interpreter.rs
@@ -37,8 +37,6 @@ pub enum RuntimeError {
     CouldNotLoadExchangeRates,
     #[error("User error: {0}")]
     UserError(String),
-    #[error("Could not parse date: {0}")]
-    DateParsingError(chrono::ParseError),
     #[error("Unrecognized datetime format")]
     DateParsingErrorUnknown,
     #[error("Unknown timezone: {0}")]
@@ -47,6 +45,8 @@ pub enum RuntimeError {
     DurationOutOfRange,
     #[error("DateTime out of range")]
     DateTimeOutOfRange,
+    #[error("Error in datetime format. See https://docs.rs/chrono/latest/chrono/format/strftime/index.html for possible format specifiers.")]
+    DateFormattingError,
 }
 
 #[derive(Debug, PartialEq, Eq)]
diff --git a/numbat/src/value.rs b/numbat/src/value.rs
index c1e461b..1d172b9 100644
--- a/numbat/src/value.rs
+++ b/numbat/src/value.rs
@@ -95,7 +95,7 @@ impl PrettyPrint for Value {
             Value::Quantity(q) => q.pretty_print(),
             Value::Boolean(b) => b.pretty_print(),
             Value::String(s) => s.pretty_print(),
-            Value::DateTime(dt) => crate::markup::string(dt.to_rfc2822()),
+            Value::DateTime(dt) => crate::markup::string(crate::datetime::to_rfc2822_save(dt)),
             Value::FunctionReference(r) => crate::markup::string(r.to_string()),
         }
     }
diff --git a/numbat/src/vm.rs b/numbat/src/vm.rs
index 8f5cb7e..da66170 100644
--- a/numbat/src/vm.rs
+++ b/numbat/src/vm.rs
@@ -866,7 +866,7 @@ impl Vm {
                             Value::Quantity(q) => q.to_string(),
                             Value::Boolean(b) => b.to_string(),
                             Value::String(s) => s,
-                            Value::DateTime(dt) => dt.to_rfc2822(),
+                            Value::DateTime(dt) => crate::datetime::to_rfc2822_save(&dt),
                             Value::FunctionReference(r) => r.to_string(),
                         };
                         joined = part + &joined; // reverse order


Test patch:
diff --git a/numbat/tests/interpreter.rs b/numbat/tests/interpreter.rs
index 352698a..0c4bd8e 100644
--- a/numbat/tests/interpreter.rs
+++ b/numbat/tests/interpreter.rs
@@ -600,3 +600,21 @@ fn test_pretty_print_prefixes() {
 fn test_full_simplify_for_function_calls() {
     expect_output("floor(1.2 hours / hour)", "1");
 }
+
+#[test]
+fn test_datetime_runtime_errors() {
+    expect_failure("datetime(\"2000-01-99\")", "Unrecognized datetime format");
+    expect_failure("now() -> tz(\"Europe/NonExisting\")", "Unknown timezone");
+    expect_failure(
+        "date(\"2000-01-01\") + 1e100 years",
+        "Exceeded maximum size for time durations",
+    );
+    expect_failure(
+        "date(\"2000-01-01\") + 100000000 years",
+        "DateTime out of range",
+    );
+    expect_failure(
+        "format_datetime(\"%Y-%m-%dT%H%:M\", now())",
+        "Error in datetime format",
+    )
+}