From 1b0957ce9140c30bf45123478aec618ae775c005 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Tue, 4 Jun 2024 16:27:31 +0200 Subject: [PATCH] fix: nushell environment variable quoting (#719) --- crates/rattler_shell/src/shell/mod.rs | 109 +++++++++++++++++--------- 1 file changed, 72 insertions(+), 37 deletions(-) diff --git a/crates/rattler_shell/src/shell/mod.rs b/crates/rattler_shell/src/shell/mod.rs index 924e41cf2..b170045dc 100644 --- a/crates/rattler_shell/src/shell/mod.rs +++ b/crates/rattler_shell/src/shell/mod.rs @@ -1,19 +1,23 @@ -//! This module contains the [`Shell`] trait and implementations for various shells. +//! This module contains the [`Shell`] trait and implementations for various +//! shells. -use crate::activation::PathModificationBehavior; -use enum_dispatch::enum_dispatch; -use itertools::Itertools; -use rattler_conda_types::Platform; -use std::collections::HashMap; -use std::ffi::OsStr; -use std::process::Command; -use std::str::FromStr; use std::{ + borrow::Cow, + collections::HashMap, + ffi::OsStr, fmt::Write, path::{Path, PathBuf}, + process::Command, + str::FromStr, }; + +use enum_dispatch::enum_dispatch; +use itertools::Itertools; +use rattler_conda_types::Platform; use thiserror::Error; +use crate::activation::PathModificationBehavior; + /// A trait for generating shell scripts. /// The trait is implemented for each shell individually. /// @@ -32,7 +36,8 @@ use thiserror::Error; /// ``` #[enum_dispatch(ShellEnum)] pub trait Shell { - /// Write a command to the script that forces the usage of UTF8-encoding for the shell script. + /// Write a command to the script that forces the usage of UTF8-encoding for + /// the shell script. fn force_utf8(&self, _f: &mut impl Write) -> std::fmt::Result { Ok(()) } @@ -46,7 +51,8 @@ pub trait Shell { /// Run a script in the current shell. fn run_script(&self, f: &mut impl Write, path: &Path) -> std::fmt::Result; - /// Test to see if the path can be executed by the shell, based on the extension of the path. + /// Test to see if the path can be executed by the shell, based on the + /// extension of the path. fn can_run_script(&self, path: &Path) -> bool { path.is_file() && path @@ -55,8 +61,8 @@ pub trait Shell { .map_or(false, |ext| ext == self.extension()) } - /// Executes a command in the current shell. Use [`Self::run_script`] when you want to run - /// another shell script. + /// Executes a command in the current shell. Use [`Self::run_script`] when + /// you want to run another shell script. fn run_command<'a>( &self, f: &mut impl Write, @@ -78,15 +84,16 @@ pub trait Shell { .map(|path| path.to_string_lossy().into_owned()) .collect_vec(); // Replace, Append, or Prepend the path variable to the paths. + let path_var = self.path_var(platform); match modification_behavior { PathModificationBehavior::Replace => (), - PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var("PATH")), - PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var("PATH")), + PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var(path_var)), + PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var(path_var)), } // Create the shell specific list of paths. let paths_string = paths_vec.join(self.path_seperator(platform)); - self.set_env_var(f, "PATH", paths_string.as_str()) + self.set_env_var(f, self.path_var(platform), paths_string.as_str()) } /// The extension that shell scripts for this interpreter usually use. @@ -95,7 +102,8 @@ pub trait Shell { /// The executable that can be called to start this shell. fn executable(&self) -> &str; - /// Constructs a [`Command`] that will execute the specified script by this shell. + /// Constructs a [`Command`] that will execute the specified script by this + /// shell. fn create_run_script_command(&self, path: &Path) -> Command; /// Path seperator @@ -107,6 +115,17 @@ pub trait Shell { } } + /// Returns the name of the PATH variable for the given platform. On + /// Windows, path variables are case-insensitive but not all shells treat + /// them case-insensitive. + fn path_var(&self, platform: &Platform) -> &str { + if platform.is_windows() { + "Path" + } else { + "PATH" + } + } + /// Format the environment variable for the shell. fn format_env_var(&self, var_name: &str) -> String { format!("${{{var_name}}}") @@ -122,8 +141,8 @@ pub trait Shell { writeln!(f, "/usr/bin/env") } - /// Write the script to the writer and do some post-processing for line-endings. - /// Only really relevant for cmd.exe scripts. + /// Write the script to the writer and do some post-processing for + /// line-endings. Only really relevant for cmd.exe scripts. fn write_script(&self, f: &mut impl std::io::Write, script: &str) -> std::io::Result<()> { f.write_all(script.as_bytes()) } @@ -204,7 +223,8 @@ impl Shell for Bash { let mut paths_vec = paths .iter() .map(|path| { - // check if we are on Windows, and if yes, convert native path to unix for (Git) Bash + // check if we are on Windows, and if yes, convert native path to unix for (Git) + // Bash if cfg!(windows) { match native_path_to_unix(path.to_string_lossy().as_ref()) { Ok(path) => path, @@ -222,15 +242,16 @@ impl Shell for Bash { .collect_vec(); // Replace, Append, or Prepend the path variable to the paths. + let path_var = self.path_var(platform); match modification_behavior { PathModificationBehavior::Replace => (), - PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var("PATH")), - PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var("PATH")), + PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var(path_var)), + PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var(path_var)), } // Create the shell specific list of paths. let paths_string = paths_vec.join(self.path_seperator(platform)); - self.set_env_var(f, "PATH", paths_string.as_str()) + self.set_env_var(f, self.path_var(platform), paths_string.as_str()) } fn extension(&self) -> &str { @@ -244,7 +265,8 @@ impl Shell for Bash { fn create_run_script_command(&self, path: &Path) -> Command { let mut cmd = Command::new(self.executable()); - // check if we are on Windows, and if yes, convert native path to unix for (Git) Bash + // check if we are on Windows, and if yes, convert native path to unix for (Git) + // Bash if cfg!(windows) { cmd.arg(native_path_to_unix(path.to_str().unwrap()).unwrap()); } else { @@ -510,6 +532,13 @@ impl Shell for Fish { fn escape_backslashes(s: &str) -> String { s.replace('\\', "\\\\") } +fn quote_if_required(s: &str) -> Cow<'_, str> { + if s.contains(|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '-') { + Cow::Owned(format!("\"{s}\"")) + } else { + Cow::Borrowed(s) + } +} /// A [`Shell`] implementation for the Bash shell. #[derive(Debug, Clone, Copy, Default)] @@ -518,15 +547,20 @@ pub struct NuShell; impl Shell for NuShell { fn set_env_var(&self, f: &mut impl Write, env_var: &str, value: &str) -> std::fmt::Result { // escape backslashes for Windows (make them double backslashes) - writeln!(f, "$env.{} = \"{}\"", env_var, escape_backslashes(value)) + writeln!( + f, + "$env.{} = \"{}\"", + quote_if_required(env_var), + escape_backslashes(value) + ) } fn unset_env_var(&self, f: &mut impl Write, env_var: &str) -> std::fmt::Result { - writeln!(f, "hide-env {env_var}") + writeln!(f, "hide-env {}", quote_if_required(env_var)) } fn run_script(&self, f: &mut impl Write, path: &Path) -> std::fmt::Result { - writeln!(f, "source \"{}\"", path.to_string_lossy()) + writeln!(f, "source-env \"{}\"", path.to_string_lossy()) } fn set_path( @@ -534,7 +568,7 @@ impl Shell for NuShell { f: &mut impl Write, paths: &[PathBuf], modification_behavior: PathModificationBehavior, - _platform: &Platform, + platform: &Platform, ) -> std::fmt::Result { let path = paths .iter() @@ -542,15 +576,16 @@ impl Shell for NuShell { .join(", "); // Replace, Append, or Prepend the path variable to the paths. + let path_var = self.path_var(platform); match modification_behavior { PathModificationBehavior::Replace => { - writeln!(f, "$env.PATH = [{path}]") + writeln!(f, "$env.{path_var} = [{path}]",) } PathModificationBehavior::Prepend => { - writeln!(f, "$env.PATH = ($env.PATH | prepend [{path}])") + writeln!(f, "$env.{path_var} = ($env.{path_var} | prepend [{path}])") } PathModificationBehavior::Append => { - writeln!(f, "$env.PATH = ($env.PATH | append [{path}])") + writeln!(f, "$env.{path_var} = ($env.{path_var} | append [{path}])") } } } @@ -603,11 +638,11 @@ impl ShellEnum { /// Determine the user's current shell from the environment /// - /// This will read the SHELL environment variable and try to determine which shell is in use - /// from that. + /// This will read the SHELL environment variable and try to determine which + /// shell is in use from that. /// - /// If SHELL is set, but contains a value that doesn't correspond to one of the supported shell - /// types, then return `None`. + /// If SHELL is set, but contains a value that doesn't correspond to one of + /// the supported shell types, then return `None`. pub fn from_env() -> Option { if let Some(env_shell) = std::env::var_os("SHELL") { Self::from_shell_path(env_shell) @@ -774,8 +809,8 @@ impl ShellScript { Ok(self) } - /// Add contents to the script. The contents will be added as is, so make sure to format it - /// correctly for the shell. + /// Add contents to the script. The contents will be added as is, so make + /// sure to format it correctly for the shell. pub fn append_script(&mut self, script: &Self) -> &mut Self { self.contents.push('\n'); self.contents.push_str(&script.contents);