Bug 924858 - Part 1: Add |append| mode flag to OS.File.open on Unix. r=yoric

To match previous behavior, this mode flag will default to |true|.

toolkit/components/osfile/modules/osfile_shared_front.jsm

@@ -304,29 +304,33 @@ AbstractFile.normalizeOpenMode = function normalizeOpenMode(mode) {
     write: false,
     trunc: false,
     create: false,
-    existing: false
+    existing: false,
+    append: true
   };
   for (let key in mode) {
-    if (!mode[key]) continue; // Only interpret true-ish keys
+    let val = !!mode[key]; // bool cast.
     switch (key) {
     case "read":
-      result.read = true;
+      result.read = val;
       break;
     case "write":
-      result.write = true;
+      result.write = val;
       break;
     case "truncate": // fallthrough
     case "trunc":
-      result.trunc = true;
-      result.write = true;
+      result.trunc = val;
+      result.write |= val;
       break;
     case "create":
-      result.create = true;
-      result.write = true;
+      result.create = val;
+      result.write |= val;
       break;
     case "existing": // fallthrough
     case "exist":
-      result.existing = true;
+      result.existing = val;
+      break;
+    case "append":
+      result.append = val;
       break;
     default:
       throw new TypeError("Mode " + key + " not understood");

toolkit/components/osfile/modules/osfile_unix_front.jsm

@@ -203,8 +203,11 @@
       *  on the other fields of |mode|.
       * - {bool} write If |true|, the file will be opened for
       *  writing. The file may also be opened for reading, depending
-      *  on the other fields of |mode|. If neither |truncate| nor
-      *  |create| is specified, the file is opened for appending.
+      *  on the other fields of |mode|.
+      * - {bool} append If |true|, the file will be opened for appending,
+      *  meaning the equivalent of |.setPosition(0, POS_END)| is executed
+      *  before each write. The default is |true|, i.e. opening a file for
+      *  appending. Specify |append: false| to open the file in regular mode.
       *
       * If neither |truncate|, |create| or |write| is specified, the file
       * is opened for reading.
@@ -251,12 +254,11 @@
            flags |= Const.O_CREAT | Const.O_EXCL;
          } else if (mode.read && !mode.write) {
            // flags are sufficient
-         } else /*append*/ {
-           if (mode.existing) {
-             flags |= Const.O_APPEND;
-           } else {
-             flags |= Const.O_APPEND | Const.O_CREAT;
-           }
+         } else if (!mode.existing) {
+           flags |= Const.O_CREAT;
+         }
+         if (mode.append) {
+           flags |= Const.O_APPEND;
          }
        }
        return error_or_file(UnixFile.open(path, flags, omode));
@@ -548,10 +550,12 @@
          let result;
          try {
            source = File.open(sourcePath);
+           // Need to open the output file with |append:false|, or else |splice|
+           // won't work.
            if (options.noOverwrite) {
-             dest = File.open(destPath, {create:true});
+             dest = File.open(destPath, {create:true, append:false});
            } else {
-             dest = File.open(destPath, {trunc:true});
+             dest = File.open(destPath, {trunc:true, append:false});
            }
            if (options.unixUserland) {
              result = pump_userland(source, dest, options);

toolkit/components/osfile/tests/xpcshell/test_osfile_async_append.js

@@ -0,0 +1,122 @@
+"use strict";
+
+do_print("starting tests");
+
+Components.utils.import("resource://gre/modules/osfile.jsm");
+Components.utils.import("resource://gre/modules/Task.jsm");
+
+/**
+ * A test to check that the |append| mode flag is correctly implemented.
+ * (see bug 925865)
+ */
+
+function setup_mode(mode) {
+  // Complete mode.
+  let realMode = {
+    read: true,
+    write: true
+  };
+  for (let k in mode) {
+    realMode[k] = mode[k];
+  }
+  return realMode;
+}
+
+// Test append mode.
+function test_append(mode) {
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                          "test_osfile_async_append.tmp");
+
+  // Clear any left-over files from previous runs.
+  try {
+    yield OS.File.remove(path);
+  } catch (ex if ex.becauseNoSuchFile) {
+    // ignore
+  }
+
+  try {
+    mode = setup_mode(mode);
+    mode.append = true;
+    if (mode.trunc) {
+      // Pre-fill file with some data to see if |trunc| actually works.
+      yield OS.File.writeAtomic(path, new Uint8Array(500));
+    }
+    let file = yield OS.File.open(path, mode);
+    try {
+      yield file.write(new Uint8Array(1000));
+      yield file.setPosition(0, OS.File.POS_START);
+      yield file.read(100);
+      // Should be at offset 100, length 1000 now.
+      yield file.write(new Uint8Array(100));
+      // Should be at offset 1100, length 1100 now.
+      let stat = yield file.stat();
+      do_check_eq(1100, stat.size);
+    } finally {
+      yield file.close();
+    }
+  } catch(ex) {
+    try {
+      yield OS.File.remove(path);
+    } catch (ex if ex.becauseNoSuchFile) {
+      // ignore.
+    }
+  }
+}
+
+// Test no-append mode.
+function test_no_append(mode) {
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                          "test_osfile_async_noappend.tmp");
+
+  // Clear any left-over files from previous runs.
+  try {
+    yield OS.File.remove(path);
+  } catch (ex if ex.becauseNoSuchFile) {
+    // ignore
+  }
+
+  try {
+    mode = setup_mode(mode);
+    mode.append = false;
+    if (mode.trunc) {
+      // Pre-fill file with some data to see if |trunc| actually works.
+      yield OS.File.writeAtomic(path, new Uint8Array(500));
+    }
+    let file = yield OS.File.open(path, mode);
+    try {
+      yield file.write(new Uint8Array(1000));
+      yield file.setPosition(0, OS.File.POS_START);
+      yield file.read(100);
+      // Should be at offset 100, length 1000 now.
+      yield file.write(new Uint8Array(100));
+      // Should be at offset 200, length 1000 now.
+      let stat = yield file.stat();
+      do_check_eq(1000, stat.size);
+    } finally {
+      yield file.close();
+    }
+  } finally {
+    try {
+      yield OS.File.remove(path);
+    } catch (ex if ex.becauseNoSuchFile) {
+      // ignore.
+    }
+  }
+}
+
+let test_flags = [
+  {},
+  {create:true},
+  {trunc:true}
+];
+function run_test() {
+  do_test_pending();
+
+  for (let t of test_flags) {
+    add_task(test_append.bind(null, t));
+    add_task(test_no_append.bind(null, t));
+  }
+  add_task(do_test_finished);
+
+  run_next_test();
+}

toolkit/components/osfile/tests/xpcshell/xpcshell.ini

@@ -5,6 +5,7 @@ tail =
 [test_osfile_closed.js]
 [test_path.js]
 [test_osfile_async.js]
+[test_osfile_async_append.js]
 [test_osfile_async_bytes.js]
 [test_osfile_async_copy.js]
 [test_profiledir.js]