tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit f58b60e4163f0b630d393f51a25ab20a6782630d
parent 00777a572262b60acd7f4a92a52ab769f4afa47d
Author: Dave Townsend <dtownsend@oxymoronical.com>
Date:   Fri,  9 Jan 2026 09:09:42 +0000

Bug 1994700: Correctly resolve relative paths from the database. r=profiles-reviewers,jhirsch

I could not find any existing way to resolve a relative path that both supports parent directory
segments (PathUtils.joinRelative does not support parent directory segments) and doesn't modify
the path in the presence of symlinks (PathUtils.normalize does this).

Differential Revision: https://phabricator.services.mozilla.com/D273247

Diffstat:
Mbrowser/components/profiles/SelectableProfile.sys.mjs | 52++++++++++++++++++++++++++++++++++++++++++----------
Mbrowser/components/profiles/SelectableProfileService.sys.mjs | 11+++++++----
Mbrowser/components/profiles/tests/unit/head.js | 19+++++--------------
Mbrowser/components/profiles/tests/unit/test_create_profile.js | 16++++++++++++++++
Mbrowser/components/profiles/tests/unit/test_recover_storeID.js | 3++-
Abrowser/components/profiles/tests/unit/test_relative_profile_path.js | 70++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mbrowser/components/profiles/tests/unit/xpcshell.toml | 2++
Mtoolkit/profile/nsToolkitProfileService.cpp | 7+++++--
Mtoolkit/profile/nsToolkitProfileService.h | 1+
Mtoolkit/profile/test/xpcshell/head.js | 11+++++------
Mtoolkit/profile/test/xpcshell/test_async_flush_current_profile.js | 28++++++++++++++++++++++++----
11 files changed, 179 insertions(+), 41 deletions(-)

