Bug 1464995 - Ensure found Firefox is an executable binary. r?whimboo,jgraham draft
authorAndreas Tolfsen <ato@sny.no>
Tue, 29 May 2018 17:10:26 +0100
changeset 804046 c0bc04c5454e09765ca4b588575a4b4c0e3ef391
parent 804045 9bf8df7e8459022470151b7f33d437756da0e48e
push id112293
push userbmo:ato@sny.no
push dateTue, 05 Jun 2018 13:34:59 +0000
reviewerswhimboo, jgraham
bugs1464995
milestone62.0a1
Bug 1464995 - Ensure found Firefox is an executable binary. r?whimboo,jgraham mozrunner fails to locate the correct binary if Firefox is found under a "firefox" or "firefox-bin" (depending on the system) because it thinks the parent directory is the executable. On Unix systems, mozrunner also falsely reports non-executable files as valid binaries. This patch introduces a new mozrunner::path module that provides two functions: one for searching the system path for a binary by a given name, and another for checking whether a path is an executable binary file. MozReview-Commit-ID: 6N06CXZZWqd
testing/mozbase/rust/mozrunner/src/lib.rs
testing/mozbase/rust/mozrunner/src/path.rs
testing/mozbase/rust/mozrunner/src/runner.rs
--- a/testing/mozbase/rust/mozrunner/src/lib.rs
+++ b/testing/mozbase/rust/mozrunner/src/lib.rs
@@ -1,8 +1,9 @@
 #[macro_use] extern crate log;
 extern crate mozprofile;
 #[cfg(target_os = "windows")]
 extern crate winreg;
 
+pub mod path;
 pub mod runner;
 
 pub use runner::platform::firefox_default_path;
new file mode 100644
--- /dev/null
+++ b/testing/mozbase/rust/mozrunner/src/path.rs
@@ -0,0 +1,44 @@
+//! Provides utilities for searching the system path.
+
+use std::env;
+use std::path::PathBuf;
+
+#[cfg(unix)]
+fn is_executable(path: &PathBuf) -> bool {
+    use std::fs;
+    use std::os::unix::fs::PermissionsExt;
+
+    // Permissions are a set of four 4-bit bitflags, represented by a single octal
+    // digit.  The lowest bit of each of the last three values represents the
+    // executable permission for all, group and user, repsectively.  We assume the
+    // file is executable if any of these are set.
+    match fs::metadata(path).ok() {
+        Some(meta) => meta.permissions().mode() & 0o111 != 0,
+        None => false,
+    }
+}
+
+#[cfg(not(unix))]
+fn is_executable(_: &PathBuf) -> bool {
+    true
+}
+
+/// Determines if the path is an executable binary.  That is, if it exists, is
+/// a file, and is executable where applicable.
+pub fn is_binary(path: &PathBuf) -> bool {
+    path.exists() && path.is_file() && is_executable(&path)
+}
+
+/// Searches the system path (`PATH`) for an executable binary and returns the
+/// first match, or `None` if not found.
+pub fn find_binary(binary_name: &str) -> Option<PathBuf> {
+    env::var_os("PATH").and_then(|path_env| {
+        for mut path in env::split_paths(&path_env) {
+            path.push(binary_name);
+            if is_binary(&path) {
+                return Some(path);
+            }
+        }
+        None
+    })
+}
--- a/testing/mozbase/rust/mozrunner/src/runner.rs
+++ b/testing/mozbase/rust/mozrunner/src/runner.rs
@@ -1,13 +1,12 @@
 use mozprofile::prefreader::PrefReaderError;
 use mozprofile::profile::Profile;
 use std::collections::HashMap;
 use std::convert::From;
-use std::env;
 use std::error::Error;
 use std::ffi::{OsStr, OsString};
 use std::fmt;
 use std::io;
 use std::io::ErrorKind;
 use std::path::{Path, PathBuf};
 use std::process;
 use std::process::{Child, Command, Stdio};
