From aa308fff96f0c5a867027322d870607eb40aff63 Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 18 May 2022 13:26:08 +0000 Subject: [PATCH] Bug 1766125 - Allow setting profile creation directory for geckodriver, r=webdriver-reviewers,whimboo This adds a `--profile-root` argument to geckodriver that sets the directory in which we create temporary profiles. This helps workaround an issue with sandboxed Firefox instances (e.g. those running on snap) where the /tmp inside the sandbox and outside the sandbox are not the same. Users can set this to a directory that's visible both in and out of the sandbox so that both geckodriver and Firefox can access the temporary profile. Differential Revision: https://phabricator.services.mozilla.com/D144970 --- testing/geckodriver/CHANGES.md | 11 ++++-- testing/geckodriver/Cargo.toml | 1 + testing/geckodriver/doc/Flags.md | 14 ++++++++ testing/geckodriver/src/browser.rs | 10 +++--- testing/geckodriver/src/capabilities.rs | 12 +++++-- testing/geckodriver/src/main.rs | 23 ++++++++++++ testing/geckodriver/src/marionette.rs | 3 ++ .../mozbase/rust/mozprofile/src/profile.rs | 10 ++++-- .../webdriver/new_session/profile_root.py | 36 +++++++++++++++++++ .../tests/webdriver/support/fixtures.py | 3 +- 10 files changed, 110 insertions(+), 13 deletions(-) create mode 100644 testing/web-platform/mozilla/tests/webdriver/new_session/profile_root.py diff --git a/testing/geckodriver/CHANGES.md b/testing/geckodriver/CHANGES.md index 3d1b710f06a4..e84c394d5f65 100644 --- a/testing/geckodriver/CHANGES.md +++ b/testing/geckodriver/CHANGES.md @@ -15,11 +15,16 @@ All notable changes to this program are documented in this file. to access the system temporary directory. geckodriver uses the temporary directory to store Firefox profiles created during the run. - This issue can be worked around by setting the `TMPDIR` environment - variable to a location that both Firefox and geckodriver have - read/write access to e.g.: + This issue can be worked around by using the `--profile-root` + command line option or setting the `TMPDIR` environment variable to + a location that both Firefox and geckodriver have read/write access + to e.g.: % mkdir $HOME/tmp + % geckodriver --profile-root=~/tmp + + or + % TMPDIR=$HOME/tmp geckodriver Alternatively, geckodriver may be used with a Firefox install that diff --git a/testing/geckodriver/Cargo.toml b/testing/geckodriver/Cargo.toml index 0b286de73896..582706b79cab 100644 --- a/testing/geckodriver/Cargo.toml +++ b/testing/geckodriver/Cargo.toml @@ -26,6 +26,7 @@ serde = "1.0" serde_derive = "1.0" serde_json = "1.0" serde_yaml = "0.8" +tempfile = "3" url = "2.0" uuid = { version = "0.8", features = ["v4"] } webdriver = { path = "../webdriver", version="0.45.0" } diff --git a/testing/geckodriver/doc/Flags.md b/testing/geckodriver/doc/Flags.md index 46137903794d..9bc156d7ac88 100644 --- a/testing/geckodriver/doc/Flags.md +++ b/testing/geckodriver/doc/Flags.md @@ -190,6 +190,19 @@ Port to use for the WebDriver server. Defaults to 4444. A helpful trick is that it is possible to bind to 0 to get the system to atomically assign a free port. + +## --profile-root PROFILE_ROOT + +Path to the directory to use when creating temporary profiles. By +default this is the system temporary directory. Both geckodriver and +Firefox must have read-write access to this path. + +This setting can be useful when Firefox is sandboxed from the host +filesystem such that it doesn't share the same system temporary +directory as geckodriver (e.g. when running Firefox inside a container +or packaged as a snap). + + ## -v[v] Increases the logging verbosity by to debug level when passing @@ -198,6 +211,7 @@ analogous to passing `--log debug` and `--log trace`, respectively. [Marionette]: /testing/marionette/index.rst + ## --websocket-portPORT Port to use to connect to WebDriver BiDi. Defaults to 9222. diff --git a/testing/geckodriver/src/browser.rs b/testing/geckodriver/src/browser.rs index 306bfe1cc43c..60e06b6106da 100644 --- a/testing/geckodriver/src/browser.rs +++ b/testing/geckodriver/src/browser.rs @@ -71,6 +71,7 @@ impl LocalBrowser { options: FirefoxOptions, marionette_port: u16, jsdebugger: bool, + profile_root: Option<&Path>, ) -> WebDriverResult { let binary = options.binary.ok_or_else(|| { WebDriverError::new( @@ -87,7 +88,7 @@ impl LocalBrowser { let mut profile = match options.profile { ProfileType::Named => None, ProfileType::Path(x) => Some(x), - ProfileType::Temporary => Some(Profile::new()?), + ProfileType::Temporary => Some(Profile::new(profile_root)?), }; let (profile_path, prefs_backup) = if let Some(ref mut profile) = profile { @@ -234,6 +235,7 @@ impl RemoteBrowser { options: FirefoxOptions, marionette_port: u16, websocket_port: Option, + profile_root: Option<&Path>, ) -> WebDriverResult { let android_options = options.android.unwrap(); @@ -248,7 +250,7 @@ impl RemoteBrowser { )); } ProfileType::Path(x) => (x, true), - ProfileType::Temporary => (Profile::new()?, false), + ProfileType::Temporary => (Profile::new(profile_root)?, false), }; set_prefs( @@ -398,7 +400,7 @@ mod tests { // several regressions related to remote.log.level. #[test] fn test_remote_log_level() { - let mut profile = Profile::new().unwrap(); + let mut profile = Profile::new(None).unwrap(); set_prefs(2828, &mut profile, false, vec![], false).ok(); let user_prefs = profile.user_prefs().unwrap(); @@ -460,7 +462,7 @@ mod tests { #[test] fn test_pref_backup() { - let mut profile = Profile::new().unwrap(); + let mut profile = Profile::new(None).unwrap(); // Create some prefs in the profile let initial_prefs = profile.user_prefs().unwrap(); diff --git a/testing/geckodriver/src/capabilities.rs b/testing/geckodriver/src/capabilities.rs index 97e1e287264a..3654cc5a7bae 100644 --- a/testing/geckodriver/src/capabilities.rs +++ b/testing/geckodriver/src/capabilities.rs @@ -432,7 +432,10 @@ impl FirefoxOptions { rv.env = FirefoxOptions::load_env(options)?; rv.log = FirefoxOptions::load_log(options)?; rv.prefs = FirefoxOptions::load_prefs(options)?; - if let Some(profile) = FirefoxOptions::load_profile(options)? { + if let Some(profile) = FirefoxOptions::load_profile( + settings.profile_root.as_ref().map(|x| x.as_path()), + options, + )? { rv.profile = ProfileType::Path(profile); } } @@ -554,7 +557,10 @@ impl FirefoxOptions { Ok(rv) } - fn load_profile(options: &Capabilities) -> WebDriverResult> { + fn load_profile( + profile_root: Option<&Path>, + options: &Capabilities, + ) -> WebDriverResult> { if let Some(profile_json) = options.get("profile") { let profile_base64 = profile_json.as_str().ok_or_else(|| { WebDriverError::new(ErrorStatus::InvalidArgument, "Profile is not a string") @@ -562,7 +568,7 @@ impl FirefoxOptions { let profile_zip = &*base64::decode(profile_base64)?; // Create an emtpy profile directory - let profile = Profile::new()?; + let profile = Profile::new(profile_root)?; unzip_buffer( profile_zip, profile diff --git a/testing/geckodriver/src/main.rs b/testing/geckodriver/src/main.rs index 115d650574ee..ea2044ee8789 100644 --- a/testing/geckodriver/src/main.rs +++ b/testing/geckodriver/src/main.rs @@ -17,6 +17,7 @@ extern crate serde; extern crate serde_derive; extern crate serde_json; extern crate serde_yaml; +extern crate tempfile; extern crate url; extern crate uuid; extern crate webdriver; @@ -267,6 +268,20 @@ fn parse_args(app: &mut App) -> ProgramResult { let binary = args.value_of("binary").map(PathBuf::from); + let profile_root = args.value_of("profile_root").map(PathBuf::from); + + // Try to create a temporary directory on startup to check that the directory exists and is writable + { + let tmp_dir = if let Some(ref tmp_root) = profile_root { + tempfile::tempdir_in(tmp_root) + } else { + tempfile::tempdir() + }; + if tmp_dir.is_err() { + usage!("Unable to write to temporary directory; consider --profile-root with a writeable directory") + } + } + let marionette_host = args.value_of("marionette_host").unwrap(); let marionette_port = match args.value_of("marionette_port") { Some(s) => match u16::from_str(s) { @@ -305,6 +320,7 @@ fn parse_args(app: &mut App) -> ProgramResult { let settings = MarionetteSettings { binary, + profile_root, connect_existing: args.is_present("connect_existing"), host: marionette_host.into(), port: marionette_port, @@ -473,6 +489,13 @@ fn make_app<'a>() -> App<'a> { .long("version") .help("Prints version and copying information"), ) + .arg( + Arg::new("profile_root") + .long("profile-root") + .takes_value(true) + .value_name("PROFILE_ROOT") + .help("Directory in which to create profiles. Defaults to the system temporary directory."), + ) .arg( Arg::new("android_storage") .long("android-storage") diff --git a/testing/geckodriver/src/marionette.rs b/testing/geckodriver/src/marionette.rs index 325cc9a10220..868078c3abfa 100644 --- a/testing/geckodriver/src/marionette.rs +++ b/testing/geckodriver/src/marionette.rs @@ -81,6 +81,7 @@ struct MarionetteHandshake { #[derive(Default)] pub(crate) struct MarionetteSettings { pub(crate) binary: Option, + pub(crate) profile_root: Option, pub(crate) connect_existing: bool, pub(crate) host: String, pub(crate) port: Option, @@ -188,12 +189,14 @@ impl MarionetteHandler { options, marionette_port, websocket_port, + self.settings.profile_root.as_ref().map(|x| x.as_path()), )?) } else if !self.settings.connect_existing { Browser::Local(LocalBrowser::new( options, marionette_port, self.settings.jsdebugger, + self.settings.profile_root.as_ref().map(|x| x.as_path()), )?) } else { Browser::Existing(marionette_port) diff --git a/testing/mozbase/rust/mozprofile/src/profile.rs b/testing/mozbase/rust/mozprofile/src/profile.rs index b3ad6cf72601..f4037adfa933 100644 --- a/testing/mozbase/rust/mozprofile/src/profile.rs +++ b/testing/mozbase/rust/mozprofile/src/profile.rs @@ -26,8 +26,14 @@ impl PartialEq for Profile { } impl Profile { - pub fn new() -> IoResult { - let dir = Builder::new().prefix("rust_mozprofile").tempdir()?; + pub fn new(temp_root: Option<&Path>) -> IoResult { + let mut dir_builder = Builder::new(); + dir_builder.prefix("rust_mozprofile"); + let dir = if let Some(temp_root) = temp_root { + dir_builder.tempdir_in(temp_root) + } else { + dir_builder.tempdir() + }?; let path = dir.path().to_path_buf(); let temp_dir = Some(dir); Ok(Profile { diff --git a/testing/web-platform/mozilla/tests/webdriver/new_session/profile_root.py b/testing/web-platform/mozilla/tests/webdriver/new_session/profile_root.py new file mode 100644 index 000000000000..582e7124e4b0 --- /dev/null +++ b/testing/web-platform/mozilla/tests/webdriver/new_session/profile_root.py @@ -0,0 +1,36 @@ +import copy +import os + +import pytest + + +def test_profile_root(tmp_path, configuration, geckodriver): + profile_path = os.path.join(tmp_path, "geckodriver-test") + os.makedirs(profile_path) + + config = copy.deepcopy(configuration) + # Ensure we don't set a profile in command line arguments + del config["capabilities"]["moz:firefoxOptions"]["args"] + + extra_args = ["--profile-root", profile_path] + + assert os.listdir(profile_path) == [] + + driver = geckodriver(config=config, extra_args=extra_args) + driver.new_session() + assert len(os.listdir(profile_path)) == 1 + driver.delete_session() + assert os.listdir(profile_path) == [] + + +def test_profile_root_missing(tmp_path, configuration, geckodriver): + profile_path = os.path.join(tmp_path, "missing-path") + + config = copy.deepcopy(configuration) + # Ensure we don't set a profile in command line arguments + del config["capabilities"]["moz:firefoxOptions"]["args"] + + extra_args = ["--profile-root", profile_path] + + with pytest.raises(Exception): + geckodriver(config=config, extra_args=extra_args) diff --git a/testing/web-platform/mozilla/tests/webdriver/support/fixtures.py b/testing/web-platform/mozilla/tests/webdriver/support/fixtures.py index e51b894936f5..5a1d443d9a89 100644 --- a/testing/web-platform/mozilla/tests/webdriver/support/fixtures.py +++ b/testing/web-platform/mozilla/tests/webdriver/support/fixtures.py @@ -83,7 +83,8 @@ def geckodriver(configuration): yield _geckodriver - driver.stop() + if driver is not None: + driver.stop() class Browser: