Skip to content

Commit

Permalink
Backed out 2 changesets (bug 1689740, bug 1649611) for mochitest fail…
Browse files Browse the repository at this point in the history 
…ures on test_ioutils_mkdir.html

Backed out changeset 91181a01b68c (bug 1649611)
Backed out changeset 4dd71fe08a0e (bug 1689740)
  • Loading branch information
@nbeleuzu
nbeleuzu committed Jan 30, 2021
1 parent f5fafb4 commit 37d1a79
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 190 deletions.
6 changes: 1 addition & 5 deletions dom/chrome-webidl/IOUtils.webidl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,11 @@ 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, optional boolean honorUmask = true);
Promise<void> setPermissions(DOMString path, unsigned long permissions);
/**
* Return whether or not the file exists at the given path.
*
Expand Down
9 changes: 0 additions & 9 deletions dom/chrome-webidl/PathUtils.webidl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,6 @@ 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.
Expand Down
38 changes: 9 additions & 29 deletions dom/system/IOUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@
#include "prtime.h"
#include "prtypes.h"

#ifndef ANDROID
# include "nsSystemInfo.h"
#endif

#define REJECT_IF_SHUTTING_DOWN(aJSPromise) \
do { \
if (sShutdownStarted) { \
Expand Down Expand Up @@ -592,8 +588,7 @@ already_AddRefed<Promise> IOUtils::GetChildren(GlobalObject& aGlobal,
/* static */
already_AddRefed<Promise> IOUtils::SetPermissions(GlobalObject& aGlobal,
const nsAString& aPath,
uint32_t aPermissions,
const bool aHonorUmask) {
const uint32_t aPermissions) {
MOZ_ASSERT(XRE_IsParentProcess());
RefPtr<Promise> promise = CreateJSPromise(aGlobal);
if (!promise) {
Expand All @@ -603,12 +598,6 @@ 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);
Expand Down Expand Up @@ -1146,9 +1135,6 @@ Result<Ok, IOUtils::IOError> IOUtils::CopyOrMoveSync(CopyOrMoveFn aMethod,
MOZ_TRY(aDest->GetLeafName(destName));
MOZ_TRY(aDest->GetParent(getter_AddRefs(destDir)));

// We know `destName` is a file and therefore must have a parent directory.
MOZ_RELEASE_ASSERT(destDir);

// NB: if destDir doesn't exist, then |CopyToFollowingLinks| or
// |MoveToFollowingLinks| will create it.
rv = (aSource->*aMethod)(destDir, destName);
Expand Down Expand Up @@ -1203,21 +1189,15 @@ Result<Ok, IOUtils::IOError> IOUtils::MakeDirectorySync(nsIFile* aFile,
if (!aCreateAncestors) {
nsCOMPtr<nsIFile> parent;
MOZ_TRY(aFile->GetParent(getter_AddRefs(parent)));
if (parent) {
bool parentExists = false;
MOZ_TRY(parent->Exists(&parentExists));
if (!parentExists) {
return Err(IOError(NS_ERROR_FILE_NOT_FOUND)
.WithMessage("Could not create directory at %s because "
"the path has missing "
"ancestor components",
aFile->HumanReadablePath().get()));
}
bool parentExists = false;
MOZ_TRY(parent->Exists(&parentExists));
if (!parentExists) {
return Err(IOError(NS_ERROR_FILE_NOT_FOUND)
.WithMessage("Could not create directory at %s because "
"the path has missing "
"ancestor components",
aFile->HumanReadablePath().get()));
}
// If we don't have a parent directory, which means this was called with a
// root directory. If the directory doesn't already exist (e.g., asking for
// a drive on Windows that does not exist), we will not be able to create
// it. We can fall through here and let the Create() call handle this.
}

nsresult rv = aFile->Create(nsIFile::DIRECTORY_TYPE, aMode);
Expand Down
3 changes: 1 addition & 2 deletions dom/system/IOUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ class IOUtils final {

static already_AddRefed<Promise> SetPermissions(GlobalObject& aGlobal,
const nsAString& aPath,
uint32_t aPermissions,
const bool aHonorUmask);
const uint32_t aPermissions);

static already_AddRefed<Promise> Exists(GlobalObject& aGlobal,
const nsAString& aPath);
Expand Down
34 changes: 0 additions & 34 deletions dom/system/PathUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,40 +217,6 @@ 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()) {
Expand Down
3 changes: 0 additions & 3 deletions dom/system/PathUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ 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);

Expand Down
26 changes: 0 additions & 26 deletions dom/system/tests/ioutils/test_ioutils_mkdir.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,32 +101,6 @@

await cleanup(newDir);
});

add_task(async function test_make_directory_root() {
if (Services.appinfo.OS === "WINNT") {
// We don't actually know the root drive, but we can find the root drive
// of the profile directory.
const profileDir = await PathUtils.getProfileDir();
let current = profileDir;
let parent = PathUtils.parent(current);
while (parent !== null) {
current = parent;
parent = PathUtils.parent(current);
}
// `current` will now be a valid root directory.
ok(await IOUtils.exists(current), "Root directory should exist");

const DRIVE_RE = /^[A-Z]:$/;
ok(
current.startsWith("\\\\") || DRIVE_RE.test(current),
`Root directory (${current}) should be a UNC path or drive`,
);
await IOUtils.makeDirectory(current, {createAncestors: false});
} else {
ok(await IOUtils.exists("/"), "Root directory should exist");
await IOUtils.makeDirectory("/", {createAncestors: false});
}
});
</script>
</head>

Expand Down
32 changes: 0 additions & 32 deletions dom/system/tests/ioutils/test_ioutils_set_permissions.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,38 +23,6 @@

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");
Expand Down
88 changes: 52 additions & 36 deletions toolkit/components/downloads/DownloadIntegration.jsm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ ChromeUtils.defineModuleGetter(
"NetUtil",
"resource://gre/modules/NetUtil.jsm"
);
ChromeUtils.defineModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm");
ChromeUtils.defineModuleGetter(
this,
"PlacesUtils",
Expand Down Expand Up @@ -263,7 +264,7 @@ var DownloadIntegration = {

this._store = new DownloadStore(
list,
PathUtils.join(await PathUtils.getProfileDir(), "downloads.json")
OS.Path.join(OS.Constants.Path.profileDir, "downloads.json")
);
this._store.onsaveitem = this.shouldPersistDownload.bind(this);

Expand Down Expand Up @@ -370,9 +371,7 @@ var DownloadIntegration = {
Ci.nsIFile
);
directoryPath = directory.path;
await IOUtils.makeDirectory(directoryPath, {
createAncestors: false,
});
await OS.File.makeDir(directoryPath, { ignoreExisting: true });
} catch (ex) {
// Either the preference isn't set or the directory cannot be created.
directoryPath = await this.getSystemDownloadsDirectory();
Expand Down Expand Up @@ -498,7 +497,7 @@ var DownloadIntegration = {
referrerInfo: aDownload.source.referrerInfo,
fileSize: aDownload.currentBytes,
sha256Hash: hash,
suggestedFileName: PathUtils.filename(aDownload.target.path),
suggestedFileName: OS.Path.basename(aDownload.target.path),
signatureInfo: sigInfo,
redirects: channelRedirects,
},
Expand Down Expand Up @@ -605,31 +604,41 @@ var DownloadIntegration = {
// whatever reason.
zone = Ci.mozIDownloadPlatform.ZONE_INTERNET;
}
// 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
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 }
);
} catch (ex) {
// If writing to the file fails, we ignore the error and continue.
if (!(ex instanceof DOMException)) {
Cu.reportError(ex);
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();
}
}
} 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);
}
}
}

