Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Description • 7 years ago

As isolated today from https://github.com/mozilla/geckodriver/issues/1058 there is a problem with the Marionette connection because the port marionette creates the server socket on is not known to geckodriver. Instead geckodriver tries to connect to a port it automatically got when checking for a free port. The result is a connection timeout after those 600 tries.

To allow geckodriver to connect to Marionette for the given instance of Firefox, several ways would be possible. After a team discussion today, we agreed on the following steps:

1. geckodriver continuous to find a free port by querying via port 0

2. If a custom profile has been specified via `args` inside the `moz:firefoxOptions` capability, the path of the profile has to be extracted. 

2.1. Inside the profile open the file `prefs.js` and replace the value of the preference `marionette.port` with the above retrieved port number. Store the changes afterward.

3. Continue to start Firefox with the profile

With step 2 Marionette will know which port to use, when starting the server socket. Once that is done, also geckodriver can connect to the known port.

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 1 • 7 years ago

For a temporary workaround see https://github.com/mozilla/geckodriver/issues/1058#issuecomment-347952270.

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 2 • 7 years ago

Given that there is a workaround for the problem we can lower the priority to P2. But it would be still great to have this fixed for the next geckodriver release.

	James Graham [:jgraham] Assignee
	Comment 3 • 7 years ago

I feel like the way this was supposed to work was by adding the preference to the profile passed in. Do we not do that?

	James Graham [:jgraham] Assignee
	Comment 4 • 7 years ago

Oh, right, maybe not when it's passed in as an argument rather than as a zip file.

	Andreas Tolfsen ❲:ato❳
	Comment 5 • 6 years ago

Yes, I believe we skip setting marionette.port when the profile
path is given.  We will still want to set the “override” prefs,
marionette.port and marionette.log.level.

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 6 • 6 years ago

Andreas, will you fix that or should I take this bug? I would like to get it shipped with the next release soonish.

	Andreas Tolfsen ❲:ato❳
	Comment 7 • 6 years ago

This is quite a hard bug to fix because we pass off ownership of the
profile to rust_mozrunner.  We somehow need to pass the ownership back
to geckodriver so it can mutate it, then back to rust_mozrunner again.
I would much prefer changing rust_mozrunner’s API altogether,
but that requires upstream changes because it does not live in tree.

In any case I’m happy for you to try to figure this out.

	Georg Horn
	Comment 8 • 6 years ago

I have made a patch that fixes the problem like described in the first post.

This was my first time of programming in rust, so i dont know if my code is very elegant but at least it works for me...

diff -c -r geckodriver-0.21.0/src/marionette.rs geckodriver-0.21.0-patched/src/marionette.rs
*** geckodriver-0.21.0/src/marionette.rs        Fri Jun 15 22:46:25 2018
--- geckodriver-0.21.0-patched/src/marionette.rs        Fri Sep 14 18:28:47 2018
***************
*** 53,58 ****
--- 53,61 ----
  use capabilities::{FirefoxCapabilities, FirefoxOptions};
  use logging;
  use prefs;
+ use regex::Regex;
+ use regex::NoExpand;
+ use std::fs;

  // localhost may be routed to the IPv6 stack on certain systems,
  // and nsIServerSocket in Marionette only supports IPv4
***************
*** 471,476 ****
--- 474,489 ----
              runner.arg("-jsdebugger");
          }
          if let Some(args) = options.args.as_ref() {
+           let mut found = false;
+           for i in 0..args.len() {
+               if found {
+                   try!(self.update_port_in_profile(port, &args[i]));
+                   break;
+               }
+               if args[i] == "-profile" {
+                   found = true;
+               }
+           }
              runner.args(args);
          }

***************
*** 484,489 ****
--- 497,516 ----

          Ok(())
      }
