From 7c5547b2ff3e77b8116d33ef282a5c986141db22 Mon Sep 17 00:00:00 2001 From: Ruben Arts <ruben.arts@hotmail.com> Date: Tue, 16 Jan 2024 16:31:59 +0100 Subject: [PATCH 1/2] feat: make environment name parsing strict --- src/project/manifest/environment.rs | 96 +++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 4 deletions(-) diff --git a/src/project/manifest/environment.rs b/src/project/manifest/environment.rs index ad9b1c829..9c2634151 100644 --- a/src/project/manifest/environment.rs +++ b/src/project/manifest/environment.rs @@ -1,8 +1,12 @@ use crate::consts; use crate::utils::spanned::PixiSpanned; +use miette::Diagnostic; +use regex::Regex; use serde::{self, Deserialize, Deserializer}; use std::borrow::Borrow; use std::hash::{Hash, Hasher}; +use std::str::FromStr; +use thiserror::Error; /// The name of an environment. This is either a string or default for the default environment. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] @@ -33,16 +37,37 @@ impl Borrow<str> for EnvironmentName { self.as_str() } } +#[derive(Debug, Clone, Error, Diagnostic, PartialEq)] +#[error("Failed to parse environment name '{attempted_parse}', please use only lowercase letters, numbers and dashes")] +pub struct ParseEnvironmentNameError { + /// The string that was attempted to be parsed. + pub attempted_parse: String, +} + +impl FromStr for EnvironmentName { + type Err = ParseEnvironmentNameError; + fn from_str(s: &str) -> Result<Self, Self::Err> { + let re = Regex::new(r"^[a-z0-9-]+$").unwrap(); // Compile the regex + if !re.is_match(s) { + // Return an error if the string does not match the regex + return Err(ParseEnvironmentNameError { + attempted_parse: s.to_string(), + }); + } + match s { + consts::DEFAULT_ENVIRONMENT_NAME => Ok(EnvironmentName::Default), + _ => Ok(EnvironmentName::Named(s.to_string())), + } + } +} impl<'de> Deserialize<'de> for EnvironmentName { fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> where D: Deserializer<'de>, { - match String::deserialize(deserializer)? { - name if name == consts::DEFAULT_ENVIRONMENT_NAME => Ok(EnvironmentName::Default), - name => Ok(EnvironmentName::Named(name)), - } + let name = String::deserialize(deserializer)?; + EnvironmentName::from_str(&name).map_err(serde::de::Error::custom) } } @@ -115,3 +140,66 @@ impl<'de> Deserialize<'de> for TomlEnvironmentMapOrSeq { .deserialize(deserializer) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_environment_name_from_str() { + assert_eq!( + EnvironmentName::from_str("default").unwrap(), + EnvironmentName::Default + ); + assert_eq!( + EnvironmentName::from_str("foo").unwrap(), + EnvironmentName::Named("foo".to_string()) + ); + assert_eq!( + EnvironmentName::from_str("foo_bar").unwrap_err(), + ParseEnvironmentNameError { + attempted_parse: "foo_bar".to_string() + } + ); + + assert!(EnvironmentName::from_str("foo-bar").is_ok()); + assert!(EnvironmentName::from_str("foo1").is_ok()); + assert!(EnvironmentName::from_str("py39").is_ok()); + + assert!(EnvironmentName::from_str("foo bar").is_err()); + assert!(EnvironmentName::from_str("foo_bar").is_err()); + assert!(EnvironmentName::from_str("foo/bar").is_err()); + assert!(EnvironmentName::from_str("foo\\bar").is_err()); + assert!(EnvironmentName::from_str("foo:bar").is_err()); + assert!(EnvironmentName::from_str("foo;bar").is_err()); + assert!(EnvironmentName::from_str("foo?bar").is_err()); + assert!(EnvironmentName::from_str("foo!bar").is_err()); + assert!(EnvironmentName::from_str("py3.9").is_err()); + assert!(EnvironmentName::from_str("py-3.9").is_err()); + assert!(EnvironmentName::from_str("py_3.9").is_err()); + assert!(EnvironmentName::from_str("py/3.9").is_err()); + assert!(EnvironmentName::from_str("py\\3.9").is_err()); + assert!(EnvironmentName::from_str("Py").is_err()); + assert!(EnvironmentName::from_str("Py3").is_err()); + assert!(EnvironmentName::from_str("Py39").is_err()); + assert!(EnvironmentName::from_str("Py-39").is_err()); + } + + #[test] + fn test_environment_name_as_str() { + assert_eq!(EnvironmentName::Default.as_str(), "default"); + assert_eq!(EnvironmentName::Named("foo".to_string()).as_str(), "foo"); + } + + #[test] + fn test_deserialize_environment_name() { + assert_eq!( + serde_json::from_str::<EnvironmentName>("\"default\"").unwrap(), + EnvironmentName::Default + ); + assert_eq!( + serde_json::from_str::<EnvironmentName>("\"foo\"").unwrap(), + EnvironmentName::Named("foo".to_string()) + ); + } +} From cd8fe313c3a319ec94534d9c47a0dc6556686b21 Mon Sep 17 00:00:00 2001 From: Ruben Arts <ruben.arts@hotmail.com> Date: Tue, 16 Jan 2024 17:28:30 +0100 Subject: [PATCH 2/2] fix: use lazy static --- Cargo.lock | 1 + Cargo.toml | 1 + src/project/manifest/environment.rs | 8 ++++++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 909ddc920..229da72bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2693,6 +2693,7 @@ dependencies = [ "insta", "is_executable", "itertools 0.12.0", + "lazy_static", "libc", "miette", "minijinja", diff --git a/Cargo.toml b/Cargo.toml index b029f7808..4a3299c87 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,6 +35,7 @@ indicatif = "0.17.7" insta = { version = "1.34.0", features = ["yaml"] } is_executable = "1.0.1" itertools = "0.12.0" +lazy_static = "1.4.0" miette = { version = "5.10.0", features = ["fancy", "supports-color", "supports-hyperlinks", "supports-unicode", "terminal_size", "textwrap"] } minijinja = { version = "1.0.11", features = ["builtins"] } once_cell = "1.19.0" diff --git a/src/project/manifest/environment.rs b/src/project/manifest/environment.rs index 9c2634151..90d78a020 100644 --- a/src/project/manifest/environment.rs +++ b/src/project/manifest/environment.rs @@ -1,5 +1,6 @@ use crate::consts; use crate::utils::spanned::PixiSpanned; +use lazy_static::lazy_static; use miette::Diagnostic; use regex::Regex; use serde::{self, Deserialize, Deserializer}; @@ -47,8 +48,11 @@ pub struct ParseEnvironmentNameError { impl FromStr for EnvironmentName { type Err = ParseEnvironmentNameError; fn from_str(s: &str) -> Result<Self, Self::Err> { - let re = Regex::new(r"^[a-z0-9-]+$").unwrap(); // Compile the regex - if !re.is_match(s) { + lazy_static! { + static ref REGEX: Regex = Regex::new(r"^[a-z0-9-]+$").expect("Regex should be able to compile"); // Compile the regex + } + + if !REGEX.is_match(s) { // Return an error if the string does not match the regex return Err(ParseEnvironmentNameError { attempted_parse: s.to_string(),