From d300d788c7e7cfbdad7f05b13064032af3ed4394 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Mon, 18 May 2020 15:18:23 +0200 Subject: [PATCH] Make the compute_document_id validate the id --- meilisearch-core/src/update/helpers.rs | 28 ++++++++++--------------- meilisearch-core/src/update/mod.rs | 2 +- meilisearch-http/src/routes/document.rs | 23 +++++--------------- 3 files changed, 17 insertions(+), 36 deletions(-) diff --git a/meilisearch-core/src/update/helpers.rs b/meilisearch-core/src/update/helpers.rs index a3be7bb22..a0a0ee266 100644 --- a/meilisearch-core/src/update/helpers.rs +++ b/meilisearch-core/src/update/helpers.rs @@ -88,17 +88,16 @@ pub fn value_to_number(value: &Value) -> Option { } } -/// Compute the hash of the given type, this is the way we produce documents ids. -pub fn compute_document_id(t: H) -> DocumentId { - let mut s = SipHasher::new(); - t.hash(&mut s); - let hash = s.finish(); - DocumentId(hash) -} - -/// Validates a string representation to be a correct document id. -pub fn validate_document_id(string: &str) -> bool { - string.chars().all(|x| x.is_ascii_alphanumeric() || x == '-' || x == '_') +/// Validates a string representation to be a correct document id and +/// returns the hash of the given type, this is the way we produce documents ids. +pub fn compute_document_id(string: &str) -> Result { + if string.chars().all(|x| x.is_ascii_alphanumeric() || x == '-' || x == '_') { + let mut s = SipHasher::new(); + string.hash(&mut s); + Ok(DocumentId(s.finish())) + } else { + Err(SerializerError::InvalidDocumentIdFormat) + } } /// Extracts and validates the document id of a document. @@ -110,12 +109,7 @@ pub fn extract_document_id(primary_key: &str, document: &IndexMap Value::String(string) => string.clone(), _ => return Err(SerializerError::InvalidDocumentIdFormat), }; - - if validate_document_id(&string) { - Ok(compute_document_id(string)) - } else { - Err(SerializerError::InvalidDocumentIdFormat) - } + compute_document_id(&string) } None => Err(SerializerError::DocumentIdNotFound), } diff --git a/meilisearch-core/src/update/mod.rs b/meilisearch-core/src/update/mod.rs index 124e6450a..3a3499c3e 100644 --- a/meilisearch-core/src/update/mod.rs +++ b/meilisearch-core/src/update/mod.rs @@ -9,7 +9,7 @@ pub use self::clear_all::{apply_clear_all, push_clear_all}; pub use self::customs_update::{apply_customs_update, push_customs_update}; pub use self::documents_addition::{apply_documents_addition, apply_documents_partial_addition, DocumentsAddition}; pub use self::documents_deletion::{apply_documents_deletion, DocumentsDeletion}; -pub use self::helpers::{index_value, value_to_string, value_to_number, compute_document_id, extract_document_id, validate_document_id}; +pub use self::helpers::{index_value, value_to_string, value_to_number, compute_document_id, extract_document_id}; pub use self::settings_update::{apply_settings_update, push_settings_update}; use std::cmp; diff --git a/meilisearch-http/src/routes/document.rs b/meilisearch-http/src/routes/document.rs index 22586ecb6..3b71e656a 100644 --- a/meilisearch-http/src/routes/document.rs +++ b/meilisearch-http/src/routes/document.rs @@ -3,12 +3,10 @@ use std::collections::{BTreeSet, HashSet}; use actix_web::{web, HttpResponse}; use actix_web_macros::{delete, get, post, put}; use indexmap::IndexMap; +use meilisearch_core::{update, Error}; use serde::Deserialize; use serde_json::Value; -use meilisearch_core::{Error, serde::SerializerError}; -use meilisearch_core::update; - use crate::error::ResponseError; use crate::helpers::Authentication; use crate::routes::{IndexParam, IndexUpdateResponse}; @@ -45,11 +43,7 @@ async fn get_document( .open_index(&path.index_uid) .ok_or(ResponseError::index_not_found(&path.index_uid))?; - if !update::validate_document_id(&path.document_id) { - return Err(Error::Serializer(SerializerError::InvalidDocumentIdFormat).into()) - } - - let document_id = update::compute_document_id(&path.document_id); + let document_id = update::compute_document_id(&path.document_id).map_err(Error::Serializer)?; let reader = data.db.main_read_txn()?; let response: Document = index @@ -72,11 +66,7 @@ async fn delete_document( .open_index(&path.index_uid) .ok_or(ResponseError::index_not_found(&path.index_uid))?; - if !update::validate_document_id(&path.document_id) { - return Err(Error::Serializer(SerializerError::InvalidDocumentIdFormat).into()) - } - - let document_id = update::compute_document_id(&path.document_id); + let document_id = update::compute_document_id(&path.document_id).map_err(Error::Serializer)?; let mut update_writer = data.db.update_write_txn()?; @@ -248,11 +238,8 @@ async fn delete_documents( let mut documents_deletion = index.documents_deletion(); for document_id in body.into_inner() { - let document_id_string = update::value_to_string(&document_id); - if !update::validate_document_id(&document_id_string) { - return Err(Error::Serializer(SerializerError::InvalidDocumentIdFormat).into()) - } - let document_id = update::compute_document_id(document_id_string); + let document_id = update::value_to_string(&document_id); + let document_id = update::compute_document_id(&document_id).map_err(Error::Serializer)?; documents_deletion.delete_document_by_id(document_id); }