Skip to content

Commit 1d89ea5

Browse files
cakebakercakebaker
authored
Merge pull request #7781 from dan-hipschman/env-ignore-flags-after-command
env: ignore flags after the command or -- argument
2 parents ed3dad8 + 814e82e commit 1d89ea5

File tree

2 files changed

+120
-22
lines changed

2 files changed

+120
-22
lines changed

‎src/uu/env/src/env.rs‎

Lines changed: 65 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,18 @@ const ABOUT: &str = help_about!("env.md");
7878
const USAGE: &str = help_usage!("env.md");
7979
const AFTER_HELP: &str = help_section!("after help", "env.md");
8080

81+
mod options {
82+
pub const IGNORE_ENVIRONMENT: &str = "ignore-environment";
83+
pub const CHDIR: &str = "chdir";
84+
pub const NULL: &str = "null";
85+
pub const FILE: &str = "file";
86+
pub const UNSET: &str = "unset";
87+
pub const DEBUG: &str = "debug";
88+
pub const SPLIT_STRING: &str = "split-string";
89+
pub const ARGV0: &str = "argv0";
90+
pub const IGNORE_SIGNAL: &str = "ignore-signal";
91+
}
92+
8193
const ERROR_MSG_S_SHEBANG: &str = "use -[v]S to pass options in shebang lines";
8294

