Merge pull request #56 from meilisearch/fix-bad-index-uid

Fix bad index uid
This commit is contained in:
marin 2021-03-05 19:57:31 +01:00 committed by GitHub
commit 964e52ef08
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 57 additions and 9 deletions

7
Cargo.lock generated
View File

@ -1677,6 +1677,7 @@ dependencies = [
"tempdir", "tempdir",
"tempfile", "tempfile",
"tokio", "tokio",
"urlencoding",
"uuid", "uuid",
"vergen", "vergen",
] ]
@ -3298,6 +3299,12 @@ dependencies = [
"serde", "serde",
] ]
[[package]]
name = "urlencoding"
version = "1.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c9232eb53352b4442e40d7900465dfc534e8cb2dc8f18656fcb2ac16112b5593"
[[package]] [[package]]
name = "utf8-width" name = "utf8-width"
version = "0.1.4" version = "0.1.4"

View File

@ -71,6 +71,7 @@ serde_url_params = "0.2.0"
tempdir = "0.3.7" tempdir = "0.3.7"
assert-json-diff = { branch = "master", git = "https://github.com/qdequele/assert-json-diff" } assert-json-diff = { branch = "master", git = "https://github.com/qdequele/assert-json-diff" }
tokio = { version = "0.2", features = ["macros", "time"] } tokio = { version = "0.2", features = ["macros", "time"] }
urlencoding = "1.1.1"
[features] [features]
default = ["sentry"] default = ["sentry"]

View File

