From ae1ca03c3188ecce93ff0395fb75217e613ee914 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Wed, 5 Dec 2018 12:31:49 -0500 Subject: [PATCH] allow non-utf8 arguments in proxies --- src/rustup-cli/proxy_mode.rs | 14 +++-- src/rustup-mock/src/clitools.rs | 13 ++++- src/rustup-mock/src/mock_bin_src.rs | 10 +++- tests/cli-misc.rs | 12 +++-- tests/cli-rustup.rs | 82 ++++++++++++++++++++++++++++- 5 files changed, 114 insertions(+), 17 deletions(-) diff --git a/src/rustup-cli/proxy_mode.rs b/src/rustup-cli/proxy_mode.rs index b2c6349fab..da07827a38 100644 --- a/src/rustup-cli/proxy_mode.rs +++ b/src/rustup-cli/proxy_mode.rs @@ -15,7 +15,7 @@ pub fn main() -> Result<()> { let ExitCode(c) = { let _setup = job::setup(); - let mut args = env::args(); + let mut args = env::args_os(); let arg0 = args.next().map(PathBuf::from); let arg0 = arg0 @@ -26,13 +26,11 @@ pub fn main() -> Result<()> { // Check for a toolchain specifier. let arg1 = args.next(); - let toolchain = arg1.as_ref().and_then(|arg1| { - if arg1.starts_with('+') { - Some(&arg1[1..]) - } else { - None - } - }); + let toolchain_arg = arg1 + .as_ref() + .map(|arg| arg.to_string_lossy()) + .filter(|arg| arg.starts_with('+')); + let toolchain = toolchain_arg.as_ref().map(|a| &a[1..]); // Build command args now while we know whether or not to skip arg 1. let cmd_args: Vec<_> = if toolchain.is_none() { diff --git a/src/rustup-mock/src/clitools.rs b/src/rustup-mock/src/clitools.rs index 90d1a309e4..73e7cf4b18 100644 --- a/src/rustup-mock/src/clitools.rs +++ b/src/rustup-mock/src/clitools.rs @@ -10,6 +10,7 @@ use std::cell::RefCell; use std::collections::HashMap; use std::env; use std::env::consts::EXE_SUFFIX; +use std::ffi::OsStr; use std::fs::{self, File}; use std::io::{self, Read, Write}; use std::mem; @@ -296,7 +297,11 @@ pub struct SanitizedOutput { pub stderr: String, } -pub fn cmd(config: &Config, name: &str, args: &[&str]) -> Command { +pub fn cmd(config: &Config, name: &str, args: I) -> Command +where + I: IntoIterator, + A: AsRef, +{ let exe_path = config.exedir.join(format!("{}{}", name, EXE_SUFFIX)); let mut cmd = Command::new(exe_path); cmd.args(args); @@ -340,7 +345,11 @@ pub fn env(config: &Config, cmd: &mut Command) { cmd.env("RUSTUP_INIT_SKIP_PATH_CHECK", "yes"); } -pub fn run(config: &Config, name: &str, args: &[&str], env: &[(&str, &str)]) -> SanitizedOutput { +pub fn run(config: &Config, name: &str, args: I, env: &[(&str, &str)]) -> SanitizedOutput +where + I: IntoIterator, + A: AsRef, +{ let mut cmd = cmd(config, name, args); for env in env { cmd.env(env.0, env.1); diff --git a/src/rustup-mock/src/mock_bin_src.rs b/src/rustup-mock/src/mock_bin_src.rs index 993ad4b308..441fe9e88a 100644 --- a/src/rustup-mock/src/mock_bin_src.rs +++ b/src/rustup-mock/src/mock_bin_src.rs @@ -6,8 +6,8 @@ use std::path::{PathBuf, Path}; use std::process::Command; fn main() { - let mut args = env::args().skip(1); - match args.next().as_ref().map(|s| &**s) { + let mut args = env::args_os().skip(1); + match args.next().as_ref().and_then(|s| s.to_str()) { Some("--version") => { let me = env::current_exe().unwrap(); let mut version_file = PathBuf::from(format!("{}.version", me.display())); @@ -62,6 +62,12 @@ fn main() { let rustc = &format!("rustc{}", EXE_SUFFIX); Command::new(rustc).arg("--version").status().unwrap(); } + Some("--echo-args") => { + let mut out = io::stderr(); + for arg in args { + writeln!(out, "{}", arg.to_string_lossy()).unwrap(); + } + } _ => panic!("bad mock proxy commandline"), } } diff --git a/tests/cli-misc.rs b/tests/cli-misc.rs index cd517605bc..d51ee4ad8c 100644 --- a/tests/cli-misc.rs +++ b/tests/cli-misc.rs @@ -41,7 +41,8 @@ fn smoke_test() { #[test] fn no_colors_in_piped_error_output() { setup(&|config| { - let out = run(config, "rustc", &[], &[]); + let args: Vec<&str> = vec![]; + let out = run(config, "rustc", &args, &[]); assert!(!out.ok); assert!(!out.stderr.contains("\u{1b}")); }); @@ -50,7 +51,8 @@ fn no_colors_in_piped_error_output() { #[test] fn rustc_with_bad_rustup_toolchain_env_var() { setup(&|config| { - let out = run(config, "rustc", &[], &[("RUSTUP_TOOLCHAIN", "bogus")]); + let args: Vec<&str> = vec![]; + let out = run(config, "rustc", &args, &[("RUSTUP_TOOLCHAIN", "bogus")]); assert!(!out.ok); assert!(out.stderr.contains("toolchain 'bogus' is not installed")); }); @@ -677,10 +679,11 @@ fn install_stops_if_rustc_exists() { let temp_dir_path = temp_dir.path().to_str().unwrap(); setup(&|config| { + let args: Vec<&str> = vec![]; let out = run( config, "rustup-init", - &[], + &args, &[ ("RUSTUP_INIT_SKIP_PATH_CHECK", "no"), ("PATH", &temp_dir_path), @@ -705,10 +708,11 @@ fn install_stops_if_cargo_exists() { let temp_dir_path = temp_dir.path().to_str().unwrap(); setup(&|config| { + let args: Vec<&str> = vec![]; let out = run( config, "rustup-init", - &[], + &args, &[ ("RUSTUP_INIT_SKIP_PATH_CHECK", "no"), ("PATH", &temp_dir_path), diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 9eda45bae8..266baae328 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -6,7 +6,7 @@ extern crate rustup_utils; extern crate tempdir; use rustup_mock::clitools::{ - self, expect_err, expect_ok, expect_ok_ex, expect_stderr_ok, expect_stdout_ok, + self, expect_err, expect_ok, expect_ok_ex, expect_stderr_ok, expect_stdout_ok, run, set_current_dist_date, this_host_triple, Config, Scenario, }; use rustup_utils::raw; @@ -1557,3 +1557,83 @@ fn docs_with_path() { assert!(String::from_utf8(out.stdout).unwrap().contains("nightly")); }); } + +#[cfg(unix)] +#[test] +fn non_utf8_arg() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + setup(&|config| { + expect_ok(config, &["rustup", "default", "nightly"]); + let out = run( + config, + "rustc", + &[ + OsStr::new("--echo-args"), + OsStr::new("echoed non-utf8 arg:"), + OsStr::from_bytes(b"\xc3\x28"), + ], + &[("RUST_BACKTRACE", "1")], + ); + assert!(out.stderr.contains("echoed non-utf8 arg")); + }); +} + +#[cfg(windows)] +#[test] +fn non_utf8_arg() { + use std::ffi::OsString; + use std::os::windows::ffi::OsStringExt; + + setup(&|config| { + expect_ok(config, &["rustup", "default", "nightly"]); + let out = run( + config, + "rustc", + &[ + OsString::from("--echo-args".to_string()), + OsString::from("echoed non-utf8 arg:".to_string()), + OsString::from_wide(&[0xd801, 0xd801]), + ], + &[("RUST_BACKTRACE", "1")], + ); + assert!(out.stderr.contains("echoed non-utf8 arg")); + }); +} + +#[cfg(unix)] +#[test] +fn non_utf8_toolchain() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + setup(&|config| { + expect_ok(config, &["rustup", "default", "nightly"]); + let out = run( + config, + "rustc", + &[OsStr::from_bytes(b"+\xc3\x28")], + &[("RUST_BACKTRACE", "1")], + ); + assert!(out.stderr.contains("toolchain '�(' is not installed")); + }); +} + +#[cfg(windows)] +#[test] +fn non_utf8_toolchain() { + use std::ffi::OsString; + use std::os::windows::ffi::OsStringExt; + + setup(&|config| { + expect_ok(config, &["rustup", "default", "nightly"]); + let out = run( + config, + "rustc", + &[OsString::from_wide(&[u16::from('+' as u8), 0xd801, 0xd801])], + &[("RUST_BACKTRACE", "1")], + ); + assert!(out.stderr.contains("toolchain '��' is not installed")); + }); +}