validate tag timestamp (#3703)

* feat: Add validation to timestamp and tag fields Timestamp and tag fields shouldnt start or end with dot Todo: Implement tests * ensure we validate escaped string * add test for tag validation * fix linter * Endwith dont need to be checked with string literal `//.` and `.` should end in the same way Also address review comment about `['//.']` -> `"//."` --------- Co-authored-by: sravan <[email protected]> Co-authored-by: Paul Masurel <[email protected]>
quickwit-oss · Aug 7, 2023 · 751fa77 · 751fa77
1 parent afc81b0
commit 751fa77
Showing 1 changed file with 126 additions and 0 deletions.
diff --git a/quickwit/quickwit-doc-mapper/src/default_doc_mapper/default_mapper.rs b/quickwit/quickwit-doc-mapper/src/default_doc_mapper/default_mapper.rs
@@ -116,6 +116,12 @@ fn validate_timestamp_field(
     timestamp_field_path: &str,
     mapping_root_node: &MappingNode,
 ) -> anyhow::Result<()> {
+    if timestamp_field_path.starts_with('.') || timestamp_field_path.starts_with("\\.") {
+        bail!("Timestamp field `{timestamp_field_path}` should not start with a `.`.");
+    }
+    if timestamp_field_path.ends_with('.') {
+        bail!("Timestamp field `{timestamp_field_path}` should not end with a `.`.");
+    }
     let Some(timestamp_field_type) =
         mapping_root_node.find_field_mapping_type(timestamp_field_path)
     else {
@@ -262,6 +268,12 @@ impl TryFrom<DefaultDocMapperBuilder> for DefaultDocMapper {
 /// - if str, the field must use the `raw` tokenizer for indexing.
 /// - the field must be indexed.
 fn validate_tag(tag_field_name: &str, schema: &Schema) -> Result<(), anyhow::Error> {
+    if tag_field_name.starts_with('.') || tag_field_name.starts_with("\\.") {
+        bail!("Tag field `{tag_field_name}` should not start with a `.`.");
+    }
+    if tag_field_name.ends_with('.') {
+        bail!("Tag field `{tag_field_name}` should not end with a `.`.");
+    }
     let field = schema
         .get_field(tag_field_name)
         .with_context(|| format!("Unknown tag field: `{tag_field_name}`"))?;
@@ -821,6 +833,120 @@ mod tests {
         .unwrap();
     }
 
+    #[test]
+    fn test_timestamp_field_that_start_with_dot_is_invalid() {
+        assert_eq!(
+            serde_json::from_str::<DefaultDocMapper>(
+                r#"{
+                "field_mappings": [
+                    {
+                        "name": "my.timestamp",
+                        "type": "datetime",
+                        "fast": true
+                    }
+                ],
+                "timestamp_field": ".my.timestamp"
+            }"#,
+            )
+            .unwrap_err()
+            .to_string(),
+            "Timestamp field `.my.timestamp` should not start with a `.`.",
+        );
+
+        assert_eq!(
+            serde_json::from_str::<DefaultDocMapper>(
+                r#"{
+                "field_mappings": [
+                    {
+                        "name": "my.timestamp",
+                        "type": "datetime",
+                        "fast": true
+                    }
+                ],
+                "timestamp_field": "\\.my\\.timestamp"
+            }"#,
+            )
+            .unwrap_err()
+            .to_string(),
+            "Timestamp field `\\.my\\.timestamp` should not start with a `.`.",
+        )
+    }
+
+    #[test]
+    fn test_timestamp_field_that_ends_with_dot_is_invalid() {
+        assert_eq!(
+            serde_json::from_str::<DefaultDocMapper>(
+                r#"{
+                    "timestamp_field": "my.timestamp."
+                }"#,
+            )
+            .unwrap_err()
+            .to_string(),
+            "Timestamp field `my.timestamp.` should not end with a `.`.",
+        );
+
+        assert_eq!(
+            serde_json::from_str::<DefaultDocMapper>(
+                r#"{
+                    "timestamp_field": "my\\.timestamp\\."
+                }"#,
+            )
+            .unwrap_err()
+            .to_string(),
+            "Timestamp field `my\\.timestamp\\.` should not end with a `.`.",
+        )
+    }
+
+    #[test]
+    fn test_tag_field_name_that_starts_with_dot_is_invalid() {
+        assert_eq!(
+            serde_json::from_str::<DefaultDocMapper>(
+                r#"{
+                    "tag_fields": [".my.tag"]
+                }"#,
+            )
+            .unwrap_err()
+            .to_string(),
+            "Tag field `.my.tag` should not start with a `.`.",
+        );
+
+        assert_eq!(
+            serde_json::from_str::<DefaultDocMapper>(
+                r#"{
+                    "tag_fields": ["\\.my\\.tag"]
+                }"#,
+            )
+            .unwrap_err()
+            .to_string(),
+            "Tag field `\\.my\\.tag` should not start with a `.`.",
+        )
+    }
+
+    #[test]
+    fn test_tag_field_name_that_ends_with_dot_is_invalid() {
+        assert_eq!(
+            serde_json::from_str::<DefaultDocMapper>(
+                r#"{
+                    "tag_fields": ["my.tag."]
+                }"#,
+            )
+            .unwrap_err()
+            .to_string(),
+            "Tag field `my.tag.` should not end with a `.`.",
+        );
+
+        assert_eq!(
+            serde_json::from_str::<DefaultDocMapper>(
+                r#"{
+                    "tag_fields": ["my\\.tag\\."]
+                }"#,
+            )
+            .unwrap_err()
+            .to_string(),
+            "Tag field `my\\.tag\\.` should not end with a `.`.",
+        )
+    }
+
     #[test]
     fn test_fail_to_build_doc_mapper_with_timestamp_field_with_multivalues_cardinality() {
         let doc_mapper = r#"{