@ -8,6 +8,7 @@ use std::ops::Deref;
use std::sync::Arc; use std::sync::Arc;
use sha2::Digest; use sha2::Digest;
use anyhow::bail;
use crate::index_controller::{IndexController, LocalIndexController, IndexMetadata, Settings, IndexSettings}; use crate::index_controller::{IndexController, LocalIndexController, IndexMetadata, Settings, IndexSettings};
use crate::option::Opt; use crate::option::Opt;
@ -126,6 +127,9 @@ impl Data {
} }
pub fn create_index(&self, name: impl AsRef<str>, primary_key: Option<impl AsRef<str>>) -> anyhow::Result<IndexMetadata> { pub fn create_index(&self, name: impl AsRef<str>, primary_key: Option<impl AsRef<str>>) -> anyhow::Result<IndexMetadata> {
if !is_index_uid_valid(name.as_ref()) {
bail!("invalid index uid: {:?}", name.as_ref())
}
let settings = IndexSettings { let settings = IndexSettings {
name: Some(name.as_ref().to_string()), name: Some(name.as_ref().to_string()),
primary_key: primary_key.map(|s| s.as_ref().to_string()), primary_key: primary_key.map(|s| s.as_ref().to_string()),
@ -145,3 +149,8 @@ impl Data {
&self.api_keys &self.api_keys
} }
} }
fn is_index_uid_valid(uid: &str) -> bool {
uid.chars().all(|x| x.is_ascii_alphanumeric() || x == '-' || x == '_')
}

View File

@ -1,13 +1,13 @@
use std::ops::Deref; use std::ops::Deref;
use anyhow::bail;
use async_compression::tokio_02::write::GzipEncoder; use async_compression::tokio_02::write::GzipEncoder;
use futures_util::stream::StreamExt; use futures_util::stream::StreamExt;
use milli::update::{IndexDocumentsMethod, UpdateFormat}; use milli::update::{IndexDocumentsMethod, UpdateFormat};
use tokio::io::AsyncWriteExt; use tokio::io::AsyncWriteExt;
use crate::index_controller::UpdateStatus; use super::{Data, is_index_uid_valid};
use crate::index_controller::{IndexController, Settings, IndexSettings, IndexMetadata}; use crate::index_controller::{UpdateStatus, IndexController, Settings, IndexSettings, IndexMetadata};
use super::Data;
impl Data { impl Data {
pub async fn add_documents<B, E>( pub async fn add_documents<B, E>(
@ -22,6 +22,10 @@ impl Data {
B: Deref<Target = [u8]>, B: Deref<Target = [u8]>,
E: std::error::Error + Send + Sync + 'static, E: std::error::Error + Send + Sync + 'static,
{ {
if !is_index_uid_valid(index.as_ref()) {
bail!("invalid index uid: {:?}", index.as_ref())
}
let file = tokio::task::spawn_blocking(tempfile::tempfile).await?; let file = tokio::task::spawn_blocking(tempfile::tempfile).await?;
let file = tokio::fs::File::from_std(file?); let file = tokio::fs::File::from_std(file?);
let mut encoder = GzipEncoder::new(file); let mut encoder = GzipEncoder::new(file);
@ -57,6 +61,9 @@ impl Data {
index: impl AsRef<str> + Send + Sync + 'static, index: impl AsRef<str> + Send + Sync + 'static,
settings: Settings settings: Settings
) -> anyhow::Result<UpdateStatus> { ) -> anyhow::Result<UpdateStatus> {
if !is_index_uid_valid(index.as_ref()) {
bail!("invalid index uid: {:?}", index.as_ref())
}
let index_controller = self.index_controller.clone(); let index_controller = self.index_controller.clone();
let update = tokio::task::spawn_blocking(move || index_controller.update_settings(index, settings)).await??; let update = tokio::task::spawn_blocking(move || index_controller.update_settings(index, settings)).await??;
Ok(update.into()) Ok(update.into())

View File

@ -1,7 +1,8 @@
use tempdir::TempDir; use actix_web::http::StatusCode;
use byte_unit::{Byte, ByteUnit}; use byte_unit::{Byte, ByteUnit};
use serde_json::Value; use serde_json::Value;
use actix_web::http::StatusCode; use tempdir::TempDir;
use urlencoding::encode;
use meilisearch_http::data::Data; use meilisearch_http::data::Data;
use meilisearch_http::option::{Opt, IndexerOpts}; use meilisearch_http::option::{Opt, IndexerOpts};
@ -60,7 +61,7 @@ impl Server {
/// Returns a view to an index. There is no guarantee that the index exists. /// Returns a view to an index. There is no guarantee that the index exists.
pub fn index<'a>(&'a self, uid: impl AsRef<str>) -> Index<'a> { pub fn index<'a>(&'a self, uid: impl AsRef<str>) -> Index<'a> {
Index { Index {
uid: uid.as_ref().to_string(), uid: encode(uid.as_ref()),
service: &self.service, service: &self.service,
} }
} }

View File

@ -45,6 +45,22 @@ async fn add_documents_no_index_creation() {
assert_eq!(response["primaryKey"], "id"); assert_eq!(response["primaryKey"], "id");
} }
#[actix_rt::test]
async fn document_add_create_index_bad_uid() {
let server = Server::new().await;
let index = server.index("883 fj!");
let (_response, code) = index.add_documents(json!([]), None).await;
assert_eq!(code, 400);
}
#[actix_rt::test]
async fn document_update_create_index_bad_uid() {
let server = Server::new().await;
let index = server.index("883 fj!");
let (_response, code) = index.update_documents(json!([]), None).await;
assert_eq!(code, 400);
}
#[actix_rt::test] #[actix_rt::test]
async fn document_addition_with_primary_key() { async fn document_addition_with_primary_key() {
let server = Server::new().await; let server = Server::new().await;

View File

@ -47,12 +47,10 @@ async fn create_existing_index() {
assert_eq!(code, 400); assert_eq!(code, 400);
} }
// test fails (issue #46)
#[actix_rt::test] #[actix_rt::test]
#[ignore]
async fn create_with_invalid_index_uid() { async fn create_with_invalid_index_uid() {
let server = Server::new().await; let server = Server::new().await;
let index = server.index("test test"); let index = server.index("test test#!");
let (_, code) = index.create(None).await; let (_, code) = index.create(None).await;
assert_eq!(code, 400); assert_eq!(code, 400);
} }

View File

@ -90,6 +90,15 @@ async fn update_setting_unexisting_index() {
assert_eq!(code, 200); assert_eq!(code, 200);
} }
#[actix_rt::test]
async fn update_setting_unexisting_index_invalid_uid() {
let server = Server::new().await;
let index = server.index("test##! ");
let (_response, code) = index.update_settings(json!({})).await;
println!("response: {}", _response);
assert_eq!(code, 400);
}
macro_rules! test_setting_routes { macro_rules! test_setting_routes {
($($setting:ident), *) => { ($($setting:ident), *) => {
$( $(