зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1649611: Remove OS.File usage from DownloadIntegration.jsm and respect umask in IOUtils::SetPermissions r=barret
Depends on D103520 Differential Revision: https://phabricator.services.mozilla.com/D99729
This commit is contained in:
Родитель
8ba96a2ae5
Коммит
e13166a7b1
|
@ -187,11 +187,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.
|
||||
*
|
||||
|
|
|
@ -58,6 +58,15 @@ namespace PathUtils {
|
|||
[Throws]
|
||||
DOMString createUniquePath(DOMString path);
|
||||
|
||||
/**
|
||||
* Creates an adjusted path using a path whose length is already close
|
||||
* to MAX_PATH. For windows only.
|
||||
*
|
||||
* @param path An absolute path.
|
||||
*/
|
||||
[Throws]
|
||||
DOMString toExtendedWindowsPath(DOMString path);
|
||||
|
||||
/**
|
||||
* Normalize a path by removing multiple separators and `..` and `.`
|
||||
* directories.
|
||||
|
|
|
@ -47,6 +47,10 @@
|
|||
#include "prtime.h"
|
||||
#include "prtypes.h"
|
||||
|
||||
#ifndef ANDROID
|
||||
# include "nsSystemInfo.h"
|
||||
#endif
|
||||
|
||||
#define REJECT_IF_SHUTTING_DOWN(aJSPromise) \
|
||||
do { \
|
||||
if (sShutdownStarted) { \
|
||||
|
@ -588,7 +592,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);
|
||||
if (!promise) {
|
||||
|
@ -598,6 +603,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
|
||||
|
||||
RunOnBackgroundThreadAndResolve<Ok>(
|
||||
promise, [file = std::move(file), permissions = aPermissions]() {
|
||||
return SetPermissionsSync(file, permissions);
|
||||
|
|
|
@ -113,7 +113,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);
|
||||
|
|
|
@ -217,6 +217,40 @@ void PathUtils::CreateUniquePath(const GlobalObject&, const nsAString& aPath,
|
|||
MOZ_ALWAYS_SUCCEEDS(path->GetPath(aResult));
|
||||
}
|
||||
|
||||
void PathUtils::ToExtendedWindowsPath(const GlobalObject&,
|
||||
const nsAString& aPath, nsString& aResult,
|
||||
ErrorResult& aErr) {
|
||||
#ifndef XP_WIN
|
||||
aErr.ThrowNotAllowedError("Operation is windows specific"_ns);
|
||||
return;
|
||||
#else
|
||||
if (aPath.IsEmpty()) {
|
||||
aErr.ThrowNotAllowedError(ERROR_EMPTY_PATH);
|
||||
return;
|
||||
}
|
||||
|
||||
const nsAString& str1 = Substring(aPath, 1, 1);
|
||||
const nsAString& str2 = Substring(aPath, 2, aPath.Length() - 2);
|
||||
|
||||
bool isUNC = aPath.Length() >= 2 &&
|
||||
(aPath.First() == '\\' || aPath.First() == '/') &&
|
||||
(str1.EqualsLiteral("\\") || str1.EqualsLiteral("/"));
|
||||
|
||||
constexpr auto pathPrefix = u"\\\\?\\"_ns;
|
||||
const nsAString& uncPath = pathPrefix + u"UNC\\"_ns + str2;
|
||||
const nsAString& normalPath = pathPrefix + aPath;
|
||||
|
||||
nsCOMPtr<nsIFile> path = new nsLocalFile();
|
||||
if (nsresult rv = path->InitWithPath(isUNC ? uncPath : normalPath);
|
||||
NS_FAILED(rv)) {
|
||||
ThrowError(aErr, rv, ERROR_INITIALIZE_PATH);
|
||||
return;
|
||||
}
|
||||
|
||||
MOZ_ALWAYS_SUCCEEDS(path->GetPath(aResult));
|
||||
#endif
|
||||
}
|
||||
|
||||
void PathUtils::Normalize(const GlobalObject&, const nsAString& aPath,
|
||||
nsString& aResult, ErrorResult& aErr) {
|
||||
if (aPath.IsEmpty()) {
|
||||
|
|
|
@ -39,6 +39,9 @@ class PathUtils final {
|
|||
static void CreateUniquePath(const GlobalObject&, const nsAString& aPath,
|
||||
nsString& aResult, ErrorResult& aErr);
|
||||
|
||||
static void ToExtendedWindowsPath(const GlobalObject&, const nsAString& aPath,
|
||||
nsString& aResult, ErrorResult& aErr);
|
||||
|
||||
static void Normalize(const GlobalObject&, const nsAString& aPath,
|
||||
nsString& aResult, ErrorResult& aErr);
|
||||
|
||||
|
|
|
@ -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,30 @@ 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(
|
||||
PathUtils.toExtendedWindowsPath(path),
|
||||
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 +651,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 +942,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);
|
||||
},
|
||||
|
||||
/**
|
||||
|
|
|
@ -323,18 +323,18 @@ add_task(async function test_unix_permissions() {
|
|||
}
|
||||
|
||||
let isTemporary = launchWhenSucceeded && (autoDelete || isPrivate);
|
||||
let stat = await OS.File.stat(download.target.path);
|
||||
let stat = await IOUtils.stat(download.target.path);
|
||||
if (Services.appinfo.OS == "WINNT") {
|
||||
// On Windows
|
||||
// Temporary downloads should be read-only
|
||||
Assert.equal(stat.winAttributes.readOnly, !!isTemporary);
|
||||
Assert.equal(stat.permissions, isTemporary ? 0o444 : 0o666);
|
||||
} else {
|
||||
// On Linux, Mac
|
||||
// Temporary downloads should be read-only and not accessible to other
|
||||
// users, while permanently downloaded files should be readable and
|
||||
// writable as specified by the system umask.
|
||||
Assert.equal(
|
||||
stat.unixMode,
|
||||
stat.permissions,
|
||||
isTemporary ? 0o400 : 0o666 & ~OS.Constants.Sys.umask
|
||||
);
|
||||
}
|
||||
|
|
|
@ -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.
|
||||
|
|
Загрузка…
Ссылка в новой задаче