diff --git a/browser/components/profiles/SelectableProfile.sys.mjs b/browser/components/profiles/SelectableProfile.sys.mjs @@ -53,6 +53,42 @@ function standardAvatarURL(avatar, size = "80") { } /** + * Resolve a relative path against an absolute path. The relative path may only + * contain parent directory or child directory parts. + * + * @param {string} absolute The absolute path + * @param {string} relative The relative path + * @returns {string} The resolved path + */ +function resolveDir(absolute, relative) { + let target = absolute; + + for (let pathPart of PathUtils.splitRelative(relative, { + allowParentDir: true, + })) { + if (pathPart === "..") { + target = PathUtils.parent(target); + + // On Windows there is no notion of a single root directory. Instead each + // disk has a root directory. Traversing to a different disk means allowing + // going above the root of the first disk then the next path part will be + // the new disk. + if (!target && AppConstants.platform != "win") { + throw new Error("Invalid path"); + } + } else { + target = target ? PathUtils.join(target, pathPart) : pathPart; + } + } + + if (!target) { + throw new Error("Invalid path"); + } + + return target; +} + +/** * The selectable profile */ export class SelectableProfile { @@ -131,7 +167,7 @@ export class SelectableProfile { * @returns {string} Path of profile */ get path() { - return PathUtils.joinRelative( + return resolveDir( ProfilesDatastoreService.constructor.getDirectory("UAppData").path, this.#path ); @@ -154,15 +190,11 @@ export class SelectableProfile { * the profile local directory */ get localDir() { - return this.rootDir.then(root => { - let relative = root.getRelativePath( - ProfilesDatastoreService.constructor.getDirectory("DefProfRt") - ); - let local = - ProfilesDatastoreService.constructor.getDirectory("DefProfLRt"); - local.appendRelativePath(relative); - return local; - }); + return this.rootDir.then(root => + ProfilesDatastoreService.toolkitProfileService.getLocalDirFromRootDir( + root + ) + ); } /** diff --git a/browser/components/profiles/SelectableProfileService.sys.mjs b/browser/components/profiles/SelectableProfileService.sys.mjs @@ -1616,10 +1616,9 @@ export class CommandLineHandler { async findDefaultProfilePath() { try { let profilesRoot = - ProfilesDatastoreService.constructor.getDirectory("DefProfRt").parent - .path; + ProfilesDatastoreService.constructor.getDirectory("UAppData"); - let iniPath = PathUtils.join(profilesRoot, "profiles.ini"); + let iniPath = PathUtils.join(profilesRoot.path, "profiles.ini"); let iniData = await IOUtils.readUTF8(iniPath); @@ -1650,7 +1649,11 @@ export class CommandLineHandler { let isRelative = iniParser.getString(section, "IsRelative") == "1"; if (isRelative) { - path = PathUtils.joinRelative(profilesRoot, path); + let profileDir = Cc["@mozilla.org/file/local;1"].createInstance( + Ci.nsIFile + ); + profileDir.setRelativeDescriptor(profilesRoot, path); + path = profileDir.path; } return path; diff --git a/browser/components/profiles/tests/unit/head.js b/browser/components/profiles/tests/unit/head.js @@ -4,21 +4,12 @@ "use strict"; -const { SelectableProfile } = ChromeUtils.importESModule( - "resource:///modules/profiles/SelectableProfile.sys.mjs" -); -const { Sqlite } = ChromeUtils.importESModule( - "resource://gre/modules/Sqlite.sys.mjs" -); - const lazy = {}; -ChromeUtils.defineLazyGetter(lazy, "SelectableProfileService", () => { - const { SelectableProfileService } = ChromeUtils.importESModule( - "resource:///modules/profiles/SelectableProfileService.sys.mjs" - ); - - return SelectableProfileService; +ChromeUtils.defineESModuleGetters(lazy, { + SelectableProfileService: + "resource:///modules/profiles/SelectableProfileService.sys.mjs", + Sqlite: "resource://gre/modules/Sqlite.sys.mjs", }); ChromeUtils.defineLazyGetter(lazy, "ProfilesDatastoreService", () => { @@ -101,7 +92,7 @@ async function openDatabase() { let dbFile = Services.dirsvc.get("UAppData", Ci.nsIFile); dbFile.append("Profile Groups"); dbFile.append(`${getProfileService().currentProfile.storeID}.sqlite`); - return Sqlite.openConnection({ + return lazy.Sqlite.openConnection({ path: dbFile.path, openNotExclusive: true, }); diff --git a/browser/components/profiles/tests/unit/test_create_profile.js b/browser/components/profiles/tests/unit/test_create_profile.js @@ -76,4 +76,20 @@ add_task(async function test_create_profile() { profiles = await SelectableProfileService.getAllProfiles(); Assert.equal(profiles.length, 2, "Two selectable profiles exist"); + + let db = await openDatabase(); + let rows = await db.execute("SELECT path FROM Profiles WHERE id=:id;", { + id: newProfile.id, + }); + await db.close(); + + Assert.equal(rows.length, 1, "There should be one row for the profile"); + let path = rows[0].getResultByName("path"); + + // Non-unix and mac prefix the profile path with "Profiles/" + if (!AppConstants.XP_UNIX || AppConstants.platform == "macosx") { + path = path.substring("Profiles".length + 1); + } + + Assert.equal(path, leafName, "The profile path should be relative"); }); diff --git a/browser/components/profiles/tests/unit/test_recover_storeID.js b/browser/components/profiles/tests/unit/test_recover_storeID.js @@ -16,7 +16,8 @@ add_task(async function test_recover_storeID() { await IOUtils.makeDirectory(groupsPath); let dbFile = PathUtils.join(groupsPath, "foobar.sqlite"); - let db = await Sqlite.openConnection({ + // eslint-disable-next-line mozilla/valid-lazy + let db = await lazy.Sqlite.openConnection({ path: dbFile, openNotExclusive: true, }); diff --git a/browser/components/profiles/tests/unit/test_relative_profile_path.js b/browser/components/profiles/tests/unit/test_relative_profile_path.js @@ -0,0 +1,70 @@ +/* Any copyright is dedicated to the Public Domain. +https://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +const PATH_SEPARATOR = AppConstants.platform == "win" ? "\\" : "/"; + +add_setup(() => { + Services.prefs.setBoolPref("browser.profiles.enabled", true); +}); + +add_task(async function test_create_profile() { + let hash = xreDirProvider.getInstallHash(); + + // In the test harness gProfD is outside of the mocked app data directory so + // this will will use a relative profile path starting with `..`. + let absolutePath = gProfD.clone(); + absolutePath.append("absoluteProfile"); + absolutePath.create(Ci.nsIFile.DIRECTORY_TYPE, 0o755); + + let profileData = { + profiles: [ + { + name: "default", + path: absolutePath.path, + isRelative: false, + }, + ], + installs: { + [hash]: { + default: absolutePath.path, + }, + }, + }; + + writeProfilesIni(profileData); + + startProfileService(); + let service = getProfileService(); + Assert.equal(service.currentProfile.rootDir.path, absolutePath.path); + + await initSelectableProfileService(); + + let currentProfile = getSelectableProfileService().currentProfile; + + Assert.equal( + (await currentProfile.rootDir).path, + absolutePath.path, + "The profile root path should be correct" + ); + + Assert.equal( + (await currentProfile.localDir).path, + absolutePath.path, + "The profile local path should be correct" + ); + + let db = await openDatabase(); + let rows = await db.execute("SELECT path FROM Profiles WHERE id=:id;", { + id: currentProfile.id, + }); + await db.close(); + + Assert.equal(rows.length, 1, "There should be one row for the profile"); + Assert.equal( + rows[0].getResultByName("path"), + `..${PATH_SEPARATOR}absoluteProfile`, + "The profile path in the database should be relative" + ); +}); diff --git a/browser/components/profiles/tests/unit/xpcshell.toml b/browser/components/profiles/tests/unit/xpcshell.toml @@ -32,6 +32,8 @@ skip-if = [ ["test_recover_storeID.js"] +["test_relative_profile_path.js"] + ["test_selectable_profile_launch.js"] ["test_selectable_profile_service_exists.js"] diff --git a/toolkit/profile/nsToolkitProfileService.cpp b/toolkit/profile/nsToolkitProfileService.cpp @@ -2372,6 +2372,10 @@ nsresult WriteProfileInfo(nsIFile* profilesDBFile, nsIFile* installDBFile, rv = profilesIni.SetString(iniSection.get(), "Path", profileInfo->mPath.get()); NS_ENSURE_SUCCESS(rv, rv); + + rv = profilesIni.SetString(iniSection.get(), "IsRelative", + profileInfo->mIsRelative ? "1" : "0"); + NS_ENSURE_SUCCESS(rv, rv); changed = true; // We must update the install default profile if it matches the old profile. @@ -2443,8 +2447,7 @@ nsToolkitProfileService::AsyncFlushCurrentProfile(JSContext* aCx, profileData->mStoreID = mCurrent->mStoreID; profileData->mShowSelector = mCurrent->mShowProfileSelector; - bool isRelative; - GetProfileDescriptor(mCurrent, &isRelative, profileData->mPath); + GetProfileDescriptor(mCurrent, &profileData->mIsRelative, profileData->mPath); nsCOMPtr<nsIRemoteService> rs = GetRemoteService(); RefPtr<nsRemoteService> remoteService = diff --git a/toolkit/profile/nsToolkitProfileService.h b/toolkit/profile/nsToolkitProfileService.h @@ -25,6 +25,7 @@ struct CurrentProfileData { nsCString mPath; nsCString mStoreID; bool mShowSelector; + bool mIsRelative; }; struct IniData { diff --git a/toolkit/profile/test/xpcshell/head.js b/toolkit/profile/test/xpcshell/head.js @@ -104,12 +104,14 @@ const BACKGROUNDTASKS_PROFILE_DATA = (() => { { name: "Profile1", path: "Path1", + isRelative: true, storeID: null, default: false, }, { name: "Profile3", path: "Path3", + isRelative: true, storeID: null, default: false, }, @@ -274,7 +276,8 @@ function writeProfilesIni(profileData) { let section = `Profile${i}`; ini.setString(section, "Name", profile.name); - ini.setString(section, "IsRelative", 1); + let isRelative = profile.isRelative ?? true; + ini.setString(section, "IsRelative", isRelative ? "1" : "0"); ini.setString(section, "Path", profile.path); if ("storeID" in profile) { ini.setString(section, "StoreID", profile.storeID); @@ -364,15 +367,11 @@ function readProfilesIni() { if (isRelative === null) { break; } - Assert.equal( - isRelative, - "1", - "Paths should always be relative in these tests." - ); let profile = { name: safeGet(ini, section, "Name"), path: safeGet(ini, section, "Path"), + isRelative: isRelative == "1", // TODO: currently, if there's a StoreID key but no value, this gets // translated into JS as an empty string, while if there's no StoreID // in the file at all, then it gets translated into JS as null. diff --git a/toolkit/profile/test/xpcshell/test_async_flush_current_profile.js b/toolkit/profile/test/xpcshell/test_async_flush_current_profile.js @@ -13,6 +13,10 @@ add_task( let hash = xreDirProvider.getInstallHash(); let defaultProfile = makeRandomProfileDir("default"); let otherProfile = makeRandomProfileDir("other"); + let absoluteProfile = gProfD.clone(); + absoluteProfile.append("absolute"); + absoluteProfile.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755); + let storeID = "b0bacafe"; let profilesIni = { profiles: [ @@ -67,21 +71,37 @@ add_task( // Now, simulate the default profile receiving app focus: asyncFlush would // fail, since profiles.ini has been updated since startup, but we should // then fall back to asyncFlushCurrentProfile, which should succeed. - let asyncRewriteDefault = async () => { + let asyncRewriteDefault = async (expectedPath, expectedRelative) => { await service.asyncFlushCurrentProfile(); let profileData = readProfilesIni(); Assert.equal( profileData.profiles[0].path, - defaultProfile.leafName, + expectedPath, + "AsyncFlushCurrentProfile should have updated the path to the path of the current managed profile" + ); + + Assert.equal( + profileData.profiles[0].isRelative, + expectedRelative, + "AsyncFlushCurrentProfile should have updated IsRelative correctly" + ); + + Assert.equal( + profileData.installs[hash].default, + expectedPath, "AsyncFlushCurrentProfile should have updated the path to the path of the current managed profile" ); }; - await asyncRewriteDefault(); + await asyncRewriteDefault(defaultProfile.leafName, true); // Just to be sure, repeat the other instance setting itself to default, // then this instance flushing over top of those changes. overwriteProfilesIni(); - await asyncRewriteDefault(); + await asyncRewriteDefault(defaultProfile.leafName, true); + + // Now change the root dir and flush. + service.currentProfile.rootDir = absoluteProfile; + await asyncRewriteDefault(absoluteProfile.path, false); } );