8395
struct Options<'a> {
@@ -216,36 +228,36 @@ pub fn uu_app() -> Command {
216228
.infer_long_args(true)
217229
.trailing_var_arg(true)
218230
.arg(
219-
Arg::new("ignore-environment")
231+
Arg::new(options::IGNORE_ENVIRONMENT)
220232
.short('i')
221-
.long("ignore-environment")
233+
.long(options::IGNORE_ENVIRONMENT)
222234
.help("start with an empty environment")
223235
.action(ArgAction::SetTrue),
224236
)
225237
.arg(
226-
Arg::new("chdir")
238+
Arg::new(options::CHDIR)
227239
.short('C') // GNU env compatibility
228-
.long("chdir")
240+
.long(options::CHDIR)
229241
.number_of_values(1)
230242
.value_name("DIR")
231243
.value_parser(ValueParser::os_string())
232244
.value_hint(clap::ValueHint::DirPath)
233245
.help("change working directory to DIR"),
234246
)
235247
.arg(
236-
Arg::new("null")
248+
Arg::new(options::NULL)
237249
.short('0')
238-
.long("null")
250+
.long(options::NULL)
239251
.help(
240252
"end each output line with a 0 byte rather than a newline (only \
241253
valid when printing the environment)",
242254
)
243255
.action(ArgAction::SetTrue),
244256
)
245257
.arg(
246-
Arg::new("file")
258+
Arg::new(options::FILE)
247259
.short('f')
248-
.long("file")
260+
.long(options::FILE)
249261
.value_name("PATH")
250262
.value_hint(clap::ValueHint::FilePath)
251263
.value_parser(ValueParser::os_string())
@@ -256,34 +268,34 @@ pub fn uu_app() -> Command {
256268
),
257269
)
258270
.arg(
259-
Arg::new("unset")
271+
Arg::new(options::UNSET)
260272
.short('u')
261-
.long("unset")
273+
.long(options::UNSET)
262274
.value_name("NAME")
263275
.action(ArgAction::Append)
264276
.value_parser(ValueParser::os_string())
265277
.help("remove variable from the environment"),
266278
)
267279
.arg(
268-
Arg::new("debug")
280+
Arg::new(options::DEBUG)
269281
.short('v')
270-
.long("debug")
282+
.long(options::DEBUG)
271283
.action(ArgAction::Count)
272284
.help("print verbose information for each processing step"),
273285
)
274286
.arg(
275-
Arg::new("split-string") // split string handling is implemented directly, not using CLAP. But this entry here is needed for the help information output.
287+
Arg::new(options::SPLIT_STRING) // split string handling is implemented directly, not using CLAP. But this entry here is needed for the help information output.
276288
.short('S')
277-
.long("split-string")
289+
.long(options::SPLIT_STRING)
278290
.value_name("S")
279291
.action(ArgAction::Set)
280292
.value_parser(ValueParser::os_string())
281293
.help("process and split S into separate arguments; used to pass multiple arguments on shebang lines")
282294
).arg(
283-
Arg::new("argv0")
284-
.overrides_with("argv0")
295+
Arg::new(options::ARGV0)
296+
.overrides_with(options::ARGV0)
285297
.short('a')
286-
.long("argv0")
298+
.long(options::ARGV0)
287299
.value_name("a")
288300
.action(ArgAction::Set)
289301
.value_parser(ValueParser::os_string())
@@ -296,8 +308,8 @@ pub fn uu_app() -> Command {
296308
.value_parser(ValueParser::os_string())
297309
)
298310
.arg(
299-
Arg::new("ignore-signal")
300-
.long("ignore-signal")
311+
Arg::new(options::IGNORE_SIGNAL)
312+
.long(options::IGNORE_SIGNAL)
301313
.value_name("SIG")
302314
.action(ArgAction::Append)
303315
.value_parser(ValueParser::os_string())
@@ -387,7 +399,31 @@ impl EnvAppData {
387399
original_args: &Vec<OsString>,
388400
) -> UResult<Vec<OsString>> {
389401
let mut all_args: Vec<OsString> = Vec::new();
390-
for arg in original_args {
402+
let mut process_flags = true;
403+
let mut expecting_arg = false;
404+
// Leave out split-string since it's a special case below
405+
let flags_with_args = [
406+
options::ARGV0,
407+
options::CHDIR,
408+
options::FILE,
409+
options::IGNORE_SIGNAL,
410+
options::UNSET,
411+
];
412+
let short_flags_with_args = ['a', 'C', 'f', 'u'];
413+
for (n, arg) in original_args.iter().enumerate() {
414+
let arg_str = arg.to_string_lossy();
415+
// Stop processing env flags once we reach the command or -- argument
416+
if 0 < n
417+
&& !expecting_arg
418+
&& (arg == "--" || !(arg_str.starts_with('-') || arg_str.contains('=')))
419+
{
420+
process_flags = false;
421+
}
422+
if !process_flags {
423+
all_args.push(arg.clone());
424+
continue;
425+
}
426+
expecting_arg = false;
391427
match arg {
392428
b if check_and_handle_string_args(b, "--split-string", &mut all_args, None)? => {
393429
self.had_string_argument = true;
@@ -411,8 +447,15 @@ impl EnvAppData {
411447
self.had_string_argument = true;
412448
}
413449
_ => {
414-
let arg_str = arg.to_string_lossy();
415-
450+
if let Some(flag) = arg_str.strip_prefix("--") {
451+
if flags_with_args.contains(&flag) {
452+
expecting_arg = true;
453+
}
454+
} else if let Some(flag) = arg_str.strip_prefix("-") {
455+
for c in flag.chars() {
456+
expecting_arg = short_flags_with_args.contains(&c);
457+
}
458+
}
416459
// Short unset option (-u) is not allowed to contain '='
417460
if arg_str.contains('=')
418461
&& arg_str.starts_with("-u")

‎tests/by-util/test_env.rs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,61 @@ fn test_invalid_arg() {
6666
new_ucmd!().arg("--definitely-invalid").fails_with_code(125);
6767
}
6868

69+
#[test]
70+
#[cfg(not(target_os = "windows"))]
71+
fn test_flags_after_command() {
72+
new_ucmd!()
73+
// This would cause an error if -u=v were processed because it's malformed
74+
.args(&["echo", "-u=v"])
75+
.succeeds()
76+
.no_stderr()
77+
.stdout_is("-u=v\n");
78+
79+
new_ucmd!()
80+
// Ensure the string isn't split
81+
// cSpell:disable
82+
.args(&["printf", "%s-%s", "-Sfoo bar"])
83+
.succeeds()
84+
.no_stderr()
85+
.stdout_is("-Sfoo bar-");
86+
// cSpell:enable
87+
88+
new_ucmd!()
89+
// Ensure -- is recognized
90+
.args(&["-i", "--", "-u=v"])
91+
.succeeds()
92+
.no_stderr()
93+
.stdout_is("-u=v\n");
94+
95+
new_ucmd!()
96+
// Recognize echo as the command after a flag that takes a value
97+
.args(&["-C", "..", "echo", "-u=v"])
98+
.succeeds()
99+
.no_stderr()
100+
.stdout_is("-u=v\n");
101+
102+
new_ucmd!()
103+
// Recognize echo as the command after a flag that takes an inline value
104+
.args(&["-C..", "echo", "-u=v"])
105+
.succeeds()
106+
.no_stderr()
107+
.stdout_is("-u=v\n");
108+
109+
new_ucmd!()
110+
// Recognize echo as the command after a flag that takes a value after another flag
111+
.args(&["-iC", "..", "echo", "-u=v"])
112+
.succeeds()
113+
.no_stderr()
114+
.stdout_is("-u=v\n");
115+
116+
new_ucmd!()
117+
// Similar to the last two combined
118+
.args(&["-iC..", "echo", "-u=v"])
119+
.succeeds()
120+
.no_stderr()
121+
.stdout_is("-u=v\n");
122+
}
123+
69124
#[test]
70125
fn test_env_help() {
71126
new_ucmd!()

0 commit comments

Comments
 (0)