Stop defaulting to $PATH searches when the binary can't be found and causing infinite recursion

src/rustup-cli/main.rs

@@ -50,6 +50,7 @@ use std::env;
 use std::path::PathBuf;
 use errors::*;
 use rustup_dist::dist::TargetTriple;
+use rustup::env_var::RUST_RECURSION_COUNT_MAX;
 
 fn main() {
     if let Err(ref e) = run_multirust() {
@@ -62,7 +63,7 @@ fn run_multirust() -> Result<()> {
     // Guard against infinite recursion
     let recursion_count = env::var("RUST_RECURSION_COUNT").ok()
         .and_then(|s| s.parse().ok()).unwrap_or(0);
-    if recursion_count > 5 {
+    if recursion_count > RUST_RECURSION_COUNT_MAX {
         return Err(ErrorKind::InfiniteRecursion.into());
     }
 

src/rustup-mock/src/clitools.rs

@@ -32,6 +32,8 @@ pub struct Config {
     pub cargodir: PathBuf,
     /// ~
     pub homedir: PathBuf,
+    /// An empty directory. Tests should not write to this.
+    pub emptydir: PathBuf,
 }
 
 // Describes all the features of the mock dist server.
@@ -67,6 +69,7 @@ pub fn setup(s: Scenario, f: &Fn(&Config)) {
     let customdir = TempDir::new("rustup-custom").unwrap();
     let cargodir = TempDir::new("rustup-cargo").unwrap();
     let homedir = TempDir::new("rustup-home").unwrap();
+    let emptydir = TempDir::new("rustup-empty").unwrap();
 
     // The uninstall process on windows involves using the directory above
     // CARGO_HOME, so make sure it's a subdir of our tempdir
@@ -80,6 +83,7 @@ pub fn setup(s: Scenario, f: &Fn(&Config)) {
         customdir: customdir.path().to_owned(),
         cargodir: cargodir,
         homedir: homedir.path().to_owned(),
+        emptydir: emptydir.path().to_owned(),
     };
 
     create_mock_dist_server(&config.distdir, s);
@@ -257,6 +261,14 @@ pub fn cmd(config: &Config, name: &str, args: &[&str]) -> Command {
 }
 
 pub fn env(config: &Config, cmd: &mut Command) {
+    // Ensure PATH is prefixed with the rustup-exe directory
+    let prev_path = env::var_os("PATH");
+    let mut new_path = config.exedir.clone().into_os_string();
+    if let Some(ref p) = prev_path {
+        new_path.push(if cfg!(windows) { ";" } else { ":" });
+        new_path.push(p);
+    }
+    cmd.env("PATH", new_path);
     cmd.env("RUSTUP_HOME", config.rustupdir.to_string_lossy().to_string());
     cmd.env("RUSTUP_DIST_SERVER", format!("file://{}", config.distdir.to_string_lossy()));
     cmd.env("CARGO_HOME", config.cargodir.to_string_lossy().to_string());

src/rustup/env_var.rs

@@ -3,6 +3,8 @@ use std::env;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
+pub const RUST_RECURSION_COUNT_MAX: u32 = 5;
+
 #[allow(unused)]
 pub fn append_path(name: &str, value: Vec<PathBuf>, cmd: &mut Command) {
     let old_value = env::var_os(name);

src/rustup/errors.rs

@@ -22,6 +22,10 @@ error_chain! {
             description("toolchain is not installed")
             display("toolchain '{}' is not installed", t)
         }
+        BinaryNotFound(t: String, bin: String) {
+            description("toolchain does not contain binary")
+            display("toolchain '{}' does not have the binary `{}`", t, bin)
+        }
         NeedMetadataUpgrade {
             description("rustup's metadata is out of date. run `rustup self upgrade-data`")
         }

src/rustup/lib.rs

@@ -25,9 +25,9 @@ mod errors;
 mod notifications;
 mod toolchain;
 mod config;
-mod env_var;
 mod install;
 pub mod settings;
 pub mod telemetry;
 pub mod command;
 pub mod telemetry_analysis;
+pub mod env_var;

src/rustup/toolchain.rs

@@ -301,13 +301,20 @@ impl<'a> Toolchain<'a> {
         };
 
         let bin_path = self.path.join("bin").join(&binary);
-        let mut cmd = Command::new(if utils::is_file(&bin_path) {
+        let path = if utils::is_file(&bin_path) {
             &bin_path
         } else {
-            // If the bin doesn't actually exist in the sysroot, let the OS try
-            // to resolve it globally for us
+            let recursion_count = env::var("RUST_RECURSION_COUNT").ok()
+                .and_then(|s| s.parse().ok()).unwrap_or(0);
+            if recursion_count > env_var::RUST_RECURSION_COUNT_MAX - 1 {
+                return Err(ErrorKind::BinaryNotFound(self.name.clone(),
+                                                     binary.to_string_lossy()
+                                                           .into())
+                            .into())
+            }
             Path::new(&binary)
-        });
+        };
+        let mut cmd = Command::new(&path);
         self.set_env(&mut cmd);
         Ok(cmd)
     }

tests/cli-misc.rs

@@ -296,6 +296,25 @@ fn rustup_run_searches_path() {
     });
 }
 
+#[test]
+fn rustup_failed_path_search() {
+    setup(&|config| {
+        use std::env::consts::EXE_SUFFIX;
+
+        let ref rustup_path = config.exedir.join(&format!("rustup{}", EXE_SUFFIX));
+        let ref tool_path = config.exedir.join(&format!("fake_proxy{}", EXE_SUFFIX));
+        utils::hardlink_file(rustup_path, tool_path).expect("Failed to create fake proxy for test");
+
+        expect_ok(config, &["rustup", "toolchain", "link", "empty", &config.emptydir.to_string_lossy()]);
+        let broken = &["rustup", "run", "empty", "fake_proxy"];
+        expect_err(config, broken, &format!(
+            "toolchain 'empty' does not have the binary `fake_proxy{}`", EXE_SUFFIX
+        ));
+
+        // Hardlink will be automatically cleaned up by test setup code
+    });
+}
+
 #[test]
 fn multirust_env_compat() {
     setup(&|config| {

tests/cli-self-upd.rs

@@ -26,13 +26,15 @@ use std::process::Command;
 use rustup_mock::clitools::{self, Config, Scenario,
                                expect_ok, expect_ok_ex,
                                expect_stdout_ok,
-                               expect_stderr_ok,
                                expect_err, expect_err_ex,
                                this_host_triple};
 use rustup_mock::dist::{calc_hash};
 use rustup_mock::{get_path, restore_path};
 use rustup_utils::{utils, raw};
 
+#[cfg(windows)]
+use rustup_mock::clitools::expect_stderr_ok;
+
 macro_rules! for_host { ($s: expr) => (&format!($s, this_host_triple())) }
 
 const TEST_VERSION: &'static str = "1.1.1";