snowykami/meilisearch
1
0
Fork 0
mirror of https://github.com/meilisearch/meilisearch.git synced 2025-10-31 07:56:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

Update the check on the latitude and longitude

Browse Source 
Latitude are not supposed to go beyound 90 degrees or below -90.
The same goes for longitude with 180 or -180.

This was badly implemented in the filters, and was not implemented for the AscDesc rules.
This commit is contained in:
Tamo
2021-10-07 15:42:08 +02:00
parent dde1da1c0e
commit 11dfe38761
2 changed files with 87 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
milli/src/asc_desc.rs
View File

@@ -12,6 +12,8 @@ use crate::{CriterionError, Error, UserError};
/// You must always cast it to a sort error or a criterion error.
#[derive(Debug)]
pub enum AscDescError {
BadLat,
BadLng,
InvalidSyntax { name: String },
ReservedKeyword { name: String },
}
@@ -19,6 +21,12 @@ pub enum AscDescError {
impl fmt::Display for AscDescError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::BadLat => {
write!(f, "Latitude must be contained between -90 and 90 degrees.",)
}
Self::BadLng => {
write!(f, "Longitude must be contained between -180 and 180 degrees.",)
}
Self::InvalidSyntax { name } => {
write!(f, "invalid asc/desc syntax for {}.", name)
}
@@ -36,6 +44,9 @@ impl fmt::Display for AscDescError {
impl From<AscDescError> for CriterionError {
fn from(error: AscDescError) -> Self {
match error {
AscDescError::BadLat | AscDescError::BadLng => {
CriterionError::ReservedNameForSort { name: "_geoPoint".to_string() }
}
AscDescError::InvalidSyntax { name } => CriterionError::InvalidName { name },
AscDescError::ReservedKeyword { name } if name.starts_with("_geoPoint") => {
CriterionError::ReservedNameForSort { name: "_geoPoint".to_string() }
@@ -60,16 +71,21 @@ impl FromStr for Member {
fn from_str(text: &str) -> Result<Member, Self::Err> {
match text.strip_prefix("_geoPoint(").and_then(|text| text.strip_suffix(")")) {
Some(point) => {
let (lat, long) = point
let (lat, lng) = point
.split_once(',')
.ok_or_else(|| AscDescError::ReservedKeyword { name: text.to_string() })
.and_then(|(lat, long)| {
.and_then(|(lat, lng)| {
lat.trim()
.parse()
.and_then(|lat| long.trim().parse().map(|long| (lat, long)))
.and_then(|lat| lng.trim().parse().map(|lng| (lat, lng)))
.map_err(|_| AscDescError::ReservedKeyword { name: text.to_string() })
})?;
Ok(Member::Geo([lat, long]))
if !(-90.0..=90.0).contains(&lat) {
return Err(AscDescError::BadLat)?;
} else if !(-180.0..=180.0).contains(&lng) {
return Err(AscDescError::BadLng)?;
}
Ok(Member::Geo([lat, lng]))
}
None => {
if is_reserved_keyword(text) || text.starts_with("_geoRadius(") {
@@ -139,6 +155,8 @@ impl FromStr for AscDesc {
#[derive(Debug)]
pub enum SortError {
BadLat,
BadLng,
BadGeoPointUsage { name: String },
InvalidName { name: String },
ReservedName { name: String },
@@ -149,6 +167,8 @@ pub enum SortError {
impl From<AscDescError> for SortError {
fn from(error: AscDescError) -> Self {
match error {
AscDescError::BadLat => SortError::BadLat,
AscDescError::BadLng => SortError::BadLng,
AscDescError::InvalidSyntax { name } => SortError::InvalidName { name },
AscDescError::ReservedKeyword { name } if name.starts_with("_geoPoint") => {
SortError::BadGeoPointUsage { name }
@@ -167,6 +187,12 @@ impl From<AscDescError> for SortError {
impl fmt::Display for SortError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::BadLat => {
write!(f, "Latitude must be contained between -90 and 90 degrees.",)
}
Self::BadLng => {
write!(f, "Longitude must be contained between -180 and 180 degrees.",)
}
Self::BadGeoPointUsage { name } => {
write!(
f,
@@ -225,6 +251,8 @@ mod tests {
("_geoPoint(42.459, 59):desc", Desc(Geo([42.459, 59.]))),
("_geoPoint(42, 59.895):desc", Desc(Geo([42., 59.895]))),
("_geoPoint(42, 59.895):desc", Desc(Geo([42., 59.895]))),
("_geoPoint(90.000000000, 180):desc", Desc(Geo([90., 180.]))),
("_geoPoint(-90, -180.0000000000):asc", Asc(Geo([-90., -180.]))),
("_geoPoint(42.0002, 59.895):desc", Desc(Geo([42.0002, 59.895]))),
("_geoPoint(42., 59.):desc", Desc(Geo([42., 59.]))),
("truc(12, 13):desc", Desc(Field(S("truc(12, 13)")))),
@@ -268,6 +296,11 @@ mod tests {
),
("_geoPoint(35, 85, 75):asc", ReservedKeyword { name: S("_geoPoint(35, 85, 75)") }),
("_geoPoint(18):asc", ReservedKeyword { name: S("_geoPoint(18)") }),
("_geoPoint(200, 200):asc", BadLat),
("_geoPoint(90.000001, 0):asc", BadLat),
("_geoPoint(0, -180.000001):desc", BadLng),
("_geoPoint(159.256, 130):asc", BadLat),
("_geoPoint(12, -2021):desc", BadLng),
];
for (req, expected_error) in invalid_req {
@@ -313,6 +346,14 @@ mod tests {
AscDescError::ReservedKeyword { name: S("_geoRadius(12, 13)") },
S("`_geoRadius` is a reserved keyword and thus can't be used as a sort expression. Use the `_geoPoint(latitude, longitude)` built-in rule to sort on `_geo` field coordinates."),
),
(
AscDescError::BadLat,
S("Latitude must be contained between -90 and 90 degrees."),
),
(
AscDescError::BadLng,
S("Longitude must be contained between -180 and 180 degrees."),
),
];
for (asc_desc_error, expected_message) in errors {

54
milli/src/search/facet/filter_condition.rs
View File

@@ -206,17 +206,19 @@ impl FilterCondition {
)))?;
}
let (lat, lng, distance) = (&parameters[0], &parameters[1], parameters[2].0);
if let Some(span) = (!(-181.0..181.).contains(&lat.0))
.then(|| &lat.1)
.or((!(-181.0..181.).contains(&lng.0)).then(|| &lng.1))
{
if !(-90.0..=90.0).contains(&lat.0) {
return Err(UserError::InvalidFilter(PestError::new_from_span(
ErrorVariant::CustomError {
message: format!(
"Latitude and longitude must be contained between -180 to 180 degrees."
),
message: format!("Latitude must be contained between -90 and 90 degrees."),
},
span.clone(),
lat.1.clone(),
)))?;
} else if !(-180.0..=180.0).contains(&lng.0) {
return Err(UserError::InvalidFilter(PestError::new_from_span(
ErrorVariant::CustomError {
message: format!("Longitude must be contained between -180 and 180 degrees."),
},
lng.1.clone(),
)))?;
}
Ok(Operator(fid, GeoLowerThan([lat.0, lng.0], distance)))
@@ -858,6 +860,18 @@ mod tests {
let expected = Operator(0, GeoLowerThan([12., 13.0005], 2000.));
assert_eq!(condition, expected);
// basic test with latitude and longitude at the max angle
let condition =
FilterCondition::from_str(&rtxn, &index, "_geoRadius(90, 180, 2000)").unwrap();
let expected = Operator(0, GeoLowerThan([90., 180.], 2000.));
assert_eq!(condition, expected);
// basic test with latitude and longitude at the min angle
let condition =
FilterCondition::from_str(&rtxn, &index, "_geoRadius(-90, -180, 2000)").unwrap();
let expected = Operator(0, GeoLowerThan([-90., -180.], 2000.));
assert_eq!(condition, expected);
// test the negation of the GeoLowerThan
let condition =
FilterCondition::from_str(&rtxn, &index, "NOT _geoRadius(50, 18, 2000.500)").unwrap();
@@ -906,20 +920,36 @@ mod tests {
assert!(error.to_string().contains("The `_geoRadius` filter expect three arguments: `_geoRadius(latitude, longitude, radius)`"));
// georadius have a bad latitude
let result = FilterCondition::from_str(&rtxn, &index, "_geoRadius(-200, 150, 10)");
let result = FilterCondition::from_str(&rtxn, &index, "_geoRadius(-100, 150, 10)");
assert!(result.is_err());
let error = result.unwrap_err();
assert!(error
.to_string()
.contains("Latitude and longitude must be contained between -180 to 180 degrees."));
.contains("Latitude must be contained between -90 and 90 degrees."));
// georadius have a bad latitude
let result = FilterCondition::from_str(&rtxn, &index, "_geoRadius(-90.0000001, 150, 10)");
assert!(result.is_err());
let error = result.unwrap_err();
assert!(error
.to_string()
.contains("Latitude must be contained between -90 and 90 degrees."));
// georadius have a bad longitude
let result = FilterCondition::from_str(&rtxn, &index, "_geoRadius(-10, 181, 10)");
let result = FilterCondition::from_str(&rtxn, &index, "_geoRadius(-10, 250, 10)");
assert!(result.is_err());
let error = result.unwrap_err();
assert!(error
.to_string()
.contains("Latitude and longitude must be contained between -180 to 180 degrees."));
.contains("Longitude must be contained between -180 and 180 degrees."));
// georadius have a bad longitude
let result = FilterCondition::from_str(&rtxn, &index, "_geoRadius(-10, 180.000001, 10)");
assert!(result.is_err());
let error = result.unwrap_err();
assert!(error
.to_string()
.contains("Longitude must be contained between -180 and 180 degrees."));
}
#[test]