Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow non-utf8 arguments to proxies #1599

Merged
merged 1 commit into from
Jan 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions src/rustup-cli/proxy_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand Down
13 changes: 11 additions & 2 deletions src/rustup-mock/src/clitools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions src/rustup-mock/src/mock_bin_src.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -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"),
}
}
Expand Down
12 changes: 8 additions & 4 deletions tests/cli-misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"));
});
Expand All @@ -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"));
});
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand Down
82 changes: 81 additions & 1 deletion tests/cli-rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"));
});
}