Skip to content

Commit

Permalink
Merge pull request #1801 from GitoxideLabs/improvements
Browse files Browse the repository at this point in the history
improvements
  • Loading branch information
Byron authored Jan 23, 2025
2 parents cc7b614 + 435984b commit 02efddd
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 0 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions gix-command/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ doctest = false
[dependencies]
gix-trace = { version = "^0.1.12", path = "../gix-trace" }
gix-path = { version = "^0.10.14", path = "../gix-path" }
gix-quote = { version = "^0.4.15", path = "../gix-quote" }

bstr = { version = "1.5.0", default-features = false, features = ["std", "unicode"] }
shell-words = "1.0"
Expand Down
18 changes: 18 additions & 0 deletions gix-command/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ pub struct Prepare {
pub env: Vec<(OsString, OsString)>,
/// If `true`, we will use `shell_program` or `sh` to execute the `command`.
pub use_shell: bool,
/// If `true`, `command` is assumed to be a command or path to the program to execute, and it will be shell-quoted
/// to assure it will be executed as is and without splitting across whitespace.
pub quote_command: bool,
/// The name or path to the shell program to use instead of `sh`.
pub shell_program: Option<OsString>,
/// If `true` (default `true` on windows and `false` everywhere else)
Expand Down Expand Up @@ -119,6 +122,15 @@ mod prepare {
self.use_shell = true;
self
}

/// If [`with_shell()`](Self::with_shell()) is set, then quote the command to assure its path is left intact.
///
/// Note that this should not be used if the command is a script - quoting is only the right choice if it's known to be a program path.
pub fn with_quoted_command(mut self) -> Self {
self.quote_command = true;
self
}

/// Set the name or path to the shell `program` to use if a shell is to be used, to avoid using the default shell which is `sh`.
pub fn with_shell_program(mut self, program: impl Into<OsString>) -> Self {
self.shell_program = Some(program.into());
Expand Down Expand Up @@ -236,6 +248,11 @@ mod prepare {
cmd.arg("-c");
if !prep.args.is_empty() {
if prep.command.to_str().map_or(true, |cmd| !cmd.contains("$@")) {
if prep.quote_command {
if let Ok(command) = gix_path::os_str_into_bstr(&prep.command) {
prep.command = gix_path::from_bstring(gix_quote::single(command)).into();
}
}
prep.command.push(" \"$@\"");
} else {
gix_trace::debug!(
Expand Down Expand Up @@ -458,6 +475,7 @@ pub fn prepare(cmd: impl Into<OsString>) -> Prepare {
args: Vec::new(),
env: Vec::new(),
use_shell: false,
quote_command: false,
allow_manual_arg_splitting: cfg!(windows),
}
}
Expand Down
52 changes: 52 additions & 0 deletions gix-command/tests/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,44 @@ mod prepare {
);
}

#[test]
fn quoted_command_without_argument_splitting() {
let cmd = std::process::Command::from(
gix_command::prepare("ls")
.arg("--foo=a b")
.command_may_be_shell_script_disallow_manual_argument_splitting()
.with_shell()
.with_quoted_command(),
);
assert_eq!(
format!("{cmd:?}"),
quoted(&[SH, "-c", "\\'ls\\' \\\"$@\\\"", "--", "--foo=a b"]),
"looks strange thanks to debug printing, but is the right amount of quotes actually"
);
}

#[test]
fn quoted_windows_command_without_argument_splitting() {
let cmd = std::process::Command::from(
gix_command::prepare("C:\\Users\\O'Shaughnessy\\with space.exe")
.arg("--foo='a b'")
.command_may_be_shell_script_disallow_manual_argument_splitting()
.with_shell()
.with_quoted_command(),
);
assert_eq!(
format!("{cmd:?}"),
quoted(&[
SH,
"-c",
"\\'C:\\\\Users\\\\O\\'\\\\\\'\\'Shaughnessy\\\\with space.exe\\' \\\"$@\\\"",
"--",
"--foo=\\'a b\\'"
]),
"again, a lot of extra backslashes, but it's correct outside of the debug formatting"
);
}

#[test]
fn single_and_complex_arguments_will_not_auto_split_on_special_characters() {
let cmd = std::process::Command::from(
Expand Down Expand Up @@ -388,6 +426,20 @@ mod prepare {
We deal with it by not doubling the '$@' argument, which seems more flexible."
);
}

#[test]
fn script_with_dollar_at_has_no_quoting() {
let cmd = std::process::Command::from(
gix_command::prepare("echo \"$@\" >&2")
.command_may_be_shell_script()
.with_quoted_command()
.arg("store"),
);
assert_eq!(
format!("{cmd:?}"),
format!(r#""{SH}" "-c" "echo \"$@\" >&2" "--" "store""#)
);
}
}

mod spawn {
Expand Down

0 comments on commit 02efddd

Please sign in to comment.