fix: avoid parsing search malformed requests (#4175)

fix: avoid parsing search malformed requests
quickwit-oss · Nov 22, 2023 · 9400c4a · 9400c4a
1 parent 3f18626
commit 9400c4a
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 4 deletions.
diff --git a/quickwit/quickwit-query/src/elastic_query_dsl/exists_query.rs b/quickwit/quickwit-query/src/elastic_query_dsl/exists_query.rs
@@ -28,9 +28,28 @@ pub struct ExistsQuery {
 }
 
 impl ConvertableToQueryAst for ExistsQuery {
-    fn convert_to_query_ast(self) -> anyhow::Result<crate::query_ast::QueryAst> {
+    fn convert_to_query_ast(self) -> anyhow::Result<QueryAst> {
         Ok(QueryAst::FieldPresence(query_ast::FieldPresenceQuery {
             field: self.field,
         }))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::elastic_query_dsl::exists_query::ExistsQuery;
+
+    #[test]
+    fn test_dsl_exists_query_deserialize_simple() {
+        let exists_query_json = r#"{
+           "field": "privileged"
+        }"#;
+        let bool_query: ExistsQuery = serde_json::from_str(exists_query_json).unwrap();
+        assert_eq!(
+            &bool_query,
+            &ExistsQuery {
+                field: "privileged".to_string(),
+            }
+        );
+    }
+}
diff --git a/quickwit/quickwit-query/src/elastic_query_dsl/mod.rs b/quickwit/quickwit-query/src/elastic_query_dsl/mod.rs
@@ -55,6 +55,7 @@ fn default_max_expansions() -> u32 {
 }
 
 #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone, Copy, Default)]
+#[serde(deny_unknown_fields)]
 pub(crate) struct MatchAllQuery {
     pub boost: Option<NotNaNf32>,
 }

diff --git a/quickwit/quickwit-serve/src/elastic_search_api/model/multi_search.rs b/quickwit/quickwit-serve/src/elastic_search_api/model/multi_search.rs
@@ -65,6 +65,7 @@ pub struct MultiSearchQueryParams {
 #[serde_as]
 #[serde_with::skip_serializing_none]
 #[derive(Default, Debug, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
 pub struct MultiSearchHeader {
     #[serde(default)]
     pub allow_no_indices: Option<bool>,

diff --git a/quickwit/quickwit-serve/src/elastic_search_api/model/search_body.rs b/quickwit/quickwit-serve/src/elastic_search_api/model/search_body.rs
@@ -53,6 +53,7 @@ struct FieldSortParams {
 }
 
 #[derive(Debug, Default, Clone, Deserialize, PartialEq)]
+#[serde(deny_unknown_fields)]
 pub struct SearchBody {
     #[serde(default)]
     pub from: Option<u64>,
@@ -164,7 +165,7 @@ mod tests {
             ]
         }
         "#;
-        let search_body: super::SearchBody = serde_json::from_str(json).unwrap();
+        let search_body: SearchBody = serde_json::from_str(json).unwrap();
         let sort_fields = search_body.sort.unwrap();
         assert_eq!(sort_fields.len(), 5);
         assert_eq!(sort_fields[0].field, "timestamp");
@@ -189,7 +190,7 @@ mod tests {
             }
         }
         "#;
-        let search_body: super::SearchBody = serde_json::from_str(json).unwrap();
+        let search_body: SearchBody = serde_json::from_str(json).unwrap();
         let field_sorts = search_body.sort.unwrap();
         assert_eq!(field_sorts.len(), 2);
         assert_eq!(field_sorts[0].field, "timestamp");
@@ -210,7 +211,7 @@ mod tests {
             ]
         }
         "#;
-        let search_body: super::SearchBody = serde_json::from_str(json).unwrap();
+        let search_body: SearchBody = serde_json::from_str(json).unwrap();
         let field_sorts = search_body.sort.unwrap();
         assert_eq!(field_sorts.len(), 4);
         assert_eq!(field_sorts[0].field, "timestamp");
@@ -222,4 +223,25 @@ mod tests {
         assert_eq!(field_sorts[3].field, "_doc");
         assert_eq!(field_sorts[3].order, SortOrder::Asc);
     }
+
+    #[test]
+    fn test_unknown_field_behaviour() {
+        let json = r#"
+            {
+                "term": {
+                    "actor.id": {
+                        "value": "95077794"
+                     }
+                }
+            }
+        "#;
+
+        let search_body = serde_json::from_str::<SearchBody>(json);
+        let error_msg = search_body.unwrap_err().to_string();
+        assert!(error_msg.contains("unknown field `term`"));
+        assert!(error_msg.contains(
+            "expected one of `from`, `size`, `query`, `sort`, `aggs`, `track_total_hits`, \
+             `stored_fields`, `search_after`"
+        ));
+    }
 }
diff --git a/quickwit/quickwit-serve/src/elastic_search_api/model/search_query_params.rs b/quickwit/quickwit-serve/src/elastic_search_api/model/search_query_params.rs
@@ -32,6 +32,7 @@ use crate::simple_list::{from_simple_list, to_simple_list};
 
 #[serde_with::skip_serializing_none]
 #[derive(Default, Debug, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
 pub struct SearchQueryParams {
     #[serde(serialize_with = "to_simple_list")]
     #[serde(deserialize_with = "from_simple_list")]