Skip to content

Commit 87fdf76

Browse files
committed
fix(cli): Forward non-UTF8 arguments to external subcommands
I noticed this because clap v4 changed the default for external subcommands from `String` to `OsString` with the assumption that this would push people to "do the right thing" more often.
1 parent e320c3e commit 87fdf76

File tree

3 files changed

+22
-9
lines changed

3 files changed

+22
-9
lines changed

src/bin/cargo/cli.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use cargo::{self, drop_print, drop_println, CliResult, Config};
55
use clap::{AppSettings, Arg, ArgMatches};
66
use itertools::Itertools;
77
use std::collections::HashMap;
8+
use std::ffi::OsStr;
9+
use std::ffi::OsString;
810
use std::fmt::Write;
911

1012
use super::commands;
@@ -241,20 +243,20 @@ fn expand_aliases(
241243
}
242244
(Some(_), None) => {
243245
// Command is built-in and is not conflicting with alias, but contains ignored values.
244-
if let Some(mut values) = args.get_many::<String>("") {
246+
if let Some(values) = args.get_many::<OsString>("") {
245247
return Err(anyhow::format_err!(
246248
"\
247249
trailing arguments after built-in command `{}` are unsupported: `{}`
248250
249251
To pass the arguments to the subcommand, remove `--`",
250252
cmd,
251-
values.join(" "),
253+
values.map(|s| s.to_string_lossy()).join(" "),
252254
)
253255
.into());
254256
}
255257
}
256258
(None, None) => {}
257-
(_, Some(mut alias)) => {
259+
(_, Some(alias)) => {
258260
// Check if this alias is shadowing an external subcommand
259261
// (binary of the form `cargo-<subcommand>`)
260262
// Currently this is only a warning, but after a transition period this will become
@@ -270,7 +272,11 @@ For more information, see issue #10049 <https://github.com/rust-lang/cargo/issue
270272
))?;
271273
}
272274

273-
alias.extend(args.get_many::<String>("").unwrap_or_default().cloned());
275+
let mut alias = alias
276+
.into_iter()
277+
.map(|s| OsString::from(s))
278+
.collect::<Vec<_>>();
279+
alias.extend(args.get_many::<OsString>("").unwrap_or_default().cloned());
274280
// new_args strips out everything before the subcommand, so
275281
// capture those global options now.
276282
// Note that an alias to an external command will not receive
@@ -346,12 +352,12 @@ fn execute_subcommand(config: &mut Config, cmd: &str, subcommand_args: &ArgMatch
346352
return exec(config, subcommand_args);
347353
}
348354

349-
let mut ext_args: Vec<&str> = vec![cmd];
355+
let mut ext_args: Vec<&OsStr> = vec![OsStr::new(cmd)];
350356
ext_args.extend(
351357
subcommand_args
352-
.get_many::<String>("")
358+
.get_many::<OsString>("")
353359
.unwrap_or_default()
354-
.map(String::as_str),
360+
.map(OsString::as_os_str),
355361
);
356362
super::execute_external_subcommand(config, cmd, &ext_args)
357363
}
@@ -400,6 +406,7 @@ pub fn cli() -> App {
400406
};
401407
App::new("cargo")
402408
.allow_external_subcommands(true)
409+
.allow_invalid_utf8_for_external_subcommands(true)
403410
.setting(AppSettings::DeriveDisplayOrder)
404411
// Doesn't mix well with our list of common cargo commands. See clap-rs/clap#3108 for
405412
// opening clap up to allow us to style our help template

src/bin/cargo/commands/help.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use cargo::util::errors::CargoResult;
44
use cargo::{drop_println, Config};
55
use cargo_util::paths::resolve_executable;
66
use flate2::read::GzDecoder;
7+
use std::ffi::OsStr;
78
use std::ffi::OsString;
89
use std::io::Read;
910
use std::io::Write;
@@ -21,7 +22,11 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
2122
let subcommand = args.get_one::<String>("SUBCOMMAND");
2223
if let Some(subcommand) = subcommand {
2324
if !try_help(config, subcommand)? {
24-
crate::execute_external_subcommand(config, subcommand, &[subcommand, "--help"])?;
25+
crate::execute_external_subcommand(
26+
config,
27+
subcommand,
28+
&[OsStr::new(subcommand), OsStr::new("--help")],
29+
)?;
2530
}
2631
} else {
2732
let mut cmd = crate::cli::cli();

src/bin/cargo/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use cargo::util::{self, closest_msg, command_prelude, CargoResult, CliResult, Co
77
use cargo_util::{ProcessBuilder, ProcessError};
88
use std::collections::BTreeMap;
99
use std::env;
10+
use std::ffi::OsStr;
1011
use std::fs;
1112
use std::path::{Path, PathBuf};
1213

@@ -152,7 +153,7 @@ fn find_external_subcommand(config: &Config, cmd: &str) -> Option<PathBuf> {
152153
.find(|file| is_executable(file))
153154
}
154155

155-
fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> CliResult {
156+
fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&OsStr]) -> CliResult {
156157
let path = find_external_subcommand(config, cmd);
157158
let command = match path {
158159
Some(command) => command,

0 commit comments

Comments
 (0)