Skip to content

Commit

Permalink
fix(cli): Forward non-UTF8 arguments to external subcommands
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
epage committed Sep 21, 2022
1 parent e320c3e commit 76c1ed1
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 15 deletions.
15 changes: 9 additions & 6 deletions src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use cargo::{self, drop_print, drop_println, CliResult, Config};
use clap::{AppSettings, Arg, ArgMatches};
use itertools::Itertools;
use std::collections::HashMap;
use std::ffi::OsStr;
use std::ffi::OsString;
use std::fmt::Write;

use super::commands;
Expand Down Expand Up @@ -241,14 +243,14 @@ fn expand_aliases(
}
(Some(_), None) => {
// Command is built-in and is not conflicting with alias, but contains ignored values.
if let Some(mut values) = args.get_many::<String>("") {
if let Some(values) = args.get_many::<OsString>("") {
return Err(anyhow::format_err!(
"\
trailing arguments after built-in command `{}` are unsupported: `{}`
To pass the arguments to the subcommand, remove `--`",
cmd,
values.join(" "),
values.map(|s| s.to_string_lossy()).join(" "),
)
.into());
}
Expand All @@ -270,7 +272,7 @@ For more information, see issue #10049 <https://github.com/rust-lang/cargo/issue
))?;
}

alias.extend(args.get_many::<String>("").unwrap_or_default().cloned());
alias.extend(args.get_many::<OsString>("").unwrap_or_default().cloned());
// new_args strips out everything before the subcommand, so
// capture those global options now.
// Note that an alias to an external command will not receive
Expand Down Expand Up @@ -346,12 +348,12 @@ fn execute_subcommand(config: &mut Config, cmd: &str, subcommand_args: &ArgMatch
return exec(config, subcommand_args);
}

let mut ext_args: Vec<&str> = vec![cmd];
let mut ext_args: Vec<&OsStr> = vec![OsStr::new(cmd)];
ext_args.extend(
subcommand_args
.get_many::<String>("")
.get_many::<OsString>("")
.unwrap_or_default()
.map(String::as_str),
.map(OsString::as_os_str),
);
super::execute_external_subcommand(config, cmd, &ext_args)
}
Expand Down Expand Up @@ -400,6 +402,7 @@ pub fn cli() -> App {
};
App::new("cargo")
.allow_external_subcommands(true)
.allow_invalid_utf8_for_external_subcommands(true)
.setting(AppSettings::DeriveDisplayOrder)
// Doesn't mix well with our list of common cargo commands. See clap-rs/clap#3108 for
// opening clap up to allow us to style our help template
Expand Down
20 changes: 16 additions & 4 deletions src/bin/cargo/commands/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use cargo::util::errors::CargoResult;
use cargo::{drop_println, Config};
use cargo_util::paths::resolve_executable;
use flate2::read::GzDecoder;
use itertools::Itertools;
use std::ffi::OsStr;
use std::ffi::OsString;
use std::io::Read;
use std::io::Write;
Expand All @@ -21,7 +23,11 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let subcommand = args.get_one::<String>("SUBCOMMAND");
if let Some(subcommand) = subcommand {
if !try_help(config, subcommand)? {
crate::execute_external_subcommand(config, subcommand, &[subcommand, "--help"])?;
crate::execute_external_subcommand(
config,
subcommand,
&[OsStr::new(subcommand), OsStr::new("--help")],
)?;
}
} else {
let mut cmd = crate::cli::cli();
Expand All @@ -34,14 +40,20 @@ fn try_help(config: &Config, subcommand: &str) -> CargoResult<bool> {
let subcommand = match check_alias(config, subcommand) {
// If this alias is more than a simple subcommand pass-through, show the alias.
Some(argv) if argv.len() > 1 => {
let alias = argv.join(" ");
let alias = argv
.into_iter()
.map(|s| s.to_string_lossy().into_owned())
.join(" ");
drop_println!(config, "`{}` is aliased to `{}`", subcommand, alias);
return Ok(true);
}
// Otherwise, resolve the alias into its subcommand.
Some(argv) => {
// An alias with an empty argv can be created via `"empty-alias" = ""`.
let first = argv.get(0).map(String::as_str).unwrap_or(subcommand);
let first = argv
.get(0)
.map(|s| s.to_str().expect("aliases are always UTF-8"))
.unwrap_or(subcommand);
first.to_string()
}
None => subcommand.to_string(),
Expand Down Expand Up @@ -77,7 +89,7 @@ fn try_help(config: &Config, subcommand: &str) -> CargoResult<bool> {
/// Checks if the given subcommand is an alias.
///
/// Returns None if it is not an alias.
fn check_alias(config: &Config, subcommand: &str) -> Option<Vec<String>> {
fn check_alias(config: &Config, subcommand: &str) -> Option<Vec<OsString>> {
aliased_command(config, subcommand).ok().flatten()
}

Expand Down
14 changes: 9 additions & 5 deletions src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use cargo::util::{self, closest_msg, command_prelude, CargoResult, CliResult, Co
use cargo_util::{ProcessBuilder, ProcessError};
use std::collections::BTreeMap;
use std::env;
use std::ffi::OsStr;
use std::ffi::OsString;
use std::fs;
use std::path::{Path, PathBuf};

Expand Down Expand Up @@ -52,7 +54,7 @@ fn builtin_aliases_execs(cmd: &str) -> Option<&(&str, &str, &str)> {
BUILTIN_ALIASES.iter().find(|alias| alias.0 == cmd)
}

fn aliased_command(config: &Config, command: &str) -> CargoResult<Option<Vec<String>>> {
fn aliased_command(config: &Config, command: &str) -> CargoResult<Option<Vec<OsString>>> {
let alias_name = format!("alias.{}", command);
let user_alias = match config.get_string(&alias_name) {
Ok(Some(record)) => Some(
Expand All @@ -66,9 +68,11 @@ fn aliased_command(config: &Config, command: &str) -> CargoResult<Option<Vec<Str
Err(_) => config.get::<Option<Vec<String>>>(&alias_name)?,
};

let result = user_alias.or_else(|| {
builtin_aliases_execs(command).map(|command_str| vec![command_str.1.to_string()])
});
let result = user_alias
.or_else(|| {
builtin_aliases_execs(command).map(|command_str| vec![command_str.1.to_string()])
})
.map(|v| v.into_iter().map(|s| OsString::from(s)).collect());
Ok(result)
}

Expand Down Expand Up @@ -152,7 +156,7 @@ fn find_external_subcommand(config: &Config, cmd: &str) -> Option<PathBuf> {
.find(|file| is_executable(file))
}

fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> CliResult {
fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&OsStr]) -> CliResult {
let path = find_external_subcommand(config, cmd);
let command = match path {
Some(command) => command,
Expand Down

0 comments on commit 76c1ed1

Please sign in to comment.