Added user input validation

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rsvenv"
-version = "0.2.0"
+version = "0.3.0"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
@@ -12,13 +12,15 @@ error-stack = "0.4.1"
 glob = "0.3.1"
 itertools = "0.12.1"
 lazy_static = "1.4.0"
+regex = "1.10.4"
 serde = { version = "1.0.197", features = ["serde_derive"] }
 serde_derive = "1.0.197"
 shellexpand = "3.1.0"
 simplelog = { version = "0.12.2", features = ["paris"] }
 sysinfo = "0.30.7"
 
 [dev-dependencies]
+cfg-if = "1.0.0"
 mockall = "0.12.1"
 tempfile = "3.10.1"
 

README.md

@@ -24,7 +24,7 @@ rsvenv list
 Result will be something like this
 ```
 Rsenv environments:
-	test_rsenv
+	3.11.8/test_rsenv
 Pyenv environments:
 	3.12.1/envs/armis
 	3.9.9/envs/proxy_pac
@@ -36,6 +36,9 @@ To create a new virtual environment
 ```bash
 rsvenv create path/to/python venv_name
 ```
+The created virtual environment name will be python_version/venv_name
+
+
 After this it is possible to use this venv in current directory
 ```bash
 rsvenv use venv_name

src/commands/activate.rs

@@ -4,7 +4,7 @@ use simplelog::{debug, error};
 
 use crate::{
     errors::{CommandExecutionError, VirtualEnvError},
-    virtualenv::{pyenv::Pyenv, rsenv::Rsenv, traits::VirtualEnvCompatible, VirtualEnvironment},
+    virtualenv::{pyenv::Pyenv, rsenv::Rsenv, VirtualEnvironment},
 };
 
 #[derive(Debug, Parser)]
@@ -13,28 +13,29 @@ pub struct Command {
     virtualenv: String,
 }
 