@@ -334,46 +333,34 @@ where
     if let Some(name) = parse_arg_name(arg) {
         name.eq_ignore_ascii_case("profile") || name.eq_ignore_ascii_case("p")
             || name.eq_ignore_ascii_case("profilemanager")
     } else {
         false
     }
 }
 
-fn find_binary(name: &str) -> Option<PathBuf> {
-    env::var("PATH").ok().and_then(|path_env| {
-        for mut path in env::split_paths(&*path_env) {
-            path.push(name);
-            if path.exists() {
-                return Some(path);
-            }
-        }
-        None
-    })
-}
-
 #[cfg(target_os = "linux")]
 pub mod platform {
-    use super::find_binary;
+    use path::find_binary;
     use std::path::PathBuf;
 
     /// Searches the system path for `firefox`.
     pub fn firefox_default_path() -> Option<PathBuf> {
         find_binary("firefox")
     }
 
     pub fn arg_prefix_char(c: char) -> bool {
         c == '-'
     }
 }
 
 #[cfg(target_os = "macos")]
 pub mod platform {
-    use super::find_binary;
+    use path::{find_binary, is_binary};
     use std::env;
     use std::path::PathBuf;
 
     /// Searches the system path for `firefox-bin`, then looks for
     /// `Applications/Firefox.app/Contents/MacOS/firefox-bin` under both `/`
     /// (system root) and the user home directory.
     pub fn firefox_default_path() -> Option<PathBuf> {
         if let Some(path) = find_binary("firefox-bin") {
@@ -389,44 +376,43 @@ pub mod platform {
             (true, "Applications/Firefox.app/Contents/MacOS/firefox-bin"),
         ].iter()
         {
             let path = match (home, prefix_home) {
                 (Some(ref home_dir), true) => home_dir.join(trial_path),
                 (None, true) => continue,
                 (_, false) => trial_path.to_path_buf(),
             };
-            if path.exists() {
+            if is_binary(path) {
                 return Some(path);
             }
         }
 
         None
     }
 
     pub fn arg_prefix_char(c: char) -> bool {
         c == '-'
     }
 }
 
 #[cfg(target_os = "windows")]
 pub mod platform {
-    use super::find_binary;
+    use path::{find_binary, is_binary};
     use std::io::Error;
     use std::path::PathBuf;
     use winreg::RegKey;
     use winreg::enums::*;
 
     /// Searches the Windows registry, then the system path for `firefox.exe`.
     ///
     /// It _does not_ currently check the `HKEY_CURRENT_USER` tree.
     pub fn firefox_default_path() -> Option<PathBuf> {
-        let opt_path = firefox_registry_path().unwrap_or(None);
-        if let Some(path) = opt_path {
-            if path.exists() {
+        if let Ok(Some(path)) = firefox_registry_path() {
+            if is_binary(&path) {
                 return Some(path);
             }
         };
         find_binary("firefox.exe")
     }
 
     fn firefox_registry_path() -> Result<Option<PathBuf>, Error> {
         let hklm = RegKey::predef(HKEY_LOCAL_MACHINE);
@@ -442,19 +428,22 @@ pub mod platform {
                 let key = key_res?;
                 let section_data = mozilla.open_subkey_with_flags(&key, KEY_READ)?;
                 let version: Result<String, _> = section_data.get_value("GeckoVer");
                 if let Ok(ver) = version {
                     if ver == current_version {
                         let mut bin_key = key.to_owned();
                         bin_key.push_str("\\bin");
                         if let Ok(bin_subtree) = mozilla.open_subkey_with_flags(bin_key, KEY_READ) {
-                            let path: Result<String, _> = bin_subtree.get_value("PathToExe");
-                            if let Ok(path) = path {
-                                return Ok(Some(PathBuf::from(path)));
+                            let path_to_exe: Result<String, _> = bin_subtree.get_value("PathToExe");
+                            if let Ok(path_to_exe) = path_to_exe {
+                                let path = PathBuf::from(path_to_exe);
+                                if is_binary(&path) {
+                                    return Ok(Some(path));
+                                }
                             }
                         }
                     }
                 }
             }
         }
         Ok(None)
     }