This is an automated email from the git hooks/post-receive script.
pierov pushed a commit to branch geckoview-99.0.1-11.0-1 in repository tor-browser.
commit 4c0756a5ad149b69e5882a84cb1c9e49393d6396 Author: Luca Greco lgreco@mozilla.com AuthorDate: Tue Mar 15 20:13:16 2022 +0000
Bug 1759162 - Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r=barret,willdurand a=pascalc
PathUtils.filename throws if the path is not an absolute path, which was likely not the case with the OS.Path.basename call used previously.
This internal change of behavior shouldn't be triggering any issue on Windows, where the hostInfo.manifest.path seems to be always normalized into an absolute path (by computing it as relative to the hostInfo.manifest if it wasn't already an absolute path), but it makes browser.pkcs11.isModuleInstalled to regress on Linux (and maybe also on MacOS if the pkcs11 manifest files include only the library name and not its full path, as it seems to be the case for the Belgium eID pkcs11 manifest packaged for Linux).
The result of PathUtils.filename is expected to only include the basename of the file (without the full dir path and the file extension) and so to fix the regression being triggered on non-windows platform we could use a fake absolute url to get the expected result using PathUtils.filename as is.
Differential Revision: https://phabricator.services.mozilla.com/D141097 --- browser/components/extensions/parent/ext-pkcs11.js | 34 +++++++++++++++------- .../test/xpcshell/test_ext_pkcs11_management.js | 17 +++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/browser/components/extensions/parent/ext-pkcs11.js b/browser/components/extensions/parent/ext-pkcs11.js index db6a6af7c87b0..5f72786cae4a7 100644 --- a/browser/components/extensions/parent/ext-pkcs11.js +++ b/browser/components/extensions/parent/ext-pkcs11.js @@ -39,19 +39,31 @@ this.pkcs11 = class extends ExtensionAPI { context ); if (hostInfo) { - if (AppConstants.platform === "win") { - // If the path specified in the manifest is not an abslute path, - // translate it relative to manifest's directory. - if (!PathUtils.isAbsolute(hostInfo.manifest.path)) { - hostInfo.manifest.path = PathUtils.normalize( - PathUtils.joinRelative( - PathUtils.parent(hostInfo.path), - hostInfo.manifest.path - ) + // We don't normalize the absolute path below because + // `Path.normalize` throws when the target file doesn't + // exist, and that might be the case on non Windows + // builds. + let absolutePath = PathUtils.isAbsolute(hostInfo.manifest.path) + ? hostInfo.manifest.path + : PathUtils.joinRelative( + PathUtils.parent(hostInfo.path), + hostInfo.manifest.path ); - } + + if (AppConstants.platform === "win") { + // On Windows, `hostInfo.manifest.path` is expected to be a normalized + // absolute path. On other platforms, this path may be relative but we + // cannot use `PathUtils.normalize()` on non-absolute paths. + absolutePath = PathUtils.normalize(absolutePath); + hostInfo.manifest.path = absolutePath; } - let manifestLib = PathUtils.filename(hostInfo.manifest.path); + + // PathUtils.filename throws if the path is not an absolute path. + // The result is expected to be the basename of the file (without + // the dir path and the extension) so it is fine to use an absolute + // path that may not be normalized (non-Windows platforms). + let manifestLib = PathUtils.filename(absolutePath); + if (AppConstants.platform !== "linux") { manifestLib = manifestLib.toLowerCase(manifestLib); } diff --git a/browser/components/extensions/test/xpcshell/test_ext_pkcs11_management.js b/browser/components/extensions/test/xpcshell/test_ext_pkcs11_management.js index 36f66ecebdece..9d6dae0d6be72 100644 --- a/browser/components/extensions/test/xpcshell/test_ext_pkcs11_management.js +++ b/browser/components/extensions/test/xpcshell/test_ext_pkcs11_management.js @@ -111,6 +111,17 @@ async function setupManifests(modules) { add_task(async function test_pkcs11() { async function background() { try { + const { os } = await browser.runtime.getPlatformInfo(); + if (os !== "win") { + // Expect this call to not throw (explicitly cover regression fixed in Bug 1759162). + let isInstalledNonAbsolute = await browser.pkcs11.isModuleInstalled( + "testmoduleNonAbsolutePath" + ); + browser.test.assertFalse( + isInstalledNonAbsolute, + "PKCS#11 module with non absolute path expected to not be installed" + ); + } let isInstalled = await browser.pkcs11.isModuleInstalled("testmodule"); browser.test.assertFalse( isInstalled, @@ -247,6 +258,12 @@ add_task(async function test_pkcs11() { path: testmodule, id: "pkcs11@tests.mozilla.org", }, + { + name: "testmoduleNonAbsolutePath", + description: "PKCS#11 Test Module", + path: ctypes.libraryName("pkcs11testmodule"), + id: "pkcs11@tests.mozilla.org", + }, { name: "othermodule", description: "PKCS#11 Test Module",