allow non-utf8 arguments in proxies

rust-lang · Jan 11, 2019 · ae1ca03 · ae1ca03
1 parent b47800c
commit ae1ca03
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 17 deletions.
diff --git 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
@@ -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<I, A>(config: &Config, name: &str, args: I) -> Command
+where
+    I: IntoIterator<Item = A>,
+    A: AsRef<OsStr>,
+{
     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<I, A>(config: &Config, name: &str, args: I, env: &[(&str, &str)]) -> SanitizedOutput
+where
+    I: IntoIterator<Item = A>,
+    A: AsRef<OsStr>,
+{
     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
@@ -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
@@ -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
@@ -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"));
+    });
+}