Merge #1769

1769: Enforce `Content-Type` header for routes requiring a body payload r=MarinPostma a=irevoire

closes #1665 

Co-authored-by: Tamo <tamo@meilisearch.com>

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::<Vec<_>>().join(", "))]
+    MissingContentType(Vec<String>),
+    #[error(
+        "The Content-Type \"{0}\" is invalid. Accepted values for the Content-Type header are: {}",
+        .1.iter().map(|s| format!("\"{}\"", s)).collect::<Vec<_>>().join(", ")
+    )]
+    InvalidContentType(String, Vec<String>),
 }
 
 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<MeilisearchHttpError> 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<QueryPayloadError> for PayloadError {
     }
 }
 
-pub fn payload_error_handler<E>(err: E) -> ResponseError
-where
-    E: Into<PayloadError>,
-{
-    err.into().into()
+impl From<PayloadError> for aweb::Error {
+    fn from(other: PayloadError) -> Self {
+        aweb::Error::from(ResponseError::from(other))
+    }
 }

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};
 

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<HttpResponse, ResponseError> {
+    static ACCEPTED_CONTENT_TYPE: Lazy<Vec<String>> = 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 {

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

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]

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<compat::Enqueued> 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,