Bug 1649611: Remove OS.File usage from DownloadIntegration.jsm and respect umask in IOUtils::SetPermissions r=barret

Differential Revision: https://phabricator.services.mozilla.com/D99729
This commit is contained in:
Emma Malysz 2021-01-15 21:42:10 +00:00
Родитель d625812205
Коммит 9497c44dcb
6 изменённых файлов: 96 добавлений и 67 удалений

Просмотреть файл

@ -166,11 +166,15 @@ namespace IOUtils {
*
* @param path An absolute file path
* @param permissions The UNIX file mode representing the permissions.
* @param honorUmask If omitted or true, any UNIX file mode value is
* modified by the process umask. If false, the exact value
* of UNIX file mode will be applied. This value has no effect
* on Windows.
*
* @return Resolves if the permissions were set successfully, otherwise
* rejects with a DOMException.
*/
Promise<void> setPermissions(DOMString path, unsigned long permissions);
Promise<void> setPermissions(DOMString path, unsigned long permissions, optional boolean honorUmask = true);
/**
* Return whether or not the file exists at the given path.
*

Просмотреть файл

@ -46,6 +46,10 @@
#include "prtime.h"
#include "prtypes.h"
#ifndef ANDROID
# include "nsSystemInfo.h"
#endif
#define REJECT_IF_SHUTTING_DOWN(aJSPromise) \
do { \
if (sShutdownStarted) { \
@ -447,7 +451,8 @@ already_AddRefed<Promise> IOUtils::GetChildren(GlobalObject& aGlobal,
/* static */
already_AddRefed<Promise> IOUtils::SetPermissions(GlobalObject& aGlobal,
const nsAString& aPath,
const uint32_t aPermissions) {
uint32_t aPermissions,
const bool aHonorUmask) {
MOZ_ASSERT(XRE_IsParentProcess());
RefPtr<Promise> promise = CreateJSPromise(aGlobal);
NS_ENSURE_TRUE(!!promise, nullptr);
@ -455,6 +460,12 @@ already_AddRefed<Promise> IOUtils::SetPermissions(GlobalObject& aGlobal,
nsCOMPtr<nsIFile> file = new nsLocalFile();
REJECT_IF_INIT_PATH_FAILED(file, aPath, promise);
#if defined(XP_UNIX) && !defined(ANDROID)
if (aHonorUmask) {
aPermissions &= ~nsSystemInfo::gUserUmask;
}
#endif
RunOnBackgroundThread<Ok>(
promise, [file = std::move(file), permissions = aPermissions]() {
return SetPermissionsSync(file, permissions);

Просмотреть файл

@ -104,7 +104,8 @@ class IOUtils final {
static already_AddRefed<Promise> SetPermissions(GlobalObject& aGlobal,
const nsAString& aPath,
const uint32_t aPermissions);
uint32_t aPermissions,
const bool aHonorUmask);
static already_AddRefed<Promise> Exists(GlobalObject& aGlobal,
const nsAString& aPath);

Просмотреть файл

@ -23,6 +23,38 @@
let stat = await IOUtils.stat(tempFile);
if (Services.appinfo.OS === "WINNT") {
// setPermissions ignores the x bit on Windows.
is(stat.permissions, 0o666, "Permissions munged on Windows");
} else {
let umask = Services.sysinfo.getProperty("umask");
is(stat.permissions, 0o421 & ~umask, "Permissions match");
}
await IOUtils.setPermissions(tempFile, 0o400);
stat = await IOUtils.stat(tempFile);
if (Services.appinfo.OS === "WINNT") {
is(stat.permissions, 0o444, "Permissions munged on Windows");
// We need to make the file writable to delete it on Windows.
await IOUtils.setPermissions(tempFile, 0o600);
} else {
is(stat.permissions, 0o400, "Permissions match");
}
await cleanup(tempFile);
});
add_task(async function test_setPermissionsWithoutHonoringUmask() {
const tempDir = await PathUtils.getTempDir();
const tempFile = PathUtils.join(tempDir, "setPermissions.tmp");
await IOUtils.writeUTF8(tempFile, "");
await IOUtils.setPermissions(tempFile, 0o421, false);
let stat = await IOUtils.stat(tempFile);
if (Services.appinfo.OS === "WINNT") {
// setPermissions ignores the x bit on Windows.
is(stat.permissions, 0o666, "Permissions munged on Windows");

Просмотреть файл

@ -58,7 +58,6 @@ ChromeUtils.defineModuleGetter(
"NetUtil",
"resource://gre/modules/NetUtil.jsm"
);
ChromeUtils.defineModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm");
ChromeUtils.defineModuleGetter(
this,
"PlacesUtils",
@ -264,7 +263,7 @@ var DownloadIntegration = {
this._store = new DownloadStore(
list,
OS.Path.join(OS.Constants.Path.profileDir, "downloads.json")
PathUtils.join(await PathUtils.getProfileDir(), "downloads.json")
);
this._store.onsaveitem = this.shouldPersistDownload.bind(this);
@ -371,7 +370,9 @@ var DownloadIntegration = {
Ci.nsIFile
);
directoryPath = directory.path;
await OS.File.makeDir(directoryPath, { ignoreExisting: true });
await IOUtils.makeDirectory(directoryPath, {
createAncestors: false,
});
} catch (ex) {
// Either the preference isn't set or the directory cannot be created.
directoryPath = await this.getSystemDownloadsDirectory();
@ -497,7 +498,7 @@ var DownloadIntegration = {
referrerInfo: aDownload.source.referrerInfo,
fileSize: aDownload.currentBytes,
sha256Hash: hash,
suggestedFileName: OS.Path.basename(aDownload.target.path),
suggestedFileName: PathUtils.filename(aDownload.target.path),
signatureInfo: sigInfo,
redirects: channelRedirects,
},
@ -604,40 +605,27 @@ var DownloadIntegration = {
// whatever reason.
zone = Ci.mozIDownloadPlatform.ZONE_INTERNET;
}
try {
// Don't write zone IDs for Local, Intranet, or Trusted sites
// to match Windows behavior.
if (zone >= Ci.mozIDownloadPlatform.ZONE_INTERNET) {
let streamPath = aDownload.target.path + ":Zone.Identifier";
let stream = await OS.File.open(
streamPath,
{ create: true },
{ winAllowLengthBeyondMaxPathWithCaveats: true }
);
try {
let zoneId = "[ZoneTransfer]\r\nZoneId=" + zone + "\r\n";
let { url, isPrivate, referrerInfo } = aDownload.source;
if (!isPrivate) {
let referrer = referrerInfo
? referrerInfo.computedReferrerSpec
: "";
zoneId +=
this._zoneIdKey("ReferrerUrl", referrer) +
this._zoneIdKey("HostUrl", url, "about:internet");
}
await stream.write(new TextEncoder().encode(zoneId));
} finally {
await stream.close();
// Don't write zone IDs for Local, Intranet, or Trusted sites
// to match Windows behavior.
if (zone >= Ci.mozIDownloadPlatform.ZONE_INTERNET) {
let path = aDownload.target.path + ":Zone.Identifier";
try {
let zoneId = "[ZoneTransfer]\r\nZoneId=" + zone + "\r\n";
let { url, isPrivate, referrerInfo } = aDownload.source;
if (!isPrivate) {
let referrer = referrerInfo
? referrerInfo.computedReferrerSpec
: "";
zoneId +=
this._zoneIdKey("ReferrerUrl", referrer) +
this._zoneIdKey("HostUrl", url, "about:internet");
}
await IOUtils.writeUTF8(path, new TextEncoder().encode(zoneId));
} catch (ex) {
// If writing to the file fails, we ignore the error and continue.
if (!(ex instanceof DOMException)) {
Cu.reportError(ex);
}
}
} catch (ex) {
// If writing to the stream fails, we ignore the error and continue.
// The Windows API error 123 (ERROR_INVALID_NAME) is expected to
// occur when working on a file system that does not support
// Alternate Data Streams, like FAT32, thus we don't report this
// specific error.
if (!(ex instanceof OS.File.Error) || ex.winLastError != 123) {
Cu.reportError(ex);
}
}
}
@ -660,26 +648,19 @@ var DownloadIntegration = {
));
// Permanently downloaded files are made accessible by other users on
// this system, while temporary downloads are marked as read-only.
let options = {};
let unixMode;
if (isTemporaryDownload) {
options.unixMode = 0o400;
options.winAttributes = { readOnly: true };
unixMode = 0o400;
} else {
options.unixMode = 0o666;
unixMode = 0o666;
}
// On Unix, the umask of the process is respected.
await OS.File.setPermissions(aDownload.target.path, options);
await IOUtils.setPermissions(aDownload.target.path, unixMode);
} catch (ex) {
// We should report errors with making the permissions less restrictive
// or marking the file as read-only on Unix and Mac, but this should not
// prevent the download from completing.
// The setPermissions API error EPERM is expected to occur when working
// on a file system that does not support file permissions, like FAT32,
// thus we don't report this error.
if (
!(ex instanceof OS.File.Error) ||
ex.unixErrno != OS.Constants.libc.EPERM
) {
if (!(ex instanceof DOMException)) {
Cu.reportError(ex);
}
}
@ -958,15 +939,15 @@ var DownloadIntegration = {
// We read the name of the directory from the list of translated strings
// that is kept by the UI helper module, even if this string is not strictly
// displayed in the user interface.
let directoryPath = OS.Path.join(
let directoryPath = PathUtils.join(
this._getDirectory(aName),
DownloadUIHelper.strings.downloadsFolder
);
// Create the Downloads folder and ignore if it already exists.
return OS.File.makeDir(directoryPath, { ignoreExisting: true }).then(
() => directoryPath
);
return IOUtils.makeDirectory(directoryPath, {
createAncestors: false,
}).then(() => directoryPath);
},
/**

Просмотреть файл

@ -83,25 +83,25 @@ add_task(async function test_getSystemDownloadsDirectory_exists_or_creates() {
) {
downloadDir = await DownloadIntegration.getSystemDownloadsDirectory();
Assert.equal(downloadDir, tempDir.path);
Assert.ok(await OS.File.exists(downloadDir));
Assert.ok(await IOUtils.exists(downloadDir));
let info = await OS.File.stat(downloadDir);
Assert.ok(info.isDir);
let info = await IOUtils.stat(downloadDir);
Assert.equal(info.type, "directory");
} else {
let targetPath = OS.Path.join(
let targetPath = PathUtils.join(
tempDir.path,
gStringBundle.GetStringFromName("downloadsFolder")
);
try {
await OS.File.removeEmptyDir(targetPath);
await IOUtils.remove(targetPath);
} catch (e) {}
downloadDir = await DownloadIntegration.getSystemDownloadsDirectory();
Assert.equal(downloadDir, targetPath);
Assert.ok(await OS.File.exists(downloadDir));
Assert.ok(await IOUtils.exists(downloadDir));
let info = await OS.File.stat(downloadDir);
Assert.ok(info.isDir);
await OS.File.removeEmptyDir(targetPath);
let info = await IOUtils.stat(downloadDir);
Assert.equal(info.type, "directory");
await IOUtils.remove(targetPath);
}
});
@ -163,8 +163,8 @@ add_task(async function test_getPreferredDownloadsDirectory() {
downloadDir = await DownloadIntegration.getPreferredDownloadsDirectory();
Assert.notEqual(downloadDir, "");
Assert.equal(downloadDir, tempDir.path);
Assert.ok(await OS.File.exists(downloadDir));
await OS.File.removeEmptyDir(tempDir.path);
Assert.ok(await IOUtils.exists(downloadDir));
await IOUtils.remove(tempDir.path);
// Should return the system downloads directory beacause the path is invalid
// in the dir preference.