bug 1280044 - handle subdirs in browser.downloads filenames, r?aswan
MozReview-Commit-ID: B4WoMbdxYjV
--- a/toolkit/components/extensions/ext-downloads.js
+++ b/toolkit/components/extensions/ext-downloads.js
@@ -18,16 +18,17 @@ XPCOMUtils.defineLazyModuleGetter(this,
"resource://devtools/shared/event-emitter.js");
Cu.import("resource://gre/modules/ExtensionUtils.jsm");
const {
ignoreEvent,
normalizeTime,
runSafeSync,
SingletonEventManager,
+ PlatformInfo,
} = ExtensionUtils;
const DOWNLOAD_ITEM_FIELDS = ["id", "url", "referrer", "filename", "incognito",
"danger", "mime", "startTime", "endTime",
"estimatedEndTime", "state",
"paused", "canResume", "error",
"bytesReceived", "totalBytes",
"fileSize", "exists",
@@ -386,22 +387,28 @@ function queryHelper(query) {
});
}
extensions.registerSchemaAPI("downloads", "addon_parent", context => {
let {extension} = context;
return {
downloads: {
download(options) {
- if (options.filename != null) {
- if (options.filename.length == 0) {
+ let {filename} = options;
+ if (filename && PlatformInfo.os === "win") {
+ // cross platform javascript code uses "/"
+ filename = filename.replace(/\//g, "\\");
+ }
+
+ if (filename != null) {
+ if (filename.length == 0) {
return Promise.reject({message: "filename must not be empty"});
}
- let path = OS.Path.split(options.filename);
+ let path = OS.Path.split(filename);
if (path.absolute) {
return Promise.reject({message: "filename must not be an absolute path"});
}
if (path.components.some(component => component == "..")) {
return Promise.reject({message: "filename must not contain back-references (..)"});
}
}
@@ -411,33 +418,37 @@ extensions.registerSchemaAPI("downloads"
return Promise.reject({message: "conflictAction prompt not yet implemented"});
}
function createTarget(downloadsDir) {
// TODO
// if (options.saveAs) { }
let target;
- if (options.filename) {
- target = OS.Path.join(downloadsDir, options.filename);
+ if (filename) {
+ target = OS.Path.join(downloadsDir, filename);
} else {
let uri = NetUtil.newURI(options.url);
- let filename;
+ let remote = "download";
if (uri instanceof Ci.nsIURL) {
- filename = uri.fileName;
+ remote = uri.fileName;
}
- target = OS.Path.join(downloadsDir, filename || "download");
+ target = OS.Path.join(downloadsDir, remote);
}
- // This has a race, something else could come along and create
- // the file between this test and them time the download code
- // creates the target file. But we can't easily fix it without
- // modifying DownloadCore so we live with it for now.
- return OS.File.exists(target).then(exists => {
+ // Create any needed subdirectories if required by filename.
+ const dir = OS.Path.dirname(target);
+ return OS.File.makeDir(dir, {from: downloadsDir}).then(() => {
+ return OS.File.exists(target);
+ }).then(exists => {
+ // This has a race, something else could come along and create
+ // the file between this test and them time the download code
+ // creates the target file. But we can't easily fix it without
+ // modifying DownloadCore so we live with it for now.
if (exists) {
switch (options.conflictAction) {
case "uniquify":
default:
target = DownloadPaths.createNiceUniqueFile(new FileUtils.File(target)).path;
break;
case "overwrite":
--- a/toolkit/components/extensions/test/xpcshell/test_ext_downloads_download.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_downloads_download.js
@@ -89,20 +89,20 @@ function waitForDownloads() {
// Create a file in the downloads directory.
function touch(filename) {
let file = downloadDir.clone();
file.append(filename);
file.create(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE);
}
// Remove a file in the downloads directory.
-function remove(filename) {
+function remove(filename, recursive = false) {
let file = downloadDir.clone();
file.append(filename);
- file.remove(false);
+ file.remove(recursive);
}
add_task(function* test_downloads() {
setup();
let extension = ExtensionTestUtils.loadExtension({
background: `(${backgroundScript})()`,
manifest: {
@@ -116,17 +116,18 @@ add_task(function* test_downloads() {
}
function testDownload(options, localFile, expectedSize, description) {
return download(options).then(msg => {
equal(msg.status, "success", `downloads.download() works with ${description}`);
return waitForDownloads();
}).then(() => {
let localPath = downloadDir.clone();
- localPath.append(localFile);
+ let parts = Array.isArray(localFile) ? localFile : [localFile];
+ parts.map(p => localPath.append(p));
equal(localPath.fileSize, expectedSize, "Downloaded file has expected size");
localPath.remove(false);
});
}
yield extension.startup();
yield extension.awaitMessage("ready");
do_print("extension started");
@@ -135,16 +136,45 @@ add_task(function* test_downloads() {
yield testDownload({url: FILE_URL}, FILE_NAME, FILE_LEN, "just source");
// Call download() with a filename property.
yield testDownload({
url: FILE_URL,
filename: "newpath.txt",
}, "newpath.txt", FILE_LEN, "source and filename");
+ // Call download() with a filename with subdirs.
+ yield testDownload({
+ url: FILE_URL,
+ filename: "sub/dir/file",
+ }, ["sub", "dir", "file"], FILE_LEN, "source and filename with subdirs");
+
+ // Call download() with a filename with existing subdirs.
+ yield testDownload({
+ url: FILE_URL,
+ filename: "sub/dir/file2",
+ }, ["sub", "dir", "file2"], FILE_LEN, "source and filename with existing subdirs");
+
+ // Only run Windows path separator test on Windows.
+ if (WINDOWS) {
+ // Call download() with a filename with Windows path separator.
+ yield testDownload({
+ url: FILE_URL,
+ filename: "sub\\dir\\file3",
+ }, ["sub", "dir", "file3"], FILE_LEN, "filename with Windows path separator");
+ }
+ remove("sub", true);
+
+ // Call download(), filename with subdir, skipping parts.
+ yield testDownload({
+ url: FILE_URL,
+ filename: "skip//part",
+ }, ["skip", "part"], FILE_LEN, "source, filename, with subdir, skipping parts");
+ remove("skip", true);
+
// Check conflictAction of "uniquify".
touch(FILE_NAME);
yield testDownload({
url: FILE_URL,
conflictAction: "uniquify",
}, FILE_NAME_UNIQUE, FILE_LEN, "conflictAction=uniquify");
// todo check that preexisting file was not modified?
remove(FILE_NAME);