-fn try_activate(f: &dyn VirtualEnvCompatible, venv: &String) -> Result<(), VirtualEnvError> {
-    if f.list().contains(venv) {
-        let venv_struct = VirtualEnvironment { kind: f };
-        if let Err(e) = venv_struct.activate(Some(venv)) {
+fn try_activate(v: VirtualEnvironment, venv: &String) -> Result<(), VirtualEnvError> {
+    if v.kind.list().contains(venv) {
+        if let Err(e) = v.activate(Some(venv)) {
             error!("{e}");
             return Err(e);
         };
+        return Ok(());
     }
-    Err(Report::new(VirtualEnvError::NotVirtualEnv(
-        venv.to_string(),
-    )))
+    Err(
+        Report::new(VirtualEnvError::NotVirtualEnv(venv.to_string()))
+            .attach_printable("{venv} is not a virtual environment"),
+    )
 }
 
 impl Command {
     pub fn execute(&self) -> Result<(), CommandExecutionError> {
         if let Err(e) = VirtualEnvironment::deactivate(true) {
             debug!("{e}");
         };
-        if let Ok(_) = try_activate(&Rsenv, &self.virtualenv) {
+        if try_activate(VirtualEnvironment { kind: &Rsenv }, &self.virtualenv).is_ok() {
             return Ok(());
         }
-        if let Ok(_) = try_activate(&Pyenv, &self.virtualenv) {
+        if try_activate(VirtualEnvironment { kind: &Pyenv }, &self.virtualenv).is_ok() {
             return Ok(());
         }
         error!(

src/errors.rs

@@ -11,6 +11,7 @@ pub enum VirtualEnvError {
     CreatingError,
     IOError,
     ConfigurationError,
+    IncorrectName,
 }
 
 impl fmt::Display for VirtualEnvError {
@@ -27,6 +28,7 @@ impl fmt::Display for VirtualEnvError {
             }
             VirtualEnvError::ConfigurationError => "Configuration error".to_owned(),
             VirtualEnvError::CreatingError => "Error while creating virtual environment".to_owned(),
+            VirtualEnvError::IncorrectName => "Incorrect virtual environment name".to_owned(),
         };
         f.write_str(&data)
     }
@@ -46,3 +48,17 @@ impl fmt::Display for CommandExecutionError {
 }
 
 impl Context for CommandExecutionError {}
+
+#[derive(Debug)]
+pub enum PythonInterpreterError {
+    UnableToDetectVersion,
+    CreateVenvError,
+}
+
+impl fmt::Display for PythonInterpreterError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("Error running python interpreter")
+    }
+}
+
+impl Context for PythonInterpreterError {}

src/virtualenv.rs

@@ -1,5 +1,6 @@
 pub mod local;
 pub mod pyenv;
+mod python;
 pub mod rsenv;
 pub mod traits;
 mod utils;
@@ -10,20 +11,19 @@ use simplelog::info;
 use std::io;
 use std::io::Write;
 
-use crate::errors::VirtualEnvError;
-use crate::virtualenv::utils::{get_current_dir, is_virtualenv};
-
 use self::local::Local;
 use self::pyenv::Pyenv;
 use self::rsenv::Rsenv;
 use self::traits::VirtualEnvCompatible;
+use crate::errors::VirtualEnvError;
+use crate::virtualenv::utils::{get_current_dir, is_virtualenv};
 
-pub struct VirtualEnvironment<'a> {
+pub struct VirtualEnvironment {
     // Venv path
-    pub kind: &'a dyn VirtualEnvCompatible,
+    pub kind: &'static dyn VirtualEnvCompatible,
 }
 
-impl VirtualEnvironment<'_> {
+impl VirtualEnvironment {
     pub fn detect() -> Option<Self> {
         if Rsenv.relevant() {
             return Some(Self { kind: &Rsenv });

src/virtualenv/local.rs

@@ -23,7 +23,7 @@ impl VirtualEnvCompatible for Local {
     fn venv_name(&self) -> Result<std::string::String, VirtualEnvError> {
         let current_path = self.root_dir()?;
         for local_venv_path in ["venv", ".venv", "virtualenv", ".virtualenv"] {
-            if let Ok(_) = is_virtualenv(&current_path.join(local_venv_path)) {
+            if is_virtualenv(&current_path.join(local_venv_path)).is_ok() {
                 return Ok(local_venv_path.to_string());
             }
         }

src/virtualenv/python.rs

@@ -0,0 +1,72 @@
+use std::{path::PathBuf, process, str::FromStr};
+
+use error_stack::{Report, Result, ResultExt};
+use glob::Pattern;
+use simplelog::info;
+
+use crate::errors::PythonInterpreterError;
+
+pub struct PythonInterpreter<'a> {
+    pub version: String,
+    pub interpreter: &'a String,
+}
+
+impl<'a> PythonInterpreter<'a> {
+    pub fn new(interpreter: &'a String) -> Result<Self, PythonInterpreterError> {
+        let version = PythonInterpreter::detect_version(interpreter)?;
+        Ok(PythonInterpreter {
+            version,
+            interpreter,
+        })
+    }
+
+    pub fn create_venv(&self, path: &PathBuf) -> Result<(), PythonInterpreterError> {
+        info!(
+            "Executing {} -m venv {}",
+            self.interpreter,
+            &path.as_path().display()
+        );
+        let status = process::Command::new(self.interpreter)
+            .arg("-m")
+            .arg("venv")
+            .arg(path)
+            .status()
+            .change_context(PythonInterpreterError::CreateVenvError)?;
+
+        if status.code().unwrap_or_default() > 0 {
+            return Err(
+                Report::new(PythonInterpreterError::CreateVenvError).attach_printable(format!(
+                    "Error creating venv {}: ",
+                    path.as_path().display()
+                )),
+            );
+        }
+
+        Ok(())
+    }
+
+    fn detect_version(interpreter: &String) -> Result<String, PythonInterpreterError> {
+        info!("Detecting python version");
+        let output = process::Command::new(interpreter)
+            .arg("-c")
+            .arg(r#"import platform; print(platform.python_version())"#)
+            .output()
+            .change_context(PythonInterpreterError::UnableToDetectVersion)
+            .attach_printable_lazy(|| "{:?}")?;
+        if output.status.code().unwrap_or(1) != 0 {
+            return Err(Report::new(PythonInterpreterError::UnableToDetectVersion)
+                .attach_printable(format!("Python executable returned {}", output.status,)));
+        }
+        let mut version = String::from_utf8(output.stdout)
+            .change_context(PythonInterpreterError::UnableToDetectVersion)
+            .attach_printable_lazy(|| "unable to read version from stdout: {:?}")?;
+
+        version = version.trim().into();
+        if !Pattern::from_str("*.*.*").unwrap().matches(&version) {
+            return Err(Report::new(PythonInterpreterError::UnableToDetectVersion)
+                .attach_printable(format!("Unexpected python version {}", &version)));
+        }
+
+        Ok(version)
+    }
+}

src/virtualenv/rsenv.rs

@@ -2,15 +2,16 @@ use std::{
     collections::HashSet,
     fs::{self, File},
     path::{Path, PathBuf},
-    process,
 };
 
 use crate::{configuration::SETTINGS, errors::VirtualEnvError};
 use error_stack::{Report, Result, ResultExt};
+use regex::Regex;
 use simplelog::{error, info};
 use std::io::Write;
 
 use super::{
+    python::PythonInterpreter,
     traits::VirtualEnvCompatible,
     utils::{get_current_dir, get_venvs_by_glob},
 };
@@ -19,8 +20,25 @@ use super::{
 pub struct Rsenv;
 
 impl Rsenv {
+    pub fn validate_name(name: &str) -> Result<(), VirtualEnvError> {
+        if Regex::new(r"^[\w._]*$").unwrap().is_match(name)
+            || Regex::new(r"^[\w._]*\/[\w._]*$").unwrap().is_match(name)
+        {
+            return Ok(());
+        }
+        Err(Report::new(VirtualEnvError::IncorrectName).attach("name is invalid"))
+    }
+
     pub fn create(&self, name: &String, python: &String) -> Result<(), VirtualEnvError> {
-        if self.list().contains(name) {
+        let interpreter =
+            PythonInterpreter::new(python).change_context(VirtualEnvError::CreatingError)?;
+
+        let name_with_version = format!("{}/{}", &interpreter.version, name);
+        let existing = self.list();
+
+        Rsenv::validate_name(name)?;
+        if existing.contains(name) || existing.contains(&name_with_version) {
+            error!("Virtual environment {name} exists");
             return Err(Report::new(VirtualEnvError::AlreadyExists(
                 name.to_string(),
             )));
@@ -36,33 +54,16 @@ impl Rsenv {
             info!("Created root dir");
         }
 
-        let venv_path = path.join(name);
-        info!(
-            "Executing {} -m venv {}",
-            python,
-            &venv_path.as_path().display()
-        );
-        let status = process::Command::new(python)
-            .arg("-m")
-            .arg("venv")
-            .arg(venv_path)
-            .status()
+        let venv_path = path.join(&interpreter.version).join(name);
+        interpreter
+            .create_venv(&venv_path)
             .change_context(VirtualEnvError::CreatingError)?;
-
-        if status.code().unwrap_or_default() > 0 {
-            return Err(
-                Report::new(VirtualEnvError::CreatingError).attach_printable(format!(
-                    "Error creating venv {}: ",
-                    path.join(name).as_path().display()
-                )),
-            );
-        }
-
-        info!("Created virtual environment {name}");
+        info!("Created venv {name_with_version}");
         Ok(())
     }
 
     pub fn delete(&self, name: String) -> Result<(), VirtualEnvError> {
+        Rsenv::validate_name(&name)?;
         if !self.list().contains(&name) {
             error!("Virtual environment `{name}` is not found");
             return Err(Report::new(VirtualEnvError::NotVirtualEnv(name.clone()))
@@ -91,7 +92,7 @@ impl VirtualEnvCompatible for Rsenv {
         .change_context(VirtualEnvError::ConfigurationError)
         .attach_printable("unable to expand SETTINGS.path to the actual path")?
         .to_string();
-        Ok(Path::new(&expanded).to_path_buf().join("versions"))
+        Ok(Path::new(&expanded).to_path_buf().join("venvs"))
     }
 
     fn list(&self) -> HashSet<String> {
@@ -135,3 +136,22 @@ impl VirtualEnvCompatible for Rsenv {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+
+    use super::*;
+
+    #[test]
+    fn test_name_ok() {
+        assert!(Rsenv::validate_name(&String::from("good_name")).is_ok());
+        assert!(Rsenv::validate_name(&String::from("Good_nAme")).is_ok());
+        assert!(Rsenv::validate_name(&String::from("Good_nAme/asdfadsf")).is_ok());
+    }
+    #[test]
+    fn test_bad_name() {
+        assert!(Rsenv::validate_name(&String::from("bad!name")).is_err());
+        assert!(Rsenv::validate_name(&String::from("Good_nAme/asdfadsf/smth")).is_err());
+        assert!(Rsenv::validate_name(&String::from("Good_nAme aa")).is_err());
+    }
+}

src/virtualenv/utils.rs

@@ -1,16 +1,21 @@
-use std::{collections::HashSet, fs, path::PathBuf};
+use std::{
+    collections::HashSet,
+    fs,
+    path::{Path, PathBuf},
+};
 
 use error_stack::{Report, Result, ResultExt};
 
 use crate::errors::VirtualEnvError;
 
-pub fn is_virtualenv(path: &PathBuf) -> Result<(), VirtualEnvError> {
+pub fn is_virtualenv(path: &Path) -> Result<(), VirtualEnvError> {
     if fs::metadata(path.join("bin").join("activate")).is_ok_and(|x| x.is_file()) {
         Ok(())
     } else {
         Err(Report::new(VirtualEnvError::NotVirtualEnv(
             path.to_string_lossy().to_string(),
-        )))
+        ))
+        .attach_printable("Is not a virtual environment"))
     }
 }
 
@@ -28,12 +33,14 @@ pub fn get_venvs_by_glob(glob: String, dir: &PathBuf) -> Result<HashSet<String>,
         .flatten()
     {
         let value = path
-            .strip_prefix(&dir)
+            .strip_prefix(dir)
             .attach_printable("Unable to strip prefix")
             .change_context(VirtualEnvError::VenvBuildError)?
             .to_str();
-        if is_virtualenv(&path).is_ok() && !value.is_none() {
-            result.insert(String::from(value.unwrap()));
+        if let Some(unwrapped) = value {
+            if is_virtualenv(&path).is_ok() {
+                result.insert(String::from(unwrapped));
+            }
         }
     }