Bug 1001849: expose general chmod, and chmod-to-umask, support in OS.File. r=yoric
authorZack Weinberg <zackw@panix.com>
Fri, 20 Jun 2014 19:54:06 -0400
changeset 190016 a5714e1eafca8447ffe7e478974f97610921380a
parent 190015 ccdc4a98cc11e9723b789bd578b8ec88d9142829
child 190017 c6426f08a82875f128b25aa1c43624b4e99ecf0f
push id8288
push userryanvm@gmail.com
push dateMon, 23 Jun 2014 14:59:00 +0000
treeherderb2g-inbound@c65bf5a0595c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyoric
bugs1001849
milestone33.0a1
Bug 1001849: expose general chmod, and chmod-to-umask, support in OS.File. r=yoric
toolkit/components/osfile/modules/osfile_async_front.jsm
toolkit/components/osfile/modules/osfile_async_worker.js
toolkit/components/osfile/modules/osfile_unix_back.jsm
toolkit/components/osfile/modules/osfile_unix_front.jsm
toolkit/components/osfile/modules/osfile_win_front.jsm
toolkit/components/osfile/tests/xpcshell/test_osfile_async_setPerms.js
toolkit/components/osfile/tests/xpcshell/xpcshell.ini
--- a/toolkit/components/osfile/modules/osfile_async_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_async_front.jsm
@@ -830,16 +830,43 @@ File.prototype = {
    * main-thread, it might still affect the overall performance of any running
    * application.
    *
    * @return {promise}
    */
   flush: function flush() {
     return Scheduler.post("File_prototype_flush",
       [this._fdmsg]);
+  },
+
+  /**
+   * Set the file's access permissions.  Without any options, the
+   * permissions are set to an approximation of what they would have
+   * been if the file had been created in its current directory in the
+   * "most typical" fashion for the operating system.  In the current
+   * implementation, this means that on Unix-like systems (including
+   * Android, B2G, etc) we set the POSIX file mode to (0666 & ~umask),
+   * and on Windows, we do nothing.
+   *
+   * This operation is likely to fail if applied to a file that was
+   * not created by the currently running program (more precisely,
+   * if it was created by a program running under a different OS-level
+   * user account).  It may also fail, or silently do nothing, if the
+   * filesystem containing the file does not support access permissions.
+   *
+   * @param {*=} options
+   * - {number} unixMode     If present, the POSIX file mode is set to exactly
+   *                         this value, unless |unixHonorUmask| is also
+   *                         present.
+   * - {bool} unixHonorUmask If true, any |unixMode| value is modified by the
+   *                         process umask, as open() would have done.
+   */
+  setPermissions: function setPermissions(options = {}) {
+    return Scheduler.post("File_prototype_setPermissions",
+                          [this._fdmsg, options]);
   }
 };
 
 
 if (SharedAll.Constants.Sys.Name != "Android") {
   /**
    * Set the last access and modification date of the file.
    * The time stamp resolution is 1 second at best, but might be worse
@@ -931,16 +958,45 @@ File.stat = function stat(path, options)
  */
 File.setDates = function setDates(path, accessDate, modificationDate) {
   return Scheduler.post("setDates",
                         [Type.path.toMsg(path), accessDate, modificationDate],
                         this);
 };
 
 /**
+ * Set the file's access permissions.  Without any options, the
+ * permissions are set to an approximation of what they would have
+ * been if the file had been created in its current directory in the
+ * "most typical" fashion for the operating system.  In the current
+ * implementation, this means that on Unix-like systems (including
+ * Android, B2G, etc) we set the POSIX file mode to (0666 & ~umask),
+ * and on Windows, we do nothing.
+ *
+ * This operation is likely to fail if applied to a file that was
+ * not created by the currently running program (more precisely,
+ * if it was created by a program running under a different OS-level
+ * user account).  It may also fail, or silently do nothing, if the
+ * filesystem containing the file does not support access permissions.
+ *
+ * @param {string} path   The path to the file.
+ *
+ * @param {*=} options
+ * - {number} unixMode     If present, the POSIX file mode is set to exactly
+ *                         this value, unless |unixHonorUmask| is also
+ *                         present.
+ * - {bool} unixHonorUmask If true, any |unixMode| value is modified by the
+ *                         process umask, as open() would have done.
+ */
+File.setPermissions = function setPermissions(path, options = {}) {
+  return Scheduler.post("setPermissions",
+                        [Type.path.toMsg(path), options]);
+};
+
+/**
  * Fetch the current directory
  *
  * @return {promise}
  * @resolves {string} The current directory, as a path usable with OS.Path
  * @rejects {OS.Error}
  */
 File.getCurrentDirectory = function getCurrentDirectory() {
   return Scheduler.post(
--- a/toolkit/components/osfile/modules/osfile_async_worker.js
+++ b/toolkit/components/osfile/modules/osfile_async_worker.js
@@ -292,16 +292,19 @@ const EXCEPTION_NAMES = {
 
      return new Meta(result, {shutdown: result.killed});
    },
    // Functions of OS.File
    stat: function stat(path, options) {
      return exports.OS.File.Info.toMsg(
        exports.OS.File.stat(Type.path.fromMsg(path), options));
    },
+   setPermissions: function setPermissions(path, options = {}) {
+     return exports.OS.File.setPermissions(Type.path.fromMsg(path), options);
+   },
    setDates: function setDates(path, accessDate, modificationDate) {
      return exports.OS.File.setDates(Type.path.fromMsg(path), accessDate,
                                      modificationDate);
    },
    getCurrentDirectory: function getCurrentDirectory() {
      return exports.OS.Shared.Type.path.toMsg(File.getCurrentDirectory());
    },
    setCurrentDirectory: function setCurrentDirectory(path) {
@@ -400,16 +403,22 @@ const EXCEPTION_NAMES = {
      });
    },
    File_prototype_stat: function stat(fd) {
      return withFile(fd,
        function do_stat() {
          return exports.OS.File.Info.toMsg(this.stat());
        });
    },
+   File_prototype_setPermissions: function setPermissions(fd, options = {}) {
+     return withFile(fd,
+       function do_setPermissions() {
+         return this.setPermissions(options);
+       });
+   },
    File_prototype_setDates: function setDates(fd, accessTime, modificationTime) {
      return withFile(fd,
        function do_setDates() {
          return this.setDates(accessTime, modificationTime);
        });
    },
    File_prototype_read: function read(fd, nbytes, options) {
      return withFile(fd,
--- a/toolkit/components/osfile/modules/osfile_unix_back.jsm
+++ b/toolkit/components/osfile/modules/osfile_unix_back.jsm
@@ -343,16 +343,22 @@
                            /*return*/ Type.negativeone_or_nothing,
                            /*path*/   Type.path);
 
        libc.declareLazyFFI(SysFile,  "fchdir",
                            "fchdir", ctypes.default_abi,
                            /*return*/ Type.negativeone_or_nothing,
                            /*fd*/     Type.fd);
 
+       libc.declareLazyFFI(SysFile,  "fchmod",
+                           "fchmod", ctypes.default_abi,
+                           /*return*/ Type.negativeone_or_nothing,
+                           /*fd*/     Type.fd,
+                           /*mode*/   Type.mode_t);
+
        libc.declareLazyFFI(SysFile,  "fchown",
                            "fchown", ctypes.default_abi,
                            /*return*/ Type.negativeone_or_nothing,
                            /*fd*/     Type.fd,
                            /*uid_t*/  Type.uid_t,
                            /*gid_t*/  Type.gid_t);
 
        libc.declareLazyFFI(SysFile,  "fsync",
--- a/toolkit/components/osfile/modules/osfile_unix_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_unix_front.jsm
@@ -169,16 +169,43 @@
       */
      File.prototype.stat = function stat() {
        throw_on_negative("stat", UnixFile.fstat(this.fd, gStatDataPtr),
                          this._path);
        return new File.Info(gStatData, this._path);
      };
 
      /**
+      * Set the file's access permissions.  Without any options, the
+      * permissions are set to an approximation of what they would
+      * have been if the file had been created in its current
+      * directory in the "most typical" fashion for the operating
+      * system.  In the current implementation, this means we set
+      * the POSIX file mode to (0666 & ~umask).
+      *
+      * This operation is likely to fail if applied to a file that was
+      * not created by the currently running program (more precisely,
+      * if it was created by a program running under a different OS-level
+      * user account).  It may also fail, or silently do nothing, if the
+      * filesystem containing the file does not support access permissions.
+      *
+      * @param {*=} options
+      * - {number} unixMode     If present, the POSIX file mode is set to
+      *                         exactly this value, unless |unixHonorUmask| is
+      *                         also present.
+      * - {bool} unixHonorUmask If true, any |unixMode| value is modified by
+      *                         the process umask, as open() would have done.
+      */
+     File.prototype.setPermissions = function setPermissions(options = {}) {
+       throw_on_negative("setPermissions",
+                         UnixFile.fchmod(this.fd, unixMode(options)),
+                         this._path);
+     };
+
+     /**
       * Set the last access and modification date of the file.
       * The time stamp resolution is 1 second at best, but might be worse
       * depending on the platform.
       *
       * WARNING: This method is not implemented on Android/B2G. On Android/B2G,
       * you should use File.setDates instead.
       *
       * @param {Date,number=} accessDate The last access date. If numeric,
@@ -905,16 +932,44 @@
          throw_on_negative("stat", UnixFile.lstat(path, gStatDataPtr), path);
        } else {
          throw_on_negative("stat", UnixFile.stat(path, gStatDataPtr), path);
        }
        return new File.Info(gStatData, path);
      };
 
      /**
+      * Set the file's access permissions.  Without any options, the
+      * permissions are set to an approximation of what they would
+      * have been if the file had been created in its current
+      * directory in the "most typical" fashion for the operating
+      * system.  In the current implementation, this means we set
+      * the POSIX file mode to (0666 & ~umask).
+      *
+      * This operation is likely to fail if applied to a file that was
+      * not created by the currently running program (more precisely,
+      * if it was created by a program running under a different OS-level
+      * user account).  It may also fail, or silently do nothing, if the
+      * filesystem containing the file does not support access permissions.
+      *
+      * @param {string} path   The name of the file to reset the permissions of.
+      * @param {*=} options
+      * - {number} unixMode     If present, the POSIX file mode is set to
+      *                         exactly this value, unless |unixHonorUmask| is
+      *                         also present.
+      * - {bool} unixHonorUmask If true, any |unixMode| value is modified by
+      *                         the process umask, as open() would have done.
+      */
+     File.setPermissions = function setPermissions(path, options = {}) {
+       throw_on_negative("setPermissions",
+                         UnixFile.chmod(path, unixMode(options)),
+                         path);
+     };
+
+     /**
       * Convert an access date and a modification date to an array
       * of two |timeval|.
       */
      function datesToTimevals(accessDate, modificationDate) {
        accessDate = normalizeDate("File.setDates", accessDate);
        modificationDate = normalizeDate("File.setDates", modificationDate);
 
        let timevals = new Type.timevals.implementation();
@@ -1106,16 +1161,35 @@
 
        if (isNaN(date)) {
          throw new TypeError("|date| parameter of " + fn + " must be a " +
                              "|Date| instance or number");
        }
        return date;
      };
 
+     /**
+      * Helper used by both versions of setPermissions.
+      */
+     function unixMode(options) {
+       let mode = 438; /* 0666 */
+       let unixHonorUmask = true;
+       if ("unixMode" in options) {
+         unixHonorUmask = false;
+         mode = options.unixMode;
+       }
+       if ("unixHonorUmask" in options) {
+         unixHonorUmask = options.unixHonorUmask;
+       }
+       if (unixHonorUmask) {
+         mode &= ~SharedAll.Constants.Sys.umask;
+       }
+       return mode;
+     }
+
      File.Unix = exports.OS.Unix.File;
      File.Error = SysAll.Error;
      exports.OS.File = File;
      exports.OS.Shared.Type = Type;
 
      Object.defineProperty(File, "POS_START", { value: SysAll.POS_START });
      Object.defineProperty(File, "POS_CURRENT", { value: SysAll.POS_CURRENT });
      Object.defineProperty(File, "POS_END", { value: SysAll.POS_END });
--- a/toolkit/components/osfile/modules/osfile_win_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_win_front.jsm
@@ -230,16 +230,24 @@
                                            this._path);
        throw_on_zero("setDates",
                      WinFile.SetFileTime(this.fd, null, accessDate.address(),
                                          modificationDate.address()),
                      this._path);
      };
 
      /**
+      * Set the file's access permission bits.
+      * Not implemented for Windows (bug 1022816).
+      */
+     File.prototype.setPermissions = function setPermissions(options = {}) {
+         // do nothing
+     };
+
+     /**
       * Flushes the file's buffers and causes all buffered data
       * to be written.
       * Disk flushes are very expensive and therefore should be used carefully,
       * sparingly and only in scenarios where it is vital that data survives
       * system crashes. Even though the function will be executed off the
       * main-thread, it might still affect the overall performance of any
       * running application.
       *
@@ -949,16 +957,24 @@
        // Directories can be opened neither for reading(!) nor for writing
        winAccess: 0,
        // Directories can only be opened with backup semantics(!)
        winFlags: Const.FILE_FLAG_BACKUP_SEMANTICS,
        winDisposition: Const.OPEN_EXISTING
      };
 
      /**
+      * Set the file's access permission bits.
+      * Not implemented for Windows (bug 1022816).
+      */
+     File.setPermissions = function setPermissions(path, options = {}) {
+         // do nothing
+     };
+
+     /**
       * Set the last access and modification date of the file.
       * The time stamp resolution is 1 second at best, but might be worse
       * depending on the platform.
       *
       * Performance note: if you have opened the file already in write mode,
       * method |File.prototype.stat| is generally much faster
       * than method |File.stat|.
       *
new file mode 100644
--- /dev/null
+++ b/toolkit/components/osfile/tests/xpcshell/test_osfile_async_setPerms.js
@@ -0,0 +1,123 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * A test to ensure that OS.File.setPermissions and
+ * OS.File.prototype.setPermissions are all working correctly.
+ * (see bug 1001849)
+ * These functions are currently Unix-specific.  The manifest skips
+ * the test on Windows.
+ */
+
+/**
+ * Helper function for test logging: prints a POSIX file permission mode as an
+ * octal number, with a leading '0' per C (not JS) convention.  When the
+ * numeric value is 0777 or lower, it is padded on the left with zeroes to
+ * four digits wide.
+ * Sample outputs:  0022, 0644, 04755.
+ */
+function format_mode(mode) {
+  if (mode <= 0o777) {
+    return ("0000" + mode.toString(8)).slice(-4);
+  } else {
+    return "0" + mode.toString(8);
+  }
+}
+
+/**
+ * Use this function to compare two mode values; it prints both values as
+ * octal numbers in the log.
+ */
+function do_check_modes_eq(left, right, text) {
+  text = text + ": " + format_mode(left) + " === " + format_mode(right);
+  do_report_result(left === right, text, Components.stack.caller, false);
+}
+
+const _umask = OS.Constants.Sys.umask;
+do_print("umask: " + format_mode(_umask));
+
+/**
+ * Compute the mode that a file should have after applying the umask,
+ * whatever it happens to be.
+ */
+function apply_umask(mode) {
+  return mode & ~_umask;
+}
+
+// Test application to paths.
+add_task(function*() {
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                          "test_osfile_async_setPerms_nonproto.tmp");
+  yield OS.File.writeAtomic(path, new Uint8Array(1));
+
+  try {
+    let stat;
+
+    yield OS.File.setPermissions(path, {unixMode: 0o4777});
+    stat = yield OS.File.stat(path);
+    do_check_modes_eq(stat.unixMode, 0o4777,
+                      "setPermissions(path, 04777)");
+
+    yield OS.File.setPermissions(path, {unixMode: 0o4777,
+                                        unixHonorUmask: true});
+    stat = yield OS.File.stat(path);
+    do_check_modes_eq(stat.unixMode, apply_umask(0o4777),
+                      "setPermissions(path, 04777&~umask)");
+
+    yield OS.File.setPermissions(path);
+    stat = yield OS.File.stat(path);
+    do_check_modes_eq(stat.unixMode, apply_umask(0o666),
+                      "setPermissions(path, {})");
+
+    yield OS.File.setPermissions(path, {unixMode: 0});
+    stat = yield OS.File.stat(path);
+    do_check_modes_eq(stat.unixMode, 0,
+                      "setPermissions(path, 0000)");
+
+  } finally {
+    yield OS.File.remove(path);
+  }
+});
+
+// Test application to open files.
+add_task(function*() {
+  // First, create a file we can mess with.
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                              "test_osfile_async_setDates_proto.tmp");
+  yield OS.File.writeAtomic(path, new Uint8Array(1));
+
+  try {
+    let fd = yield OS.File.open(path, {write: true});
+    let stat;
+
+    yield fd.setPermissions({unixMode: 0o4777});
+    stat = yield fd.stat();
+    do_check_modes_eq(stat.unixMode, 0o4777,
+                      "fd.setPermissions(04777)");
+
+    yield fd.setPermissions({unixMode: 0o4777, unixHonorUmask: true});
+    stat = yield fd.stat();
+    do_check_modes_eq(stat.unixMode, apply_umask(0o4777),
+                      "fd.setPermissions(04777&~umask)");
+
+    yield fd.setPermissions();
+    stat = yield fd.stat();
+    do_check_modes_eq(stat.unixMode, apply_umask(0o666),
+                      "fd.setPermissions({})");
+
+    yield fd.setPermissions({unixMode: 0});
+    stat = yield fd.stat();
+    do_check_modes_eq(stat.unixMode, 0,
+                      "fd.setPermissions(0000)");
+
+    yield fd.close();
+  } finally {
+    yield OS.File.remove(path);
+  }
+});
+
+function run_test() {
+  run_next_test();
+}
--- a/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
@@ -31,8 +31,14 @@ support-files =
 [test_duration.js]
 [test_read_write.js]
 [test_compression.js]
 [test_osfile_writeAtomic_backupTo_option.js]
 [test_osfile_error.js]
 [test_queue.js]
 [test_loader.js]
 [test_constants.js]
+
+# Unimplemented on Windows (bug 1022816).
+# Spurious failure on Android test farm due to non-POSIX behavior of
+# filesystem backing /mnt/sdcard (not worth trying to fix).
+[test_osfile_async_setPerms.js]
+skip-if = os == "win" || os == "android"