From ef638fd5627e91c4c90c4fb01875da17a5b25e7e Mon Sep 17 00:00:00 2001 From: Adrien Guillo Date: Thu, 21 Dec 2023 19:31:25 +0100 Subject: [PATCH] Fix ugly code --- quickwit/quickwit-config/src/lib.rs | 34 ++++++++----------- .../src/search_api/rest_handler.rs | 31 ++++++++++------- 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/quickwit/quickwit-config/src/lib.rs b/quickwit/quickwit-config/src/lib.rs index e4406ce259c..c98b088a89e 100644 --- a/quickwit/quickwit-config/src/lib.rs +++ b/quickwit/quickwit-config/src/lib.rs @@ -23,7 +23,7 @@ use std::str::FromStr; use anyhow::{bail, Context}; use json_comments::StripComments; -use once_cell::sync::OnceCell; +use once_cell::sync::Lazy; use quickwit_common::net::is_valid_hostname; use quickwit_common::uri::Uri; use regex::Regex; @@ -105,12 +105,10 @@ pub struct ConfigApiSchemas; /// Checks whether an identifier conforms to Quickwit object naming conventions. pub fn validate_identifier(label: &str, value: &str) -> anyhow::Result<()> { - static IDENTIFIER_REGEX: OnceCell = OnceCell::new(); - - if IDENTIFIER_REGEX - .get_or_init(|| Regex::new(r"^[a-zA-Z][a-zA-Z0-9-_\.]{2,254}$").expect("Failed to compile regular expression. This should never happen! Please, report on https://github.com/quickwit-oss/quickwit/issues.")) - .is_match(value) - { + static IDENTIFIER_REGEX: Lazy = Lazy::new(|| { + Regex::new(r"^[a-zA-Z][a-zA-Z0-9-_\.]{2,254}$").expect("regular expression should compile") + }); + if IDENTIFIER_REGEX.is_match(value) { return Ok(()); } bail!( @@ -123,34 +121,30 @@ pub fn validate_identifier(label: &str, value: &str) -> anyhow::Result<()> { /// Index ID patterns accept the same characters as identifiers AND accept `*` /// chars to allow for glob-like patterns. pub fn validate_index_id_pattern(pattern: &str) -> anyhow::Result<()> { - static IDENTIFIER_REGEX_WITH_GLOB_PATTERN: OnceCell = OnceCell::new(); - - if !IDENTIFIER_REGEX_WITH_GLOB_PATTERN - .get_or_init(|| Regex::new(r"^[a-zA-Z\*][a-zA-Z0-9-_\.\*]{0,254}$").expect("Failed to compile regular expression. This should never happen! Please, report on https://github.com/quickwit-oss/quickwit/issues.")) - .is_match(pattern) - { + static IDENTIFIER_REGEX_WITH_GLOB_PATTERN: Lazy = Lazy::new(|| { + Regex::new(r"^[a-zA-Z\*][a-zA-Z0-9-_\.\*]{0,254}$") + .expect("regular expression should compile") + }); + if !IDENTIFIER_REGEX_WITH_GLOB_PATTERN.is_match(pattern) { bail!( - "index ID pattern `{pattern}` is invalid. patterns must match the following regular \ + "index ID pattern `{pattern}` is invalid: patterns must match the following regular \ expression: `^[a-zA-Z\\*][a-zA-Z0-9-_\\.\\*]{{0,254}}$`" ); } - // Forbid multiple stars in the pattern to force the user making simpler patterns // as multiple stars does not bring any value. if pattern.contains("**") { bail!( - "index ID pattern `{pattern}` is invalid. patterns must not contain multiple \ + "index ID pattern `{pattern}` is invalid: patterns must not contain multiple \ consecutive `*`" ); } - // If there is no star in the pattern, we need at least 3 characters. if !pattern.contains('*') && pattern.len() < 3 { bail!( - "index ID pattern `{pattern}` is invalid. an index ID must have at least 3 characters" + "index ID pattern `{pattern}` is invalid: an index ID must have at least 3 characters" ); } - Ok(()) } @@ -283,6 +277,6 @@ mod tests { assert!(validate_index_id_pattern("foo!") .unwrap_err() .to_string() - .contains("index ID pattern `foo!` is invalid.")); + .contains("index ID pattern `foo!` is invalid:")); } } diff --git a/quickwit/quickwit-serve/src/search_api/rest_handler.rs b/quickwit/quickwit-serve/src/search_api/rest_handler.rs index b073e30b21e..5bfdba93975 100644 --- a/quickwit/quickwit-serve/src/search_api/rest_handler.rs +++ b/quickwit/quickwit-serve/src/search_api/rest_handler.rs @@ -20,7 +20,6 @@ use std::convert::TryFrom; use std::sync::Arc; -use anyhow::Context; use futures::stream::StreamExt; use hyper::header::HeaderValue; use hyper::HeaderMap; @@ -57,21 +56,27 @@ use crate::{with_arg, BodyFormat}; pub struct SearchApi; pub(crate) async fn extract_index_id_patterns( - comma_separated_index_patterns: String, + comma_separated_index_id_patterns: String, ) -> Result, Rejection> { - let index_pattern = percent_decode_str(&comma_separated_index_patterns) - .decode_utf8() - .context("index pattern does not contain valid utf8 characters") - .map_err(|error| crate::rest::InvalidArgument(error.to_string()))?; - - let mut index_ids_patterns = Vec::new(); - for index_id_pattern in index_pattern.split(',').collect::>() { + let percent_decoded_comma_separated_index_id_patterns = + percent_decode_str(&comma_separated_index_id_patterns) + .decode_utf8() + .map_err(|error| { + let message = format!( + "failed to percent decode comma-separated index ID patterns \ + `{comma_separated_index_id_patterns}`: {error}" + ); + crate::rest::InvalidArgument(message) + })?; + let mut index_id_patterns = Vec::new(); + + for index_id_pattern in percent_decoded_comma_separated_index_id_patterns.split(',') { validate_index_id_pattern(index_id_pattern) .map_err(|error| crate::rest::InvalidArgument(error.to_string()))?; - index_ids_patterns.push(index_id_pattern.to_string()); + index_id_patterns.push(index_id_pattern.to_string()); } - assert!(!index_ids_patterns.is_empty()); - Ok(index_ids_patterns) + assert!(!index_id_patterns.is_empty()); + Ok(index_id_patterns) } #[derive(Debug, Default, Eq, PartialEq, Deserialize, utoipa::ToSchema)] @@ -654,7 +659,7 @@ mod tests { .unwrap(); assert_eq!( rejection.0, - "index ID pattern `quickwit-demo-index**` is invalid. patterns must not contain \ + "index ID pattern `quickwit-demo-index**` is invalid: patterns must not contain \ multiple consecutive `*`" ); }