From 32bd9f091fd74b50e4f3623d42a471d2ebb23241 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Renault?= <clement@meilisearch.com>
Date: Tue, 7 Dec 2021 17:20:11 +0100
Subject: [PATCH] Detect the filters that are too deep and return an error

---
 filter-parser/src/lib.rs         |  6 +++--
 milli/src/search/facet/filter.rs | 40 ++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/filter-parser/src/lib.rs b/filter-parser/src/lib.rs
index 3ef31c3c8..0e49e00e9 100644
--- a/filter-parser/src/lib.rs
+++ b/filter-parser/src/lib.rs
@@ -118,10 +118,12 @@ impl<'a> FilterCondition<'a> {
         match self {
             FilterCondition::Condition { fid, .. } if depth == 0 => Some(fid),
             FilterCondition::Or(left, right) => {
-                left.token_at_depth(depth - 1).or_else(|| right.token_at_depth(depth - 1))
+                let depth = depth.saturating_sub(1);
+                right.token_at_depth(depth).or_else(|| left.token_at_depth(depth))
             }
             FilterCondition::And(left, right) => {
-                left.token_at_depth(depth - 1).or_else(|| right.token_at_depth(depth - 1))
+                let depth = depth.saturating_sub(1);
+                right.token_at_depth(depth).or_else(|| left.token_at_depth(depth))
             }
             FilterCondition::GeoLowerThan { point: [point, _], .. } if depth == 0 => Some(point),
             FilterCondition::GeoGreaterThan { point: [point, _], .. } if depth == 0 => Some(point),
diff --git a/milli/src/search/facet/filter.rs b/milli/src/search/facet/filter.rs
index 5a88c14dc..2c78816fb 100644
--- a/milli/src/search/facet/filter.rs
+++ b/milli/src/search/facet/filter.rs
@@ -128,6 +128,11 @@ impl<'a> Filter<'a> {
             Ok(fc) => Ok(fc),
             Err(e) => Err(Error::UserError(UserError::InvalidFilter(e.to_string()))),
         }?;
+
+        if let Some(token) = condition.token_at_depth(MAX_FILTER_DEPTH) {
+            return Err(token.as_external_error(FilterError::TooDeep).into());
+        }
+
         Ok(Self { condition })
     }
 }
@@ -431,6 +436,8 @@ impl<'a> From<FilterCondition<'a>> for Filter<'a> {
 
 #[cfg(test)]
 mod tests {
+    use std::fmt::Write;
+
     use big_s::S;
     use either::Either;
     use heed::EnvOpenOptions;
@@ -598,4 +605,37 @@ mod tests {
             "Bad longitude `180.000001`. Longitude must be contained between -180 and 180 degrees."
         ));
     }
+
+    #[test]
+    fn filter_depth() {
+        let path = tempfile::tempdir().unwrap();
+        let mut options = EnvOpenOptions::new();
+        options.map_size(10 * 1024 * 1024); // 10 MB
+        let index = Index::new(options, &path).unwrap();
+
+        // Set the filterable fields to be the channel.
+        let mut wtxn = index.write_txn().unwrap();
+        let mut builder = Settings::new(&mut wtxn, &index);
+        builder.set_searchable_fields(vec![S("account_ids")]);
+        builder.set_filterable_fields(hashset! { S("account_ids") });
+        builder.execute(|_| ()).unwrap();
+        wtxn.commit().unwrap();
+
+        // generates a big (2 MiB) filter with too much of ORs.
+        let tipic_filter = "account_ids=14361 OR ";
+        let mut filter_string = String::with_capacity(tipic_filter.len() * 14360);
+        for i in 1..=14361 {
+            let _ = write!(&mut filter_string, "account_ids={}", i);
+            if i != 14361 {
+                let _ = write!(&mut filter_string, " OR ");
+            }
+        }
+
+        let error = Filter::from_str(&filter_string).unwrap_err();
+        assert!(
+            error.to_string().starts_with("Too many filter conditions"),
+            "{}",
+            error.to_string()
+        );
+    }
 }