Stricter date parsing. (#3846)

* Stricter date parsing. * Apply suggestions from code review Co-authored-by: Adrien Guillo <[email protected]> --------- Co-authored-by: Adrien Guillo <[email protected]>
quickwit-oss · Oct 22, 2023 · eb7ee89 · eb7ee89
1 parent 87fe8bf
commit eb7ee89
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 5 deletions.
diff --git a/quickwit/Cargo.lock b/quickwit/Cargo.lock
diff --git a/quickwit/quickwit-datetime/Cargo.toml b/quickwit/quickwit-datetime/Cargo.toml
@@ -10,6 +10,7 @@ homepage = "https://quickwit.io/"
 documentation = "https://quickwit.io/docs/"
 
 [dependencies]
+anyhow = { workspace = true }
 itertools = { workspace = true }
 ouroboros = "0.18.0"
 serde = { workspace = true }

diff --git a/quickwit/quickwit-datetime/src/date_time_format.rs b/quickwit/quickwit-datetime/src/date_time_format.rs
@@ -63,12 +63,18 @@ impl StrptimeParser {
     /// Parse a given date according to the datetime format specified during the StrptimeParser
     /// creation. If the date format does not provide a specific a time, the time will be set to
     /// 00:00:00.
-    fn parse_primitive_date_time(
-        &self,
-        date_time_str: &str,
-    ) -> Result<PrimitiveDateTime, time::error::Parse> {
+    fn parse_primitive_date_time(&self, date_time_str: &str) -> anyhow::Result<PrimitiveDateTime> {
         let mut parsed = Parsed::new();
-        parsed.parse_items(date_time_str.as_bytes(), self.borrow_items())?;
+        if !parsed
+            .parse_items(date_time_str.as_bytes(), self.borrow_items())?
+            .is_empty()
+        {
+            anyhow::bail!(
+                "datetime string `{}` does not match format `{}`",
+                date_time_str,
+                self.borrow_strptime_format()
+            );
+        }
         // The parsed datetime contains a date but seems to be missing "time".
         // We complete it artificially with 00:00:00.
         if parsed.hour_24().is_none()
@@ -314,6 +320,8 @@ impl<'de> Deserialize<'de> for DateTimeOutputFormat {
 
 #[cfg(test)]
 mod tests {
+    use time::macros::datetime;
+
     use super::*;
 
     #[test]
@@ -431,4 +439,18 @@ mod tests {
             assert!(error_str.contains(&format!("unknown output format: `{format}`")));
         }
     }
+
+    #[test]
+    fn test_strictly_parse_datetime_format() {
+        let parser = StrptimeParser::from_str("%Y-%m-%d").unwrap();
+        assert_eq!(
+            parser.parse_date_time("2021-01-01").unwrap(),
+            datetime!(2021-01-01 00:00:00 UTC)
+        );
+        let error_str = parser.parse_date_time("2021-01-01TABC").unwrap_err();
+        assert_eq!(
+            error_str,
+            "datetime string `2021-01-01TABC` does not match format `%Y-%m-%d`"
+        );
+    }
 }
diff --git a/quickwit/quickwit-query/src/json_literal.rs b/quickwit/quickwit-query/src/json_literal.rs
@@ -34,6 +34,7 @@ fn get_default_date_time_format() -> &'static [DateTimeInputFormat] {
                 DateTimeInputFormat::Rfc3339,
                 DateTimeInputFormat::Rfc2822,
                 DateTimeInputFormat::Timestamp,
+                DateTimeInputFormat::from_str("%Y-%m-%dT%H:%M:%S").unwrap(),
                 DateTimeInputFormat::from_str("%Y-%m-%d %H:%M:%S.%f").unwrap(),
                 DateTimeInputFormat::from_str("%Y-%m-%d %H:%M:%S").unwrap(),
                 DateTimeInputFormat::from_str("%Y-%m-%d").unwrap(),
@@ -212,6 +213,14 @@ mod tests {
         assert_eq!(dt_opt, Some(DateTime::from_utc(expected_datetime)));
     }
 
+    #[test]
+    fn test_interpret_datetime_rfc3339_with_no_timezone() {
+        let dt_opt =
+            DateTime::interpret_json(&JsonLiteral::String("2023-05-25T18:00:00".to_string()));
+        let expected_datetime = datetime!(2023-05-25 18:00 UTC);
+        assert_eq!(dt_opt, Some(DateTime::from_utc(expected_datetime)));
+    }
+
     #[test]
     fn test_interpret_datetime_fractional_millis() {
         let dt_opt =