Skip to content

Commit 05972f1

Browse files
committed
feat: add Prepare::with_shell_allow_argument_splitting() flag.
This flag is enabled on windows by default to emulate `git` which doesn't actually use a shell at all there. This should help to allow windows users to use more complex programs, which might not have been possible previously. It also allows certain areas of `gix` to indicate that they are safe to use with this flag, which always is the case when `git` is prefixed, like with credential helpers.
1 parent cfe078c commit 05972f1

File tree

4 files changed

+103
-10
lines changed

4 files changed

+103
-10
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gix-command/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ doctest = false
1414

1515
[dependencies]
1616
gix-trace = { version = "^0.1.3", path = "../gix-trace" }
17+
gix-path = { version = "^0.10.0", path = "../gix-path" }
1718

1819
bstr = { version = "1.5.0", default-features = false, features = ["std"] }
20+
shell-words = "1.0"
1921

2022
[dev-dependencies]
2123
gix-testtools = { path = "../tests/tools" }

gix-command/src/lib.rs

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,19 @@ pub struct Prepare {
2020
pub env: Vec<(OsString, OsString)>,
2121
/// If `true`, we will use `sh` to execute the `command`.
2222
pub use_shell: bool,
23+
/// If `true` (default `true` on windows and `false` everywhere else)
24+
/// we will see if it's safe to manually invoke `command` after splitting
25+
/// its arguments as a shell would do.
26+
/// Note that outside of windows, it's generally not advisable as this
27+
/// removes support for literal shell scripts with shell-builtins.
28+
///
29+
/// This mimics the behaviour we see with `git` on windows, which also
30+
/// won't invoke the shell there at all.
31+
///
32+
/// Only effective if `use_shell` is `true` as well, as the shell will
33+
/// be used as a fallback if it's not possible to split arguments as
34+
/// the command-line contains 'scripting'.
35+
pub allow_manual_arg_splitting: bool,
2336
}
2437