+
+     fn update_port_in_profile(&mut self, port: u16, profile_dir: &String) -> WebDriverResult<()>
+     {
+       let mut filename: String = profile_dir.to_owned();
+       filename.push_str("/prefs.js");
+       let content = fs::read_to_string(&filename).expect("Unable to read prefs.js");
+       let re = Regex::new(r#"marionette.port", [0-9]*"#).unwrap();
+       let tmp = format!(r#"marionette.port", {}"#, port);
+       let result = re.replace(&content, NoExpand(&tmp));
+       let mut file = File::create(&filename).expect("Unable to open prefs.js");
+       file.write_all(result.as_bytes()).expect("Unable to write to prefs.js");
+
+         Ok(())
+     }

      pub fn set_prefs(&self, port: u16, profile: &mut Profile, custom_profile: bool,
                       extra_prefs: Vec<(String, Pref)>)

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 9 • 6 years ago

Hi Avast. Thank you for the proposal in how to get this bug fixed. Generally we only accept real patch files, and as such you would have to create the patch based on the latest state in mozilla-central. Please see the following link for details:

https://firefox-source-docs.mozilla.org/testing/geckodriver/geckodriver/index.html#for-developers

Otherwise I want to refer to comment 7 of this bug, which outlines how a proper solution to fix the underlying issue should be done.

So would you have a bit of time to work with Andreas and get a patch completed?

	Georg Horn
	Comment 10 • 6 years ago

Hello Hendrik, sorry for my stupid name, i have entered the real name in my profile now.

I possibly do not understand how geckodriver and firefox work together, but as far as i understand it is useless to modify the profile that is created by geckodriver and passed to firefox, because firefox is started with a completely other profile.

I am using the perl bindings and the following script:

$driver = Selenium::Remote::Driver->new(
    'extra_capabilities' => {
        'moz:firefoxOptions' => {
            'args' => [ "-profile", "$ENV{'HOME'}/.mozilla/firefox/default" ],
        },
    }
);

creates a folder /tmp/rust_mozprofile.... which contains a user.js which in turn contains a line "user_pref("marionette.port", 33573);" but firefox is started with "/usr/bin/firefox" "-marionette" "-profile" "/home/horn/.mozilla/firefox/default" and the prefs.js in /home/horn/.mozilla/firefox/default contains the line "user_pref("marionette.port", 2828);"

So i personally think the solution described in the first post of this bug is correct here and i tried to implement that.

of course one can discuss if this way of passing the marionette port-number is optimal at all. I personally believe that firefox should determine a free port number and open a socket on that port and then pass the portnumber to geckodriver who then can connect to the port. possibly this can simply be done by making firefox print the portnumber to stdout and geckodriver read that portnumber from a pipe. But this is all speculation from my side...

	Andreas Tolfsen ❲:ato❳
	Comment 11 • 6 years ago

From what you describe, I would not expect the temporary profile
to be created, although maybe we are creating that unconditionally?
That would seem like a separate bug.

It then correctly recognises the /home/horn/.mozilla/firefox/default
profile you pass by moz:firefoxOptions.  The problem with this
approach is that geckodriver does not know how to write the Marionette
port number to this profile.  2828 is the default port number, but
geckodriver assigns a random port number so you can operate on
multiple Firefoxen simultaneously.  It is obviously a bug that we
don’t support custom profiles yet.

You may have greater luck launching geckodriver like this: "geckodriver
--marionette-port 2828" as that forces the port to be the default
port already defined in /home/horn/.mozilla/firefox/default, and
as geckodriver passes the -marionette flag to Firefox, this should
let you connect and use the custom profile.

IHTH

	Georg Horn
	Comment 12 • 6 years ago

Yes, the temporary profile is created also if "-profile" is given in args.

> It then correctly recognises the /home/horn/.mozilla/firefox/default
> profile you pass by moz:firefoxOptions. The problem with this
> approach is that geckodriver does not know how to write the Marionette
> port number to this profile.

That's what my patch does and what was proposed in the description of this bug, it opens the file prefs.js in /home/horn/.mozilla/firefox/default and replaces the port number with the current random port number.

This also works with multiple firefox instances at one time if your test script provides a separate copy of the original profile folder for each instance it starts. I do that like this:

$moz_prof_dir = "$ENV{'HOME'}/.mozilla/firefox/profile.$$";
system("cp -a $ENV{'HOME'}/.mozilla/firefox/default $moz_prof_dir");
$driver = Selenium::Remote::Driver->new(
  extra_capabilities => {
    'moz:firefoxOptions' => {
        args => [ "-profile", $moz_prof_dir ],
    }
  }
);

Works perfectly for me. If geckodriver creates the profile folder, it too creates one for each firefox session and writes the port number to the file user.js. If you provide the profile by yourself, you tell geckodriver where this profile lives and it just has to insert the proper port number into that profile instead of creating it completely.

> You may have greater luck launching geckodriver like this: "geckodriver
> --marionette-port 2828" as that forces the port to be the default
> port already defined in /home/horn/.mozilla/firefox/default

That's the workaround that was proposed above and it works indeed, but it does NOT work with multiple firefoxes simultaneously.

	Pulsebot
	Comment 20 • 2 years ago

Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/2ef8122c2767
Make geckodriver read marionette port from MarionetteActivePort in profile, r=webdriver-reviewers,jdescottes,whimboo