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> 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() + ); + } }