2538
mod prepare {
@@ -54,6 +67,14 @@ mod prepare {
5467
self
5568
}
5669

70+
/// Use a shell, but try to split arguments by hand if this be safely done without a shell.
71+
///
72+
/// If that's not the case, use a shell instead.
73+
pub fn with_shell_allow_argument_splitting(mut self) -> Self {
74+
self.allow_manual_arg_splitting = true;
75+
self.with_shell()
76+
}
77+
5778
/// Configure the process to use `stdio` for _stdin.
5879
pub fn stdin(mut self, stdio: Stdio) -> Self {
5980
self.stdin = stdio;
@@ -103,14 +124,38 @@ mod prepare {
103124
impl From<Prepare> for Command {
104125
fn from(mut prep: Prepare) -> Command {
105126
let mut cmd = if prep.use_shell {
106-
let mut cmd = Command::new(if cfg!(windows) { "sh" } else { "/bin/sh" });
107-
cmd.arg("-c");
108-
if !prep.args.is_empty() {
109-
prep.command.push(" \"$@\"")
127+
let split_args = prep
128+
.allow_manual_arg_splitting
129+
.then(|| {
130+
if gix_path::into_bstr(std::borrow::Cow::Borrowed(prep.command.as_ref()))
131+
.find_byteset(b"\\|&;<>()$`\n*?[#~%")
132+
.is_none()
133+
{
134+
prep.command
135+
.to_str()
136+
.and_then(|args| shell_words::split(args).ok().map(Vec::into_iter))
137+
} else {
138+
None
139+
}
140+
})
141+
.flatten();
142+
match split_args {
143+
Some(mut args) => {
144+
let mut cmd = Command::new(args.next().expect("non-empty input"));
145+
cmd.args(args);
146+
cmd
147+
}
148+
None => {
149+
let mut cmd = Command::new(if cfg!(windows) { "sh" } else { "/bin/sh" });
150+
cmd.arg("-c");
151+
if !prep.args.is_empty() {
152+
prep.command.push(" \"$@\"")
153+
}
154+
cmd.arg(prep.command);
155+
cmd.arg("--");
156+
cmd
157+
}
110158
}
111-
cmd.arg(prep.command);
112-
cmd.arg("--");
113-
cmd
114159
} else {
115160
Command::new(prep.command)
116161
};
@@ -140,5 +185,6 @@ pub fn prepare(cmd: impl Into<OsString>) -> Prepare {
140185
args: Vec::new(),
141186
env: Vec::new(),
142187
use_shell: false,
188+
allow_manual_arg_splitting: cfg!(windows),
143189
}
144190
}

gix-command/tests/command.rs

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,25 @@ mod prepare {
1515
let cmd = std::process::Command::from(gix_command::prepare(""));
1616
assert_eq!(format!("{cmd:?}"), "\"\"");
1717
}
18+
1819
#[test]
1920
fn single_and_multiple_arguments() {
2021
let cmd = std::process::Command::from(gix_command::prepare("ls").arg("first").args(["second", "third"]));
2122
assert_eq!(format!("{cmd:?}"), quoted(&["ls", "first", "second", "third"]));
2223
}
2324

25+
#[test]
26+
fn multiple_arguments_in_one_line_with_auto_split() {
27+
let cmd = std::process::Command::from(
28+
gix_command::prepare("echo first second third").with_shell_allow_argument_splitting(),
29+
);
30+
assert_eq!(
31+
format!("{cmd:?}"),
32+
quoted(&["echo", "first", "second", "third"]),
33+
"we split by hand which works unless one tries to rely on shell-builtins (which we can't detect)"
34+
);
35+
}
36+
2437
#[test]
2538
fn single_and_multiple_arguments_as_part_of_command() {
2639
let cmd = std::process::Command::from(gix_command::prepare("ls first second third"));
@@ -36,7 +49,11 @@ mod prepare {
3649
let cmd = std::process::Command::from(gix_command::prepare("ls first second third").with_shell());
3750
assert_eq!(
3851
format!("{cmd:?}"),
39-
quoted(&[SH, "-c", "ls first second third", "--"]),
52+
if cfg!(windows) {
53+
quoted(&["ls", "first", "second", "third"])
54+
} else {
55+
quoted(&[SH, "-c", "ls first second third", "--"])
56+
},
4057
"with shell, this works as it performs word splitting"
4158
);
4259
}
@@ -46,17 +63,43 @@ mod prepare {
4663
let cmd = std::process::Command::from(gix_command::prepare("ls --foo \"a b\"").arg("additional").with_shell());
4764
assert_eq!(
4865
format!("{cmd:?}"),
49-
format!("\"{SH}\" \"-c\" \"ls --foo \\\"a b\\\" \\\"$@\\\"\" \"--\" \"additional\""),
66+
if cfg!(windows) {
67+
quoted(&["ls", "--foo", "a b", "additional"])
68+
} else {
69+
format!(r#""{SH}" "-c" "ls --foo \"a b\" \"$@\"" "--" "additional""#)
70+
},
5071
"with shell, this works as it performs word splitting"
5172
);
5273
}
5374

75+
#[test]
76+
fn single_and_complex_arguments_with_auto_split() {
77+
let cmd =
78+
std::process::Command::from(gix_command::prepare("ls --foo=\"a b\"").with_shell_allow_argument_splitting());
79+
assert_eq!(
80+
format!("{cmd:?}"),
81+
format!(r#""ls" "--foo=a b""#),
82+
"splitting can also handle quotes"
83+
);
84+
}
85+
86+
#[test]
87+
fn single_and_complex_arguments_will_not_auto_split_on_special_characters() {
88+
let cmd =
89+
std::process::Command::from(gix_command::prepare("ls --foo=~/path").with_shell_allow_argument_splitting());
90+
assert_eq!(
91+
format!("{cmd:?}"),
92+
format!(r#""{SH}" "-c" "ls --foo=~/path" "--""#),
93+
"splitting can also handle quotes"
94+
);
95+
}
96+
5497
#[test]
5598
fn tilde_path_and_multiple_arguments_as_part_of_command_with_shell() {
5699
let cmd = std::process::Command::from(gix_command::prepare("~/bin/exe --foo \"a b\"").with_shell());
57100
assert_eq!(
58101
format!("{cmd:?}"),
59-
format!("\"{SH}\" \"-c\" \"~/bin/exe --foo \\\"a b\\\"\" \"--\""),
102+
format!(r#""{SH}" "-c" "~/bin/exe --foo \"a b\"" "--""#),
60103
"this always needs a shell as we need tilde expansion"
61104
);
62105
}

0 commit comments

Comments
 (0)