1574446 - mozrunner checks wrong Windows registry location for Firefox binary, depending on geckodriver bit architecture

Status: NEW

(Testing :: Mozbase Rust, defect, P3)

Description

[Andreas Tolfsen ❲:ato❳]

Windows ships with a 32-bit compatibility layer which means you can
run 32-bit programs on a 64-bit system installation of Windows.
Some users use the 32-bit releases of geckodriver on 64-bit systems,
and this causes problems when mozrunner tries to deduce the default
location of the Firefox binary.

Quoting whimboo in
https://github.com/mozilla/geckodriver/issues/1168:

the problem here is that a 32bit executable of geckodriver will
check a different path in the Registry than the 64bit one. As
such a 64bit installation of Firefox cannot be found. It might be
similar vice versa. As such we always might have to check for both
a 32bit and 64bit installation of Firefox.

This was originally filed as
https://github.com/mozilla/geckodriver/issues/1045 on GitHub, then
migrated to https://github.com/jgraham/rust_mozrunner/issues/15,
but we lost track of this issue when bug tracking for the mozbase
Rust port was moved to Bugzilla.

The suggestion from florentbr to check a single registry path
that is shared between 32- and 64-bit systems seems sensible.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: bug 1574446: mozrunner: look for Firefox executable in in HKEY_CURRENT_USER

The current lookup for the Firefox executable on Windows exhibits
two problems: it relies on architecture specific paths (such
as Wow6432Node), and it skips looking at HKEY_CURRENT_USER for
applications that are not installed system-wide.

The first problem manifests when using the 32-bit version of
geckodriver on a 64-bit host system, or when the Firefox executable
has a different architecture than geckodriver.

The second issue is that mozrunner entirely ignores user-specific
installations of Firefox.

This patch looks at the two following keys, which seems to more
reliably be able to tell us the location of the Firefox executable,
irregardless of the process executable's bitness:

HKEY_CURRENT_USER\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\firefox.exe
HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\firefox.exe

When they exist, these should return:

(Default)    REG_SZ    C:\Program Files (x86)\FirefoxESR\firefox.exe
Path    REG_SZ    C:\Program Files (x86)\FirefoxESR\firefox.exe

Fixes: https://github.com/mozilla/geckodriver/issues/1045

Comment 2

[Andreas Tolfsen ❲:ato❳]

I’ve attached a patch for this based on the Windows Registry keys
suggested on GitHub. It compiles fine, but since I don’t have
access to a Windows system I have not been able to test it. I will
reach out to someone on the GitHub issue ticket to ask for help on
that, using a build off try.

Comment 3

[BugBot [:suhaib / :marco]]

The priority flag is not set for this bug.
:jgraham, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Andreas, would you mind creating new try builds for testing it on both 32 and 64 systems? You can use the toolchain builds when pushing to try. Thanks.

Comment 5

[Andreas Tolfsen ❲:ato❳]

new builds:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4fb01f284af80dbcbc6d9a617bfdc733189ca7f

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The patch isn't working, and it actually doesn't hard-block the 0.26.0 release which we want to ship soon. So moving to 0.27.0.

Comment 7

[Andreas Tolfsen ❲:ato❳]

Without access to a Windows test system, I’m not able to continue with this.

The attached patch is a step in the right direction, and all that’s remaining is to find the right canonical Windows Registry path for the Firefox executable and encode that correctly.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Jim, do you have any advice for us what the best REGEX key on Windows is to find the right Firefox executable (32bit vs 64 bit)? Also not only on the default installation path, but also if users have it installed themselves in their user app folder? Thanks!

Comment 9

[Jim Mathies [:jimm]]

Sounds more installer related. Adam, any thoughts here?

Comment 10

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I think whimboo's comment on the patch is correct: The name of the key includes firefox.exe. The name of the value is just an empty string; "(Default)" is just what regedit shows as the name for the default value of a key, which has no name.

So you need something like (untested):

let maybe_exe =
  app_paths
    .open_subkey("firefox.exe")
    .and_then(|exe_key| -> Result<OsString, _> { exe_key.get_value("") });

if let Ok(exe) = maybe_exe {
  let path = PathBuf::from(exe);
  if is_binary(&path) {
    return Ok(Some(path));
  }
}

(edit: simplified example a bit)

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This is a rare case, which could be worked around by just using the right binary of geckodriver. Given that we don't have the time right now to get this fixed I will remove it as blocker for any upcoming geckodriver release.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I filed bug 1921933 to split-off the search in HKCU so that we keep this bug for the differences when running on different architectures.