1555319 - The case insensitive file system on Windows makes it possible to run Firefox with seemingly different installation paths

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect, P1)

Description

[Dave Townsend [:mossop]]

How you run Firefox on Windows can make XRE_EXECUTABLE_FILE differ in its case. This causes Firefox to think it is installed in different locations

STR:

1. Start -> Run -> C:\Program Files\Mozilla Firefox\firefox.exe
2. Start -> Run -> C:\program files\mozilla firefox\firefox.exe

This launches two different instances of Firefox with two different profiles.

This may be possible on other operating systems, though it is rare that they run with a case insensitive filesystem.

Comment 1

[Dave Townsend [:mossop]]

Matt, does Windows have a way to get the correct case for a path?

Comment 2

[Molly Howell (she/her) (no longer active)]

I don't think there's one function that you can pass an entire path to and get the correctly cased version back; you have to do it recursively by calling FindFirstFile on each path component (and fixing the drive letter manually).

But if all you need is the path to the running executable, GetModuleFileName should return a consistent version of that.

Comment 3

[Dave Townsend [:mossop]]

(In reply to Matt Howell (he/him) [:mhowell] from comment #2)

But if all you need is the path to the running executable, GetModuleFileName should return a consistent version of that.

As far as I can tell that is what we're already doing: https://searchfox.org/mozilla-central/source/xpcom/build/BinaryPath.h#83

Comment 4

[Molly Howell (she/her) (no longer active)]

You are correct, I didn't realize that was how that worked, that is not helpful at all. I guess that leaves you with the first thing I mentioned; the only example of that method that I have to show you is the installer's GetLongPath function, which does solve this problem but is possibly less than helpful as an example. The algorithm it's doing is:

1. Call GetFullPathName on the entire path to resolve relative paths and normalize backslashes.
2. Manually uppercase the drive letter.
3. For each path component, call FindFirstFile on the path up to that component, then append the cFileName member of the WIN32_FIND_DATA struct that it returns (which will have the canonical casing) to a buffer. Then return that buffer.

Comment 5

[Zertrin]

Thanks, this might be what happened to me yesterday in Bug 1553526 since I noticed that in the registry I have these keys:

HKEY_CLASSES_ROOT\FirefoxURL\shell\open\command = "C:\Program Files\mozilla firefox\firefox.exe" -osint -url "%1"
HKEY_CLASSES_ROOT\FirefoxURL-308046B0AF4A39CB\shell\open\command = "C:\Program Files\Mozilla Firefox\firefox.exe" -osint -url "%1"
HKEY_CLASSES_ROOT\FirefoxURL-CA9422711AE1A81C\shell\open\command = "C:\Program Files\Firefox Developer Edition\firefox.exe" -osint -url "%1"

(with HKEY_CURRENT_USER\SOFTWARE\Microsoft\Windows\Shell\Associations\URLAssociations(http|https)\UserChoice set to FirefoxURL)

Would you recommend (as a workaround) to edit HKEY_CLASSES_ROOT\FirefoxURL\shell\open\command to use Mozilla Firefox instead of mozilla firefox? (might be useful to other users affected by this problem and landing here, until FF find a proper solution to the problem)

Comment 6

[Molly Howell (she/her) (no longer active)]

That workaround should be fine, yes. If our installer made that registry entry, then it's a bug that it got created with that casing in the first place, but we probably fixed that in bug 1510276.

Comment 8

[Dave Townsend [:mossop]]

Ok, seems we do have a function to fix paths with incorrect cases (https://searchfox.org/mozilla-central/source/widget/windows/WinUtils.cpp#1897) and that function is already applied to GreBinD which appears to be the same path that we are hashing (the parent directory of the executable). So switching to hashing that path should solve this problem. We just need to figure out what to do for users who now have different profiles for the different ways they are running Firefox.

Comment 9

[therube]

(Some versions of Windows do support Case Sensitivity, https://www.voidtools.com/forum/viewtopic.php?f=4&t=8171.)

Comment 11

[Neha Kochar [:neha]]

Dave, is this a regression introduced in 67?

Comment 12

[Dave Townsend [:mossop]]

(In reply to Neha Kochar [:neha] from comment #11)

Dave, is this a regression introduced in 67?

Not sure how to answer that. It means that a feature introduced in 67 isn't working correctly.

Comment 13

[Dave Townsend [:mossop]]

Attachment: Bug 1555319: Normalize the case of the installation path to always get a consistent install hash. r=froydnj

The XRE_EXECUTABLE_FILE directory entry gives us the actual path that the binary
was launched with. On systems where the filesystem is case insensitive this
can be in any case, which ends up being a different install hash. This patch
ensures that we get the correct case for the install path before generating the
hash.

We have the problem of users who are already affected by this issue. This patch
also leaves the old hash available, if no default profile is found for the
correct hash then we also check for a profile for the old hash, if so we use it
for this hash going forwards. Testing this is kind of a pain, we have to add a
way to override the old hash that we will check against. I'm not totally happy with
how it is done here but not sure there is anything better.

This also adds a test that calling xpcshell with differing cases returns the
same install hash.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7aef19c3fd3d
Normalize the case of the installation path to always get a consistent install hash. r=froydnj

Comment 15

[Dorel Luca [:dluca]]

Backed out changeset 7aef19c3fd3d (bug 1555319) for build bustage on OSX. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=253156483&repo=autoland

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=253156483&revision=7aef19c3fd3dd72dc7779e67a582fca6d2595643

Backout:
https://hg.mozilla.org/integration/autoland/rev/4a415941bef62b74389de9520e60fc7cc5cf3962

Comment 16

[Julien Cristau [:jcristau]]

I think it's too late for 68 at this point. We may want to uplift to esr68 down the line.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a595782402c
Normalize the case of the installation path to always get a consistent install hash. r=froydnj

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa7807c7e488
Follow-up to correct the static analysis issue on OSX. r=bustage-fix

Comment 19

[Natalia Csoregi [:nataliaCs]]

Backed out for xpcshell failures on test_install_hash.js

backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/9f8ae6dbdee31f3b0e54a38d535b87b19daa41d6

push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=aa7807c7e488ac6443bde4f436bebf601309f168&searchStr=xpcshell&group_state=expanded&selectedJob=253566695

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253566748&repo=mozilla-inbound&lineNumber=2603

task 2019-06-26T18:59:19.856Z] 18:59:19 INFO - TEST-START | toolkit/xre/test/test_install_hash.js
[task 2019-06-26T18:59:20.688Z] 18:59:20 WARNING - TEST-UNEXPECTED-FAIL | toolkit/xre/test/test_install_hash.js | xpcshell return code: 0
[task 2019-06-26T18:59:20.688Z] 18:59:20 INFO - TEST-INFO took 834ms
[task 2019-06-26T18:59:20.688Z] 18:59:20 INFO - >>>>>>>
[task 2019-06-26T18:59:20.690Z] 18:59:20 INFO - PID 18617 | ERROR 2019-06-26T18:59:19Z: audio_thread_priority::rt_linux: Could not make thread real-time.
[task 2019-06-26T18:59:20.690Z] 18:59:20 INFO - PID 18617 | [18617, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2555
[task 2019-06-26T18:59:20.690Z] 18:59:20 INFO - PID 18617 | Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2019-06-26T18:59:20.690Z] 18:59:20 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2019-06-26T18:59:20.692Z] 18:59:20 INFO - (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2019-06-26T18:59:20.693Z] 18:59:20 INFO - (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2019-06-26T18:59:20.693Z] 18:59:20 INFO - running event loop
[task 2019-06-26T18:59:20.693Z] 18:59:20 INFO - PID 18617 | [18617, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /builds/worker/workspace/build/src/dom/media/CubebUtils.cpp, line 391
[task 2019-06-26T18:59:20.694Z] 18:59:20 INFO - "CONSOLE_MESSAGE: (info) No chrome package registered for chrome://branding/locale/brand.properties"
[task 2019-06-26T18:59:20.696Z] 18:59:20 INFO - toolkit/xre/test/test_install_hash.js | Starting testSameBinary
[task 2019-06-26T18:59:20.697Z] 18:59:20 INFO - (xpcshell/head.js) | test testSameBinary pending (2)
[task 2019-06-26T18:59:20.698Z] 18:59:20 INFO - (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2019-06-26T18:59:20.699Z] 18:59:20 INFO - PID 18617 | [18617, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, kKnownEsrVersion) failed with result 0x80004002: file /builds/worker/workspace/build/src/toolkit/components/resistfingerprinting/nsRFPService.cpp, line 662
[task 2019-06-26T18:59:20.701Z] 18:59:20 INFO - PID 18617 | ERROR 2019-06-26T18:59:20Z: audio_thread_priority::rt_linux: Could not make thread real-time.
[task 2019-06-26T18:59:20.702Z] 18:59:20 INFO - PID 18617 | [18635, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /builds/worker/workspace/build/src/dom/media/CubebUtils.cpp, line 391
[task 2019-06-26T18:59:20.703Z] 18:59:20 INFO - PID 18617 | [18635, Main Thread] WARNING: OOPDeinit() without successful OOPInit(): file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 3037
[task 2019-06-26T18:59:20.704Z] 18:59:20 INFO - PID 18617 | [18635, Main Thread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/workspace/build/src/xpcom/base/nsTraceRefcnt.cpp, line 194
[task 2019-06-26T18:59:20.706Z] 18:59:20 INFO - PID 18617 | [18635, Main Thread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/workspace/build/src/xpcom/base/nsTraceRefcnt.cpp, line 194
[task 2019-06-26T18:59:20.707Z] 18:59:20 WARNING - TEST-UNEXPECTED-FAIL | toolkit/xre/test/test_install_hash.js | testSameBinary - [testSameBinary : 69] Should have the same hash when running the same binary. - "59791E7BED559D6F\nCouldn't convert chrome URL: chrome://branding/locale/brand.properties\nnsStringStats\n => mAllocCount: 9511\n => mReallocCount: 0\n => mFreeCount: 9511\n => mShareCount: 8701\n => mAdoptCount: 47\n => mAdoptFreeCount: 47\n => Process ID: 18635, Thread ID: 140553406239872" == "59791E7BED559D6F"
[task 2019-06-26T18:59:20.708Z] 18:59:20 INFO - /builds/worker/workspace/build/tests/xpcshell/tests/toolkit/xre/test/test_install_hash.js:testSameBinary:69
[task 2019-06-26T18:59:20.709Z] 18:59:20 INFO - exiting test

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9c66afa36c9
Normalize the case of the installation path to always get a consistent install hash. r=froydnj

Comment 21

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/f9c66afa36c9

Comment 23

[maroonmoon]

Hello,
I had raised a Visual Studio bug report which can be seen here:
https://developercommunity.visualstudio.com/content/problem/598424/create-a-pull-request-opens-in-a-new-profile-of-fi.html?childToView=639464

It seems this bug fix should solve the issue. Anyway this can be checked?

I also have a FF bug report here: https://bugzilla.mozilla.org/show_bug.cgi?id=1560333 which I guess can be linked to this report and closed as well.

Regards

Comment 24

[Dave Townsend [:mossop]]

Comment on attachment 9071724 [details]
Bug 1555319: Normalize the case of the installation path to always get a consistent install hash. r=froydnj

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This breaks profile selection in some cases
• User impact if declined: The user will get different profiles depending on launch method.
• Fix Landed on Version: 69
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Has seen plenty of testing on nightly with no issues.
• String or UUID changes made by this patch: None

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9071724 [details]
Bug 1555319: Normalize the case of the installation path to always get a consistent install hash. r=froydnj

Fixes some fallout from the profile-per-install changes. Well-baked on Nightly. Approved for 68.1esr.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/e09145bef79d

Comment 28

[Rami]

Thank you for fixing this issue Dave.

I can confirm the bug is fixed in Firefox 69.