diff --git a/CHANGELOG.md b/CHANGELOG.md index b68dbdc61..f0b1ace8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,10 @@ - Add support for faceted search (#631) - Add support for configuring the lmdb map size (#646, #647) - Add exposed port for Dockerfile (#654) - - Add sentry probe + - Add sentry probe (#664) + - Fix url trailing slash and double slash issues (#659) + - Fix accept all Content-Type by default (#653) + - Return the error message from Serde when a deserialization error is encountered (#661) ## v0.10.1 diff --git a/Cargo.lock b/Cargo.lock index 4b718210a..26b4bb72c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1641,6 +1641,7 @@ dependencies = [ "actix-web", "actix-web-macros", "assert-json-diff", + "bytes 0.5.4", "chrono", "crossbeam-channel", "env_logger", @@ -1660,6 +1661,7 @@ dependencies = [ "pretty-bytes", "rand 0.7.3", "sentry", + "regex", "serde", "serde_json", "serde_qs", diff --git a/meilisearch-http/Cargo.toml b/meilisearch-http/Cargo.toml index 1f891f091..8e4876397 100644 --- a/meilisearch-http/Cargo.toml +++ b/meilisearch-http/Cargo.toml @@ -24,6 +24,7 @@ actix-rt = "1" actix-service = "1.0.5" actix-web = "2" actix-web-macros = "0.1.0" +bytes = "0.5.4" chrono = { version = "0.4.11", features = ["serde"] } crossbeam-channel = "0.4.2" env_logger = "0.7.1" @@ -39,6 +40,7 @@ meilisearch-tokenizer = {path = "../meilisearch-tokenizer", version = "0.10.1"} mime = "0.3.16" pretty-bytes = "0.2.2" rand = "0.7.3" +regex = "1.3.6" serde = { version = "1.0.105", features = ["derive"] } serde_json = { version = "1.0.50", features = ["preserve_order"] } serde_qs = "0.5.2" diff --git a/meilisearch-http/src/data.rs b/meilisearch-http/src/data.rs index da171b53d..056efe73d 100644 --- a/meilisearch-http/src/data.rs +++ b/meilisearch-http/src/data.rs @@ -134,7 +134,7 @@ impl Data { let db_opt = DatabaseOptions { main_map_size: opt.main_map_size, - update_map_size: opt.update_map_size + update_map_size: opt.update_map_size, }; let db = Arc::new(Database::open_or_create(opt.db_path, db_opt).unwrap()); diff --git a/meilisearch-http/src/error.rs b/meilisearch-http/src/error.rs index e24a57bb8..e79cb56b6 100644 --- a/meilisearch-http/src/error.rs +++ b/meilisearch-http/src/error.rs @@ -4,6 +4,7 @@ use actix_http::ResponseBuilder; use actix_web as aweb; use actix_web::http::StatusCode; use serde_json::json; +use actix_web::error::JsonPayloadError; #[derive(Debug)] pub enum ResponseError { @@ -23,6 +24,8 @@ pub enum ResponseError { FilterParsing(String), RetrieveDocument(u64, String), SearchDocuments(String), + PayloadTooLarge, + UnsupportedMediaType, FacetExpression(String), } @@ -108,6 +111,8 @@ impl fmt::Display for ResponseError { Self::RetrieveDocument(id, err) => write!(f, "impossible to retrieve the document with id: {}; {}", id, err), Self::SearchDocuments(err) => write!(f, "impossible to search documents; {}", err), Self::FacetExpression(e) => write!(f, "error parsing facet filter expression: {}", e), + Self::PayloadTooLarge => f.write_str("Payload to large"), + Self::UnsupportedMediaType => f.write_str("Unsupported media type") } } } @@ -138,6 +143,8 @@ impl aweb::error::ResponseError for ResponseError { Self::MissingAuthorizationHeader => StatusCode::FORBIDDEN, Self::Internal(_) => StatusCode::INTERNAL_SERVER_ERROR, Self::Maintenance => StatusCode::SERVICE_UNAVAILABLE, + Self::PayloadTooLarge => StatusCode::PAYLOAD_TOO_LARGE, + Self::UnsupportedMediaType => StatusCode::UNSUPPORTED_MEDIA_TYPE, } } } @@ -190,3 +197,19 @@ impl From for ResponseError { ResponseError::Internal(err.to_string()) } } + +impl From for ResponseError { + fn from(err: JsonPayloadError) -> ResponseError { + match err { + JsonPayloadError::Deserialize(err) => ResponseError::BadRequest(format!("Invalid JSON: {}", err)), + JsonPayloadError::Overflow => ResponseError::PayloadTooLarge, + JsonPayloadError::ContentType => ResponseError::UnsupportedMediaType, + JsonPayloadError::Payload(err) => ResponseError::BadRequest(format!("Problem while decoding the request: {}", err)), + } + } +} + + +pub fn json_error_handler(err: JsonPayloadError) -> ResponseError { + err.into() +} diff --git a/meilisearch-http/src/helpers/authentication.rs b/meilisearch-http/src/helpers/authentication.rs index f42898683..894718d53 100644 --- a/meilisearch-http/src/helpers/authentication.rs +++ b/meilisearch-http/src/helpers/authentication.rs @@ -17,7 +17,6 @@ pub enum Authentication { Admin, } - impl Transform for Authentication where S: Service, Error = Error>, diff --git a/meilisearch-http/src/helpers/mod.rs b/meilisearch-http/src/helpers/mod.rs index 996333141..83a206bc8 100644 --- a/meilisearch-http/src/helpers/mod.rs +++ b/meilisearch-http/src/helpers/mod.rs @@ -1,4 +1,6 @@ pub mod authentication; pub mod meilisearch; +pub mod normalize_slashes; pub use authentication::Authentication; +pub use normalize_slashes::NormalizeSlashes; diff --git a/meilisearch-http/src/helpers/normalize_slashes.rs b/meilisearch-http/src/helpers/normalize_slashes.rs new file mode 100644 index 000000000..2e19cd889 --- /dev/null +++ b/meilisearch-http/src/helpers/normalize_slashes.rs @@ -0,0 +1,88 @@ +/// +/// This middleware normalizes slashes in paths +/// * consecutive instances of `/` get collapsed into one `/` +/// * any ending `/` is removed. +/// Original source from: https://gitlab.com/snippets/1884466 +/// +/// Ex: +/// /this///url/ +/// becomes : /this/url +/// +use actix_service::{Service, Transform}; +use actix_web::{ + dev::ServiceRequest, + dev::ServiceResponse, + http::uri::{PathAndQuery, Uri}, + Error as ActixError, +}; +use futures::future::{ok, Ready}; +use regex::Regex; +use std::task::{Context, Poll}; + +pub struct NormalizeSlashes; + +impl Transform for NormalizeSlashes +where + S: Service, Error = ActixError>, + S::Future: 'static, +{ + type Request = ServiceRequest; + type Response = ServiceResponse; + type Error = ActixError; + type InitError = (); + type Transform = SlashNormalization; + type Future = Ready>; + + fn new_transform(&self, service: S) -> Self::Future { + ok(SlashNormalization { service }) + } +} + +pub struct SlashNormalization { + service: S, +} + +impl Service for SlashNormalization +where + S: Service, Error = ActixError>, + S::Future: 'static, +{ + type Request = ServiceRequest; + type Response = ServiceResponse; + type Error = ActixError; + type Future = S::Future; + + fn poll_ready(&mut self, cx: &mut Context) -> Poll> { + self.service.poll_ready(cx) + } + + fn call(&mut self, mut req: ServiceRequest) -> Self::Future { + let head = req.head(); + + let path = head.uri.path(); + let original_len = path.len(); + let slash_regex = Regex::new("//+").unwrap(); + let new_path = slash_regex.replace_all(path, "/"); + let new_path = new_path.trim_end_matches("/"); + + if original_len != new_path.len() { + let mut parts = head.uri.clone().into_parts(); + + let path = match parts.path_and_query.as_ref().map(|pq| pq.query()).flatten() { + Some(q) => bytes::Bytes::from(format!("{}?{}", new_path, q)), + None => bytes::Bytes::from(new_path.to_string()), + }; + + if let Ok(pq) = PathAndQuery::from_maybe_shared(path) { + parts.path_and_query = Some(pq); + + if let Ok(uri) = Uri::from_parts(parts) { + req.match_info_mut().get_mut().update(&uri); + req.head_mut().uri = uri; + } + } + } + + self.service.call(req) + } +} diff --git a/meilisearch-http/src/lib.rs b/meilisearch-http/src/lib.rs index d4f2822ad..465f96caa 100644 --- a/meilisearch-http/src/lib.rs +++ b/meilisearch-http/src/lib.rs @@ -8,6 +8,7 @@ pub mod option; pub mod routes; pub use self::data::Data; +use self::error::json_error_handler; use actix_http::Error; use actix_service::ServiceFactory; use actix_web::{dev, web, App}; @@ -28,7 +29,12 @@ pub fn create_app( > { App::new() .app_data(web::Data::new(data.clone())) - .app_data(web::JsonConfig::default().limit(1024 * 1024 * 10)) // Json Limit of 10Mb + .app_data( + web::JsonConfig::default() + .limit(1024 * 1024 * 10) // Json Limit of 10Mb + .content_type(|_mime| true) // Accept all mime types + .error_handler(|err, _req| json_error_handler(err).into()), + ) .service(routes::load_html) .service(routes::load_css) .configure(routes::document::services) diff --git a/meilisearch-http/src/main.rs b/meilisearch-http/src/main.rs index 46d8ff96d..7d48fe12b 100644 --- a/meilisearch-http/src/main.rs +++ b/meilisearch-http/src/main.rs @@ -5,6 +5,7 @@ use actix_web::{middleware, HttpServer}; use log::info; use main_error::MainError; use meilisearch_http::data::Data; +use meilisearch_http::helpers::NormalizeSlashes; use meilisearch_http::option::Opt; use meilisearch_http::{create_app, index_update_callback}; use structopt::StructOpt; @@ -72,6 +73,7 @@ async fn main() -> Result<(), MainError> { ) .wrap(middleware::Logger::default()) .wrap(middleware::Compress::default()) + .wrap(NormalizeSlashes) }) .bind(opt.http_addr)? .run() diff --git a/meilisearch-http/src/option.rs b/meilisearch-http/src/option.rs index 2c93f320a..d9fe3599c 100644 --- a/meilisearch-http/src/option.rs +++ b/meilisearch-http/src/option.rs @@ -33,5 +33,5 @@ pub struct Opt { /// The maximum size, in bytes, of the update lmdb database directory #[structopt(long, env = "MEILI_UPDATE_MAP_SIZE", default_value = "107374182400")] // 100GB - pub update_map_size: usize + pub update_map_size: usize, } diff --git a/meilisearch-http/tests/common.rs b/meilisearch-http/tests/common.rs index 731d69987..3b9ac8c2e 100644 --- a/meilisearch-http/tests/common.rs +++ b/meilisearch-http/tests/common.rs @@ -4,9 +4,10 @@ use serde_json::{json, Value}; use std::time::Duration; use actix_web::{http::StatusCode, test}; +use meilisearch_core::DatabaseOptions; use meilisearch_http::data::Data; use meilisearch_http::option::Opt; -use meilisearch_core::DatabaseOptions; +use meilisearch_http::helpers::NormalizeSlashes; use tempdir::TempDir; use tokio::time::delay_for; @@ -28,7 +29,7 @@ impl Server { env: "development".to_owned(), no_analytics: true, main_map_size: default_db_options.main_map_size, - update_map_size: default_db_options.update_map_size + update_map_size: default_db_options.update_map_size, }; let data = Data::new(opt.clone()); @@ -126,7 +127,7 @@ impl Server { pub async fn get_request(&mut self, url: &str) -> (Value, StatusCode) { eprintln!("get_request: {}", url); - let mut app = test::init_service(meilisearch_http::create_app(&self.data)).await; + let mut app = test::init_service(meilisearch_http::create_app(&self.data).wrap(NormalizeSlashes)).await; let req = test::TestRequest::get().uri(url).to_request(); let res = test::call_service(&mut app, req).await; @@ -140,7 +141,7 @@ impl Server { pub async fn post_request(&mut self, url: &str, body: Value) -> (Value, StatusCode) { eprintln!("post_request: {}", url); - let mut app = test::init_service(meilisearch_http::create_app(&self.data)).await; + let mut app = test::init_service(meilisearch_http::create_app(&self.data).wrap(NormalizeSlashes)).await; let req = test::TestRequest::post() .uri(url) @@ -169,7 +170,7 @@ impl Server { pub async fn put_request(&mut self, url: &str, body: Value) -> (Value, StatusCode) { eprintln!("put_request: {}", url); - let mut app = test::init_service(meilisearch_http::create_app(&self.data)).await; + let mut app = test::init_service(meilisearch_http::create_app(&self.data).wrap(NormalizeSlashes)).await; let req = test::TestRequest::put() .uri(url) @@ -197,7 +198,7 @@ impl Server { pub async fn delete_request(&mut self, url: &str) -> (Value, StatusCode) { eprintln!("delete_request: {}", url); - let mut app = test::init_service(meilisearch_http::create_app(&self.data)).await; + let mut app = test::init_service(meilisearch_http::create_app(&self.data).wrap(NormalizeSlashes)).await; let req = test::TestRequest::delete().uri(url).to_request(); let res = test::call_service(&mut app, req).await; diff --git a/meilisearch-http/tests/url_normalizer.rs b/meilisearch-http/tests/url_normalizer.rs new file mode 100644 index 000000000..c2c9187ee --- /dev/null +++ b/meilisearch-http/tests/url_normalizer.rs @@ -0,0 +1,18 @@ +mod common; + +#[actix_rt::test] +async fn url_normalizer() { + let mut server = common::Server::with_uid("movies"); + + let (_response, status_code) = server.get_request("/version").await; + assert_eq!(status_code, 200); + + let (_response, status_code) = server.get_request("//version").await; + assert_eq!(status_code, 200); + + let (_response, status_code) = server.get_request("/version/").await; + assert_eq!(status_code, 200); + + let (_response, status_code) = server.get_request("//version/").await; + assert_eq!(status_code, 200); +}