Skip to content

Commit

Permalink
[rust] Avoids resolving symbolic links and consider the cache might n…
Browse files Browse the repository at this point in the history
…ot be writable (#12877)

* [rust] Canonicalize paths only in Windows

* [rust] Warning if writing metadata is not possible

* [rust] Fix condition for displaying warning due to incompatible version

* [rust] Warning when cache path cannot be created

* [rust] Make metadata optional

* [rust] Improve warning messages

* [rust] Clean function to write metadata

* [rust] Rename path buf function

* [rust] Fix logic related to checking driver version (broken in 5e2972e)
  • Loading branch information
bonigarcia authored Oct 5, 2023
1 parent 87dcb36 commit d13ac33
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 98 deletions.
16 changes: 9 additions & 7 deletions rust/src/chrome.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use std::path::PathBuf;
use crate::config::ARCH::{ARM64, X32};
use crate::config::OS::{LINUX, MACOS, WINDOWS};
use crate::downloads::{parse_json_from_url, read_version_from_link};
use crate::files::{compose_driver_path_in_cache, path_buf_to_string, BrowserPath};
use crate::files::{compose_driver_path_in_cache, path_to_string, BrowserPath};
use crate::logger::Logger;
use crate::metadata::{
create_driver_metadata, get_driver_version_from_metadata, get_metadata, write_metadata,
Expand Down Expand Up @@ -277,7 +277,8 @@ impl SeleniumManager for ChromeManager {
fn request_driver_version(&mut self) -> Result<String, Box<dyn Error>> {
let major_browser_version_binding = self.get_major_browser_version();
let major_browser_version = major_browser_version_binding.as_str();
let mut metadata = get_metadata(self.get_logger(), self.get_cache_path()?);
let cache_path = self.get_cache_path()?;
let mut metadata = get_metadata(self.get_logger(), &cache_path);

match get_driver_version_from_metadata(
&metadata.drivers,
Expand Down Expand Up @@ -326,7 +327,7 @@ impl SeleniumManager for ChromeManager {
&driver_version,
driver_ttl,
));
write_metadata(&metadata, self.get_logger(), self.get_cache_path()?);
write_metadata(&metadata, self.get_logger(), cache_path);
}
Ok(driver_version)
}
Expand Down Expand Up @@ -381,7 +382,7 @@ impl SeleniumManager for ChromeManager {

fn get_driver_path_in_cache(&self) -> Result<PathBuf, Box<dyn Error>> {
Ok(compose_driver_path_in_cache(
self.get_cache_path()?,
self.get_cache_path()?.unwrap_or_default(),
self.driver_name,
self.get_os(),
self.get_platform_label(),
Expand Down Expand Up @@ -412,7 +413,8 @@ impl SeleniumManager for ChromeManager {
fn download_browser(&mut self) -> Result<Option<PathBuf>, Box<dyn Error>> {
let browser_version;
let browser_name = self.browser_name;
let mut metadata = get_metadata(self.get_logger(), self.get_cache_path()?);
let cache_path = self.get_cache_path()?;
let mut metadata = get_metadata(self.get_logger(), &cache_path);
let major_browser_version = self.get_major_browser_version();
let major_browser_version_int = major_browser_version.parse::<i32>().unwrap_or_default();

Expand Down Expand Up @@ -465,7 +467,7 @@ impl SeleniumManager for ChromeManager {
&browser_version,
browser_ttl,
));
write_metadata(&metadata, self.get_logger(), self.get_cache_path()?);
write_metadata(&metadata, self.get_logger(), cache_path);
}
}
}
Expand Down Expand Up @@ -513,7 +515,7 @@ impl SeleniumManager for ChromeManager {
)?;
}
if browser_binary_path.exists() {
self.set_browser_path(path_buf_to_string(browser_binary_path.clone()));
self.set_browser_path(path_to_string(&browser_binary_path));
Ok(Some(browser_binary_path))
} else {
Ok(None)
Expand Down
6 changes: 3 additions & 3 deletions rust/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use crate::config::OS::{LINUX, MACOS, WINDOWS};
use crate::shell::run_shell_command_by_os;
use crate::{
default_cache_folder, format_one_arg, path_buf_to_string, Command, REQUEST_TIMEOUT_SEC,
default_cache_folder, format_one_arg, path_to_string, Command, REQUEST_TIMEOUT_SEC,
UNAME_COMMAND,
};
use crate::{ARCH_AMD64, ARCH_ARM64, ARCH_X86, TTL_SEC, WMIC_COMMAND_OS};
Expand All @@ -30,7 +30,7 @@ use std::fs::read_to_string;
use std::path::Path;
use toml::Table;

thread_local!(static CACHE_PATH: RefCell<String> = RefCell::new(path_buf_to_string(default_cache_folder())));
thread_local!(static CACHE_PATH: RefCell<String> = RefCell::new(path_to_string(&default_cache_folder())));

pub const CONFIG_FILE: &str = "se-config.toml";
pub const ENV_PREFIX: &str = "SE_";
Expand Down Expand Up @@ -263,7 +263,7 @@ fn write_cache_path(cache_path: String) {
}

fn read_cache_path() -> String {
let mut cache_path: String = path_buf_to_string(default_cache_folder());
let mut cache_path: String = path_to_string(&default_cache_folder());
CACHE_PATH.with(|value| {
let path: String = (&*value.borrow().to_string()).into();
if !path.is_empty() {
Expand Down
7 changes: 4 additions & 3 deletions rust/src/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ impl SeleniumManager for EdgeManager {

fn request_driver_version(&mut self) -> Result<String, Box<dyn Error>> {
let mut major_browser_version = self.get_major_browser_version();
let mut metadata = get_metadata(self.get_logger(), self.get_cache_path()?);
let cache_path = self.get_cache_path()?;
let mut metadata = get_metadata(self.get_logger(), &cache_path);

match get_driver_version_from_metadata(
&metadata.drivers,
Expand Down Expand Up @@ -194,7 +195,7 @@ impl SeleniumManager for EdgeManager {
&driver_version,
driver_ttl,
));
write_metadata(&metadata, self.get_logger(), self.get_cache_path()?);
write_metadata(&metadata, self.get_logger(), cache_path);
}

Ok(driver_version)
Expand Down Expand Up @@ -235,7 +236,7 @@ impl SeleniumManager for EdgeManager {

fn get_driver_path_in_cache(&self) -> Result<PathBuf, Box<dyn Error>> {
Ok(compose_driver_path_in_cache(
self.get_cache_path()?,
self.get_cache_path()?.unwrap_or_default(),
self.driver_name,
self.get_os(),
self.get_platform_label(),
Expand Down
21 changes: 10 additions & 11 deletions rust/src/files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ pub fn uncompress_sfx(
let file_reader = Cursor::new(&file_bytes[index_7z..]);
sevenz_rust::decompress(file_reader, zip_parent).unwrap();

let zip_parent_str = path_buf_to_string(zip_parent.to_path_buf());
let target_str = path_buf_to_string(target.to_path_buf());
let zip_parent_str = path_to_string(zip_parent);
let target_str = path_to_string(target);
let core_str = format!(r#"{}\core"#, zip_parent_str);
log.trace(format!(
"Moving extracted files and folders from {} to {}",
Expand All @@ -191,11 +191,7 @@ pub fn uncompress_pkg(
major_browser_version: i32,
) -> Result<(), Box<dyn Error>> {
let tmp_dir = Builder::new().prefix(PKG).tempdir()?;
let out_folder = format!(
"{}/{}",
path_buf_to_string(tmp_dir.path().to_path_buf()),
PKG
);
let out_folder = format!("{}/{}", path_to_string(tmp_dir.path()), PKG);
let mut command = Command::new_single(format_two_args(
PKGUTIL_COMMAND,
compressed_file,
Expand All @@ -205,7 +201,7 @@ pub fn uncompress_pkg(
run_shell_command_by_os(os, command)?;

fs::create_dir_all(target)?;
let target_folder = path_buf_to_string(target.to_path_buf());
let target_folder = path_to_string(target);
command = if major_browser_version == 0 || major_browser_version > 84 {
Command::new_single(format_three_args(
MV_PAYLOAD_COMMAND,
Expand Down Expand Up @@ -246,7 +242,7 @@ pub fn uncompress_dmg(
run_shell_command_by_os(os, command)?;

fs::create_dir_all(target)?;
let target_folder = path_buf_to_string(target.to_path_buf());
let target_folder = path_to_string(target);
command = Command::new_single(format_three_args(
CP_VOLUME_COMMAND,
volume,
Expand Down Expand Up @@ -497,8 +493,11 @@ pub fn parse_version(version_text: String, log: &Logger) -> Result<String, Box<d
Ok(parsed_version)
}

pub fn path_buf_to_string(path_buf: PathBuf) -> String {
path_buf.into_os_string().into_string().unwrap_or_default()
pub fn path_to_string(path: &Path) -> String {
path.to_path_buf()
.into_os_string()
.into_string()
.unwrap_or_default()
}

pub fn read_bytes_from_file(file_path: &str) -> Result<Vec<u8>, Box<dyn Error>> {
Expand Down
16 changes: 9 additions & 7 deletions rust/src/firefox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::metadata::{
};
use crate::{
create_browser_metadata, create_http_client, download_to_tmp_folder, format_three_args,
format_two_args, get_browser_version_from_metadata, path_buf_to_string, uncompress, Logger,
format_two_args, get_browser_version_from_metadata, path_to_string, uncompress, Logger,
SeleniumManager, BETA, CANARY, DASH_VERSION, DEV, NIGHTLY, OFFLINE_REQUEST_ERR_MSG,
REG_CURRENT_VERSION_ARG, STABLE,
};
Expand Down Expand Up @@ -274,7 +274,8 @@ impl SeleniumManager for FirefoxManager {
fn request_driver_version(&mut self) -> Result<String, Box<dyn Error>> {
let major_browser_version_binding = self.get_major_browser_version();
let major_browser_version = major_browser_version_binding.as_str();
let mut metadata = get_metadata(self.get_logger(), self.get_cache_path()?);
let cache_path = self.get_cache_path()?;
let mut metadata = get_metadata(self.get_logger(), &cache_path);

match get_driver_version_from_metadata(
&metadata.drivers,
Expand Down Expand Up @@ -303,7 +304,7 @@ impl SeleniumManager for FirefoxManager {
&driver_version,
driver_ttl,
));
write_metadata(&metadata, self.get_logger(), self.get_cache_path()?);
write_metadata(&metadata, self.get_logger(), cache_path);
}

Ok(driver_version)
Expand Down Expand Up @@ -355,7 +356,7 @@ impl SeleniumManager for FirefoxManager {

fn get_driver_path_in_cache(&self) -> Result<PathBuf, Box<dyn Error>> {
Ok(compose_driver_path_in_cache(
self.get_cache_path()?,
self.get_cache_path()?.unwrap_or_default(),
self.driver_name,
self.get_os(),
self.get_platform_label(),
Expand Down Expand Up @@ -387,7 +388,8 @@ impl SeleniumManager for FirefoxManager {
let browser_version;
let browser_name = self.browser_name;
let original_browser_version = self.get_config().browser_version.clone();
let mut metadata = get_metadata(self.get_logger(), self.get_cache_path()?);
let cache_path = self.get_cache_path()?;
let mut metadata = get_metadata(self.get_logger(), &cache_path);
let major_browser_version = self.get_major_browser_version();
let is_browser_version_nightly = original_browser_version.eq_ignore_ascii_case(NIGHTLY)
|| original_browser_version.eq_ignore_ascii_case(CANARY);
Expand Down Expand Up @@ -426,7 +428,7 @@ impl SeleniumManager for FirefoxManager {
&browser_version,
browser_ttl,
));
write_metadata(&metadata, self.get_logger(), self.get_cache_path()?);
write_metadata(&metadata, self.get_logger(), cache_path);
}
}
}
Expand Down Expand Up @@ -474,7 +476,7 @@ impl SeleniumManager for FirefoxManager {
)?;
}
if browser_binary_path.exists() {
self.set_browser_path(path_buf_to_string(browser_binary_path.clone()));
self.set_browser_path(path_to_string(&browser_binary_path));
Ok(Some(browser_binary_path))
} else {
Ok(None)
Expand Down
6 changes: 4 additions & 2 deletions rust/src/grid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ impl SeleniumManager for GridManager {
fn request_driver_version(&mut self) -> Result<String, Box<dyn Error>> {
let major_browser_version_binding = self.get_major_browser_version();
let major_browser_version = major_browser_version_binding.as_str();
let mut metadata = get_metadata(self.get_logger(), self.get_cache_path()?);
let cache_path = self.get_cache_path()?;
let mut metadata = get_metadata(self.get_logger(), &cache_path);

match get_driver_version_from_metadata(
&metadata.drivers,
Expand Down Expand Up @@ -156,7 +157,7 @@ impl SeleniumManager for GridManager {
&driver_version,
driver_ttl,
));
write_metadata(&metadata, self.get_logger(), self.get_cache_path()?);
write_metadata(&metadata, self.get_logger(), cache_path);
}

Ok(driver_version)
Expand Down Expand Up @@ -193,6 +194,7 @@ impl SeleniumManager for GridManager {
let driver_version = self.get_driver_version();
Ok(self
.get_cache_path()?
.unwrap_or_default()
.join(browser_name)
.join(driver_version)
.join(format!("{driver_name}-{driver_version}.{GRID_EXTENSION}")))
Expand Down
7 changes: 4 additions & 3 deletions rust/src/iexplorer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ impl SeleniumManager for IExplorerManager {
fn request_driver_version(&mut self) -> Result<String, Box<dyn Error>> {
let major_browser_version_binding = self.get_major_browser_version();
let major_browser_version = major_browser_version_binding.as_str();
let mut metadata = get_metadata(self.get_logger(), self.get_cache_path()?);
let cache_path = self.get_cache_path()?;
let mut metadata = get_metadata(self.get_logger(), &cache_path);

match get_driver_version_from_metadata(
&metadata.drivers,
Expand Down Expand Up @@ -164,7 +165,7 @@ impl SeleniumManager for IExplorerManager {
&driver_version,
driver_ttl,
));
write_metadata(&metadata, self.get_logger(), self.get_cache_path()?);
write_metadata(&metadata, self.get_logger(), cache_path);
}

Ok(driver_version)
Expand Down Expand Up @@ -196,7 +197,7 @@ impl SeleniumManager for IExplorerManager {

fn get_driver_path_in_cache(&self) -> Result<PathBuf, Box<dyn Error>> {
Ok(compose_driver_path_in_cache(
self.get_cache_path()?,
self.get_cache_path()?.unwrap_or_default(),
self.driver_name,
"Windows",
self.get_platform_label(),
Expand Down
Loading

0 comments on commit d13ac33

Please sign in to comment.