Skip to content

Commit

Permalink
fix: Fix unlinking existing binaries (shaka-project#12)
Browse files Browse the repository at this point in the history
We failed to unlink binaries because we checked for the existence of
"path" instead of the correct variable name "outputPath".  This went
unnoticed because "path" is a global referring to the "path" module,
so the code still ran.

Another factor that made this more difficult to notice is that in
other methods, we clobbered the global module "path" with a method
parameter of the same name.  This has also been fixed by renaming
method parameters from "path" to something else.
  • Loading branch information
joeyparrish authored Feb 17, 2022
1 parent d3a3782 commit 8a43765
Showing 1 changed file with 20 additions and 19 deletions.
39 changes: 20 additions & 19 deletions utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,37 +84,37 @@ class InstallerUtils {
/**
* Get a version number from the Windows registry.
*
* @param {string} path
* @param {string} regPath
* @param {string} key
* @return {!Promise<?string>}
*/
static async getWindowsRegistryVersion(path, key) {
static async getWindowsRegistryVersion(regPath, key) {
if (os.platform() != 'win32') {
return null;
}

// Try the 64-bit registry first, then fall back to the 32-bit registry.
// Necessary values could be in either location.
let result = await regQuery(path, '64');
if (!result[path].exists || !result[path].values[key]) {
result = await regQuery(path, '32');
let result = await regQuery(regPath, '64');
if (!result[regPath].exists || !result[regPath].values[key]) {
result = await regQuery(regPath, '32');
}
if (!result[path].exists || !result[path].values[key]) {
if (!result[regPath].exists || !result[regPath].values[key]) {
return null;
}

return result[path].values[key].value;
return result[regPath].values[key].value;
}

/**
* Test if a file exists.
*
* @param {path}
* @param {filePath}
* @return {!Promise<boolean}
*/
static async fileExists(path) {
static async fileExists(filePath) {
try {
await fsPromises.stat(path);
await fsPromises.stat(filePath);
return true;
} catch (error) {
return false;
Expand All @@ -124,30 +124,31 @@ class InstallerUtils {
/**
* Get a version number from the metadata of a Windows executable.
*
* @param {string} path
* @param {string} executablePath
* @return {!Promise<?string>}
*/
static async getWindowsExeVersion(path) {
static async getWindowsExeVersion(executablePath) {
if (os.platform() != 'win32') {
return null;
}

if (!(await InstallerUtils.fileExists(path))) {
if (!(await InstallerUtils.fileExists(executablePath))) {
// No such file.
// If it's a relative path, ask the registry for a full one.
if (!path.includes('/') && !path.includes('\\')) {
path = await InstallerUtils.getWindowsRegistryVersion(
WINDOWS_REGISTRY_APP_PATHS + path,
if (!executablePath.includes('/') && !executablePath.includes('\\')) {
executablePath = await InstallerUtils.getWindowsRegistryVersion(
WINDOWS_REGISTRY_APP_PATHS + executablePath,
'');
if (!path || !(await InstallerUtils.fileExists(path))) {
if (!executablePath ||
!(await InstallerUtils.fileExists(executablePath))) {
return null;
}
}
}

const result = await InstallerUtils.runCommand([
'powershell',
`(Get-Item "${path}").VersionInfo.ProductVersion`,
`(Get-Item "${executablePath}").VersionInfo.ProductVersion`,
]);

const output = result.stdout.trim();
Expand Down Expand Up @@ -330,7 +331,7 @@ class InstallerUtils {
// permission errors overwriting it. Unlinking it first will ensure the
// newly-written file is a fresh filesystem inode that doesn't conflict
// with what's running.
if (await InstallerUtils.fileExists(path)) {
if (await InstallerUtils.fileExists(outputPath)) {
await fsPromises.unlink(outputPath);
}

Expand Down

0 comments on commit 8a43765

Please sign in to comment.