diff --git a/meilisearch-http/src/error.rs b/meilisearch-http/src/error.rs index cbe963615..1f2f4742d 100644 --- a/meilisearch-http/src/error.rs +++ b/meilisearch-http/src/error.rs @@ -11,21 +11,31 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, thiserror::Error)] pub enum MeilisearchHttpError { - #[error("A Content-Type header is missing. Accepted values for the Content-Type header are: \"application/json\", \"application/x-ndjson\", \"text/csv\"")] - MissingContentType, - #[error("The Content-Type \"{0}\" is invalid. Accepted values for the Content-Type header are: \"application/json\", \"application/x-ndjson\", \"text/csv\"")] - InvalidContentType(String), + #[error("A Content-Type header is missing. Accepted values for the Content-Type header are: {}", + .0.iter().map(|s| format!("\"{}\"", s)).collect::>().join(", "))] + MissingContentType(Vec), + #[error( + "The Content-Type \"{0}\" is invalid. Accepted values for the Content-Type header are: {}", + .1.iter().map(|s| format!("\"{}\"", s)).collect::>().join(", ") + )] + InvalidContentType(String, Vec), } impl ErrorCode for MeilisearchHttpError { fn error_code(&self) -> Code { match self { - MeilisearchHttpError::MissingContentType => Code::MissingContentType, - MeilisearchHttpError::InvalidContentType(_) => Code::InvalidContentType, + MeilisearchHttpError::MissingContentType(_) => Code::MissingContentType, + MeilisearchHttpError::InvalidContentType(_, _) => Code::InvalidContentType, } } } +impl From for aweb::Error { + fn from(other: MeilisearchHttpError) -> Self { + aweb::Error::from(ResponseError::from(other)) + } +} + #[derive(Debug, Serialize, Deserialize, Clone)] #[serde(rename_all = "camelCase")] pub struct ResponseError { @@ -121,9 +131,8 @@ impl From for PayloadError { } } -pub fn payload_error_handler(err: E) -> ResponseError -where - E: Into, -{ - err.into().into() +impl From for aweb::Error { + fn from(other: PayloadError) -> Self { + aweb::Error::from(ResponseError::from(other)) + } } diff --git a/meilisearch-http/src/lib.rs b/meilisearch-http/src/lib.rs index 0b9dacbe0..cbb417ab1 100644 --- a/meilisearch-http/src/lib.rs +++ b/meilisearch-http/src/lib.rs @@ -11,10 +11,14 @@ pub mod routes; use std::path::Path; use std::time::Duration; +use crate::error::MeilisearchHttpError; use crate::extractors::authentication::AuthConfig; +use actix_web::error::JsonPayloadError; +use error::PayloadError; +use http::header::CONTENT_TYPE; pub use option::Opt; -use actix_web::web; +use actix_web::{web, HttpRequest}; use extractors::authentication::policies::*; use extractors::payload::PayloadConfig; @@ -98,14 +102,25 @@ pub fn configure_data(config: &mut web::ServiceConfig, data: MeiliSearch, opt: & .app_data(data) .app_data( web::JsonConfig::default() - .limit(http_payload_size_limit) - .content_type(|_mime| true) // Accept all mime types - .error_handler(|err, _req| error::payload_error_handler(err).into()), + .content_type(|mime| mime == mime::APPLICATION_JSON) + .error_handler(|err, req: &HttpRequest| match err { + JsonPayloadError::ContentType => match req.headers().get(CONTENT_TYPE) { + Some(content_type) => MeilisearchHttpError::InvalidContentType( + content_type.to_str().unwrap_or("unknown").to_string(), + vec![mime::APPLICATION_JSON.to_string()], + ) + .into(), + None => MeilisearchHttpError::MissingContentType(vec![ + mime::APPLICATION_JSON.to_string(), + ]) + .into(), + }, + err => PayloadError::from(err).into(), + }), ) .app_data(PayloadConfig::new(http_payload_size_limit)) .app_data( - web::QueryConfig::default() - .error_handler(|err, _req| error::payload_error_handler(err).into()), + web::QueryConfig::default().error_handler(|err, _req| PayloadError::from(err).into()), ); } @@ -180,6 +195,7 @@ macro_rules! create_app { use actix_web::middleware::TrailingSlash; use actix_web::App; use actix_web::{middleware, web}; + use meilisearch_http::error::{MeilisearchHttpError, ResponseError}; use meilisearch_http::routes; use meilisearch_http::{configure_auth, configure_data, dashboard}; diff --git a/meilisearch-http/src/routes/indexes/documents.rs b/meilisearch-http/src/routes/indexes/documents.rs index 50ba31c97..e890bab24 100644 --- a/meilisearch-http/src/routes/indexes/documents.rs +++ b/meilisearch-http/src/routes/indexes/documents.rs @@ -6,6 +6,7 @@ use log::debug; use meilisearch_lib::index_controller::{DocumentAdditionFormat, Update}; use meilisearch_lib::milli::update::IndexDocumentsMethod; use meilisearch_lib::MeiliSearch; +use once_cell::sync::Lazy; use serde::Deserialize; use serde_json::Value; use tokio::sync::mpsc; @@ -176,14 +177,29 @@ async fn document_addition( body: Payload, method: IndexDocumentsMethod, ) -> Result { + static ACCEPTED_CONTENT_TYPE: Lazy> = Lazy::new(|| { + vec![ + "application/json".to_string(), + "application/x-ndjson".to_string(), + "application/csv".to_string(), + ] + }); let format = match content_type { Some("application/json") => DocumentAdditionFormat::Json, Some("application/x-ndjson") => DocumentAdditionFormat::Ndjson, Some("text/csv") => DocumentAdditionFormat::Csv, Some(other) => { - return Err(MeilisearchHttpError::InvalidContentType(other.to_string()).into()) + return Err(MeilisearchHttpError::InvalidContentType( + other.to_string(), + ACCEPTED_CONTENT_TYPE.clone(), + ) + .into()) + } + None => { + return Err( + MeilisearchHttpError::MissingContentType(ACCEPTED_CONTENT_TYPE.clone()).into(), + ) } - None => return Err(MeilisearchHttpError::MissingContentType.into()), }; let update = Update::DocumentAddition { diff --git a/meilisearch-http/tests/content_type.rs b/meilisearch-http/tests/content_type.rs new file mode 100644 index 000000000..5402a7cd6 --- /dev/null +++ b/meilisearch-http/tests/content_type.rs @@ -0,0 +1,111 @@ +#![allow(dead_code)] + +mod common; + +use crate::common::Server; +use actix_web::test; +use meilisearch_http::create_app; +use serde_json::{json, Value}; + +#[actix_rt::test] +async fn strict_json_bad_content_type() { + let routes = [ + // all the POST routes except the dumps that can be created without any body or content-type + // and the search that is not a strict json + "/indexes", + "/indexes/doggo/documents/delete-batch", + "/indexes/doggo/search", + "/indexes/doggo/settings", + "/indexes/doggo/settings/displayed-attributes", + "/indexes/doggo/settings/distinct-attribute", + "/indexes/doggo/settings/filterable-attributes", + "/indexes/doggo/settings/ranking-rules", + "/indexes/doggo/settings/searchable-attributes", + "/indexes/doggo/settings/sortable-attributes", + "/indexes/doggo/settings/stop-words", + "/indexes/doggo/settings/synonyms", + ]; + let bad_content_types = [ + "application/csv", + "application/x-ndjson", + "application/x-www-form-urlencoded", + "text/plain", + "json", + "application", + "json/application", + ]; + + let document = "{}"; + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + for route in routes { + // Good content-type, we probably have an error since we didn't send anything in the json + // so we only ensure we didn't get a bad media type error. + let req = test::TestRequest::post() + .uri(route) + .set_payload(document) + .insert_header(("content-type", "application/json")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + assert_ne!(status_code, 415, + "calling the route `{}` with a content-type of json isn't supposed to throw a bad media type error", route); + + // No content-type. + let req = test::TestRequest::post() + .uri(route) + .set_payload(document) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 415, "calling the route `{}` without content-type is supposed to throw a bad media type error", route); + assert_eq!( + response, + json!({ + "message": r#"A Content-Type header is missing. Accepted values for the Content-Type header are: "application/json""#, + "errorCode": "missing_content_type", + "errorType": "invalid_request_error", + "errorLink": "https://docs.meilisearch.com/errors#missing_content_type", + }), + "when calling the route `{}` with no content-type", + route, + ); + + for bad_content_type in bad_content_types { + // Always bad content-type + let req = test::TestRequest::post() + .uri(route) + .set_payload(document.to_string()) + .insert_header(("content-type", bad_content_type)) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 415); + let expected_error_message = format!( + r#"The Content-Type "{}" is invalid. Accepted values for the Content-Type header are: "application/json""#, + bad_content_type + ); + assert_eq!( + response, + json!({ + "message": expected_error_message, + "errorCode": "invalid_content_type", + "errorType": "invalid_request_error", + "errorLink": "https://docs.meilisearch.com/errors#invalid_content_type", + }), + "when calling the route `{}` with a content-type of `{}`", + route, + bad_content_type, + ); + } + } +} diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index 13265dcfd..a9189a30c 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -22,6 +22,7 @@ async fn add_documents_test_json_content_types() { &server.service.options )) .await; + // post let req = test::TestRequest::post() .uri("/indexes/dog/documents") .set_payload(document.to_string()) @@ -33,6 +34,19 @@ async fn add_documents_test_json_content_types() { let response: Value = serde_json::from_slice(&body).unwrap_or_default(); assert_eq!(status_code, 202); assert_eq!(response, json!({ "updateId": 0 })); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/json")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 202); + assert_eq!(response, json!({ "updateId": 1 })); } /// no content type is still supposed to be accepted as json @@ -52,6 +66,7 @@ async fn add_documents_test_no_content_types() { &server.service.options )) .await; + // post let req = test::TestRequest::post() .uri("/indexes/dog/documents") .set_payload(document.to_string()) @@ -63,11 +78,23 @@ async fn add_documents_test_no_content_types() { let response: Value = serde_json::from_slice(&body).unwrap_or_default(); assert_eq!(status_code, 202); assert_eq!(response, json!({ "updateId": 0 })); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/json")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 202); + assert_eq!(response, json!({ "updateId": 1 })); } /// any other content-type is must be refused #[actix_rt::test] -#[ignore] async fn add_documents_test_bad_content_types() { let document = json!([ { @@ -83,6 +110,7 @@ async fn add_documents_test_bad_content_types() { &server.service.options )) .await; + // post let req = test::TestRequest::post() .uri("/indexes/dog/documents") .set_payload(document.to_string()) @@ -91,8 +119,32 @@ async fn add_documents_test_bad_content_types() { let res = test::call_service(&app, req).await; let status_code = res.status(); let body = test::read_body(res).await; - assert_eq!(status_code, 405); - assert!(body.is_empty()); + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 415); + assert_eq!( + response["message"], + json!( + r#"The Content-Type "text/plain" is invalid. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "application/csv""# + ) + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/plain")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 415); + assert_eq!( + response["message"], + json!( + r#"The Content-Type "text/plain" is invalid. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "application/csv""# + ) + ); } #[actix_rt::test] diff --git a/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs b/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs index 0040d4cea..35640aaef 100644 --- a/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs +++ b/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs @@ -4,7 +4,6 @@ use std::path::{Path, PathBuf}; use serde_json::{Deserializer, Value}; use tempfile::NamedTempFile; -use uuid::Uuid; use crate::index_controller::dump_actor::loaders::compat::{asc_ranking_rule, desc_ranking_rule}; use crate::index_controller::dump_actor::Metadata; @@ -200,7 +199,7 @@ impl From for Enqueued { method, // Just ignore if the uuid is no present. If it is needed later, an error will // be thrown. - content_uuid: content.unwrap_or_else(Uuid::default), + content_uuid: content.unwrap_or_default(), } } compat::UpdateMeta::ClearDocuments => Update::ClearDocuments,