Expand All @@ -651,19 +660,26 @@ var DownloadIntegration = {
));
// Permanently downloaded files are made accessible by other users on
// this system, while temporary downloads are marked as read-only.
let unixMode;
let options = {};
if (isTemporaryDownload) {
unixMode = 0o400;
options.unixMode = 0o400;
options.winAttributes = { readOnly: true };
} else {
unixMode = 0o666;
options.unixMode = 0o666;
}
// On Unix, the umask of the process is respected.
await IOUtils.setPermissions(aDownload.target.path, unixMode);
await OS.File.setPermissions(aDownload.target.path, options);
} 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.
if (!(ex instanceof DOMException)) {
// 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
) {
Cu.reportError(ex);
}
}
Expand Down Expand Up @@ -942,15 +958,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 = PathUtils.join(
let directoryPath = OS.Path.join(
this._getDirectory(aName),
DownloadUIHelper.strings.downloadsFolder
);

// Create the Downloads folder and ignore if it already exists.
return IOUtils.makeDirectory(directoryPath, {
createAncestors: false,
}).then(() => directoryPath);
return OS.File.makeDir(directoryPath, { ignoreExisting: true }).then(
() => directoryPath
);
},

/**
Expand Down
Loading

0 comments on commit 37d1a79

Please sign in to comment.