From 1b7f6ea1e767a4856296417cc7ecb06ff38147df Mon Sep 17 00:00:00 2001
From: Kerollmops <clement@meilisearch.com>
Date: Tue, 17 Aug 2021 16:14:56 +0200
Subject: [PATCH] Return a new error when the sort criteria is not sortable

---
 milli/src/criterion.rs  |  9 +++++++++
 milli/src/error.rs      | 10 ++++++++++
 milli/src/search/mod.rs | 20 ++++++++++++++++++--
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/milli/src/criterion.rs b/milli/src/criterion.rs
index 38162a74b..47eb7c7dc 100644
--- a/milli/src/criterion.rs
+++ b/milli/src/criterion.rs
@@ -64,6 +64,15 @@ pub enum AscDesc {
     Desc(String),
 }
 
+impl AscDesc {
+    pub fn field(&self) -> &str {
+        match self {
+            AscDesc::Asc(field) => field,
+            AscDesc::Desc(field) => field,
+        }
+    }
+}
+
 impl FromStr for AscDesc {
     type Err = UserError;
 
diff --git a/milli/src/error.rs b/milli/src/error.rs
index 713935869..9bda74631 100644
--- a/milli/src/error.rs
+++ b/milli/src/error.rs
@@ -58,6 +58,7 @@ pub enum UserError {
     InvalidFacetsDistribution { invalid_facets_name: HashSet<String> },
     InvalidFilter(pest::error::Error<ParserRule>),
     InvalidFilterAttribute(pest::error::Error<ParserRule>),
+    InvalidSortableAttribute { field: String, valid_fields: HashSet<String> },
     InvalidStoreFile,
     MaxDatabaseSizeReached,
     MissingDocumentId { document: Object },
@@ -226,6 +227,15 @@ only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and undersco
                 )
             }
             Self::InvalidFilterAttribute(error) => error.fmt(f),
+            Self::InvalidSortableAttribute { field, valid_fields } => {
+                let valid_names =
+                    valid_fields.iter().map(AsRef::as_ref).collect::<Vec<_>>().join(", ");
+                write!(
+                    f,
+                    "Attribute {} is not sortable, available sortable attributes are: {}",
+                    field, valid_names
+                )
+            }
             Self::MissingDocumentId { document } => {
                 let json = serde_json::to_string(document).unwrap();
                 write!(f, "document doesn't have an identifier {}", json)
diff --git a/milli/src/search/mod.rs b/milli/src/search/mod.rs
index ce2efcc98..23e5c1834 100644
--- a/milli/src/search/mod.rs
+++ b/milli/src/search/mod.rs
@@ -19,6 +19,7 @@ pub use self::facet::{FacetDistribution, FacetNumberIter, FilterCondition, Opera
 pub use self::matching_words::MatchingWords;
 use self::query_tree::QueryTreeBuilder;
 use crate::criterion::AscDesc;
+use crate::error::UserError;
 use crate::search::criteria::r#final::{Final, FinalResult};
 use crate::{DocumentId, Index, Result};
 
@@ -142,13 +143,28 @@ impl<'a> Search<'a> {
             None => MatchingWords::default(),
         };
 
+        // We check that we are allowed to use the sort criteria, we check
+        // that they are declared in the sortable fields.
+        let sortable_fields = self.index.sortable_fields(self.rtxn)?;
+        if let Some(sort_criteria) = &self.sort_criteria {
+            for asc_desc in sort_criteria {
+                let field = asc_desc.field();
+                if !sortable_fields.contains(field) {
+                    return Err(UserError::InvalidSortableAttribute {
+                        field: field.to_string(),
+                        valid_fields: sortable_fields,
+                    }
+                    .into());
+                }
+            }
+        }
+
         let criteria_builder = criteria::CriteriaBuilder::new(self.rtxn, self.index)?;
-        let sort_criteria = self.sort_criteria.clone();
         let criteria = criteria_builder.build(
             query_tree,
             primitive_query,
             filtered_candidates,
-            sort_criteria,
+            self.sort_criteria.clone(),
         )?;
 
         match self.index.distinct_field(self.rtxn)? {