Bug 1022816 - OS.File will now be able to change the readOnly, hidden, and system file attributes on Windows. r=paolo, a=sledru
authorGanesh Sahukari <sahukariganesh2@gmail.com>
Fri, 13 Mar 2015 15:51:53 +0000
changeset 260258 8a2c933394da
parent 260257 c0624fb0b902
child 260259 7e818f6d12f4
child 260261 0efa961d5162
child 260372 8534deb8936b
child 260373 4392694fbfe5
push id729
push userryanvm@gmail.com
push date2015-04-23 18:28 +0000
treeherdermozilla-release@8a2c933394da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaolo, sledru
bugs1022816
milestone38.0
Bug 1022816 - OS.File will now be able to change the readOnly, hidden, and system file attributes on Windows. r=paolo, a=sledru
dom/system/OSFileConstants.cpp
toolkit/components/osfile/modules/osfile_win_allthreads.jsm
toolkit/components/osfile/modules/osfile_win_front.jsm
toolkit/components/osfile/tests/xpcshell/test_osfile_win_async_setPermissions.js
toolkit/components/osfile/tests/xpcshell/xpcshell.ini
--- a/dom/system/OSFileConstants.cpp
+++ b/dom/system/OSFileConstants.cpp
@@ -722,19 +722,21 @@ static const dom::ConstantSpec gWinPrope
   INT_CONSTANT(CREATE_NEW),
   INT_CONSTANT(OPEN_ALWAYS),
   INT_CONSTANT(OPEN_EXISTING),
   INT_CONSTANT(TRUNCATE_EXISTING),
 
   // CreateFile attributes
   INT_CONSTANT(FILE_ATTRIBUTE_ARCHIVE),
   INT_CONSTANT(FILE_ATTRIBUTE_DIRECTORY),
+  INT_CONSTANT(FILE_ATTRIBUTE_HIDDEN),
   INT_CONSTANT(FILE_ATTRIBUTE_NORMAL),
   INT_CONSTANT(FILE_ATTRIBUTE_READONLY),
   INT_CONSTANT(FILE_ATTRIBUTE_REPARSE_POINT),
+  INT_CONSTANT(FILE_ATTRIBUTE_SYSTEM),
   INT_CONSTANT(FILE_ATTRIBUTE_TEMPORARY),
   INT_CONSTANT(FILE_FLAG_BACKUP_SEMANTICS),
 
   // CreateFile error constant
   { "INVALID_HANDLE_VALUE", INT_TO_JSVAL(INT_PTR(INVALID_HANDLE_VALUE)) },
 
 
   // CreateFile flags
--- a/toolkit/components/osfile/modules/osfile_win_allthreads.jsm
+++ b/toolkit/components/osfile/modules/osfile_win_allthreads.jsm
@@ -205,24 +205,26 @@ exports.Error = OSError;
 
 /**
  * Code shared by implementation of File.Info on Windows
  *
  * @constructor
  */
 let AbstractInfo = function AbstractInfo(path, isDir, isSymLink, size,
                                          winBirthDate,
-                                         lastAccessDate, lastWriteDate) {
+                                         lastAccessDate, lastWriteDate,
+                                         winAttributes) {
   this._path = path;
   this._isDir = isDir;
   this._isSymLink = isSymLink;
   this._size = size;
   this._winBirthDate = winBirthDate;
   this._lastAccessDate = lastAccessDate;
   this._lastModificationDate = lastWriteDate;
+  this._winAttributes = winAttributes;
 };
 
 AbstractInfo.prototype = {
   /**
    * The path of the file, used for error-reporting.
    *
    * @type {string}
    */
@@ -280,16 +282,25 @@ AbstractInfo.prototype = {
    *
    * Note that the definition of last access may depend on the underlying
    * operating system and file system.
    *
    * @type {Date}
    */
   get lastModificationDate() {
     return this._lastModificationDate;
+  },
+  /**
+   * The Object with following boolean properties of this file.
+   * {readOnly, system, hidden}
+   *
+   * @type {object}
+   */
+  get winAttributes() {
+    return this._winAttributes;
   }
 };
 exports.AbstractInfo = AbstractInfo;
 
 /**
  * Code shared by implementation of File.DirectoryIterator.Entry on Windows
  *
  * @constructor
--- a/toolkit/components/osfile/modules/osfile_win_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_win_front.jsm
@@ -231,20 +231,29 @@
        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
+       if (!("winAttributes" in options)) {
+         return;
+       }
+       let oldAttributes = WinFile.GetFileAttributes(this._path);
+       if (oldAttributes == Const.INVALID_FILE_ATTRIBUTES) {
+         throw new File.Error("setPermissions", ctypes.winLastError, this._path);
+       }
+       let newAttributes = toFileAttributes(options.winAttributes, oldAttributes);
+       throw_on_zero("setPermissions",
+                     WinFile.SetFileAttributes(this._path, newAttributes),
+                     this._path);
      };
 
      /**
       * 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
@@ -899,19 +908,24 @@
        let isSymLink = !!(stat.dwFileAttributes & Const.FILE_ATTRIBUTE_REPARSE_POINT);
 
        let winBirthDate = FILETIME_to_Date(stat.ftCreationTime, this._path);
        let lastAccessDate = FILETIME_to_Date(stat.ftLastAccessTime, this._path);
        let lastWriteDate = FILETIME_to_Date(stat.ftLastWriteTime, this._path);
 
        let value = ctypes.UInt64.join(stat.nFileSizeHigh, stat.nFileSizeLow);
        let size = Type.uint64_t.importFromC(value);
+       let winAttributes = {
+         readOnly: !!(stat.dwFileAttributes & Const.FILE_ATTRIBUTE_READONLY),
+         system: !!(stat.dwFileAttributes & Const.FILE_ATTRIBUTE_SYSTEM),
+         hidden: !!(stat.dwFileAttributes & Const.FILE_ATTRIBUTE_HIDDEN),
+       };
 
        SysAll.AbstractInfo.call(this, path, isDir, isSymLink, size,
-         winBirthDate, lastAccessDate, lastWriteDate);
+         winBirthDate, lastAccessDate, lastWriteDate, winAttributes);
      };
      File.Info.prototype = Object.create(SysAll.AbstractInfo.prototype);
 
      /**
       * Return a version of an instance of File.Info that can be sent
       * from a worker thread to the main thread. Note that deserialization
       * is asymmetric and returns an object with a different implementation.
       */
@@ -958,20 +972,29 @@
        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
+       if (!("winAttributes" in options)) {
+         return;
+       }
+       let oldAttributes = WinFile.GetFileAttributes(path);
+       if (oldAttributes == Const.INVALID_FILE_ATTRIBUTES) {
+         throw new File.Error("setPermissions", ctypes.winLastError, path);
+       }
+       let newAttributes = toFileAttributes(options.winAttributes, oldAttributes);
+       throw_on_zero("setPermissions",
+                     WinFile.SetFileAttributes(path, newAttributes),
+                     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.
       *
       * Performance note: if you have opened the file already in write mode,
@@ -1170,16 +1193,44 @@
       */
      function throw_on_null(operation, result, path) {
        if (result == null || (result.isNull && result.isNull())) {
          throw new File.Error(operation, ctypes.winLastError, path);
        }
        return result;
      }
 
+     /**
+      * Helper used by both versions of setPermissions
+      */
+     function toFileAttributes(winAttributes, oldDwAttrs) {
+       if ("readOnly" in winAttributes) {
+         if (winAttributes.readOnly) {
+           oldDwAttrs |= Const.FILE_ATTRIBUTE_READONLY;
+         } else {
+           oldDwAttrs &= ~Const.FILE_ATTRIBUTE_READONLY;
+         }
+       }
+       if ("system" in winAttributes) {
+         if (winAttributes.system) {
+           oldDwAttrs |= Const.FILE_ATTRIBUTE_SYSTEM;
+         } else {
+           oldDwAttrs &= ~Const.FILE_ATTRIBUTE_SYSTEM;
+         }
+       }
+       if ("hidden" in winAttributes) {
+         if (winAttributes.hidden) {
+           oldDwAttrs |= Const.FILE_ATTRIBUTE_HIDDEN;
+         } else {
+           oldDwAttrs &= ~Const.FILE_ATTRIBUTE_HIDDEN;
+         }
+       }
+       return oldDwAttrs;
+     }
+
      File.Win = exports.OS.Win.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 });
new file mode 100644
--- /dev/null
+++ b/toolkit/components/osfile/tests/xpcshell/test_osfile_win_async_setPermissions.js
@@ -0,0 +1,114 @@
+/* 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 1022816)
+ * The manifest tests on Windows.
+ */
+
+// Sequence of setPermission parameters.
+let testSequence = [
+  [ { winAttributes: { readOnly: true, system: true, hidden: true } },
+    { readOnly: true, system: true, hidden: true } ],
+  [ { winAttributes: { readOnly: false } },
+    { readOnly: false, system: true, hidden: true } ],
+  [ { winAttributes: { system: false } },
+    { readOnly: false, system: false, hidden: true } ],
+  [ { winAttributes: { hidden: false } },
+    { readOnly: false, system: false, hidden: false } ],
+  [ { winAttributes: {readOnly: true, system: false, hidden: false} },
+    { readOnly: true, system: false, hidden: false } ],
+  [ { winAttributes: {readOnly: false, system: true, hidden: false} },
+    { readOnly: false, system: true, hidden: false } ],
+  [ { winAttributes: {readOnly: false, system: false, hidden: true} },
+    { readOnly: false, system: false, hidden: true } ],
+];
+
+// Test application to paths.
+add_task(function* test_path_setPermissions() {
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                          "test_osfile_win_async_setPermissions_path.tmp");
+  yield OS.File.writeAtomic(path, new Uint8Array(1));
+
+  try {
+    for (let [options, attributesExpected] of testSequence) {
+      if (options !== null) {
+        do_print("Setting permissions to " + JSON.stringify(options));
+        yield OS.File.setPermissions(path, options);
+      }
+
+      let stat = yield OS.File.stat(path);
+      do_print("Got stat winAttributes: " + JSON.stringify(stat.winAttributes));
+
+      do_check_eq(stat.winAttributes.readOnly, attributesExpected.readOnly);
+      do_check_eq(stat.winAttributes.system, attributesExpected.system);
+      do_check_eq(stat.winAttributes.hidden, attributesExpected.hidden);
+
+    }
+  } finally {
+    yield OS.File.remove(path);
+  }
+});
+
+// Test application to open files.
+add_task(function* test_file_setPermissions() {
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                              "test_osfile_win_async_setPermissions_file.tmp");
+  yield OS.File.writeAtomic(path, new Uint8Array(1));
+
+  try {
+    let fd = yield OS.File.open(path, { write: true });
+    try {
+      for (let [options, attributesExpected] of testSequence) {
+        if (options !== null) {
+          do_print("Setting permissions to " + JSON.stringify(options));
+          yield fd.setPermissions(options);
+        }
+
+        let stat = yield fd.stat();
+        do_print("Got stat winAttributes: " + JSON.stringify(stat.winAttributes));
+        do_check_eq(stat.winAttributes.readOnly, attributesExpected.readOnly);
+        do_check_eq(stat.winAttributes.system, attributesExpected.system);
+        do_check_eq(stat.winAttributes.hidden, attributesExpected.hidden);
+      }
+    } finally {
+      yield fd.close();
+    }
+  } finally {
+    yield OS.File.remove(path);
+  }
+});
+
+// Test application to Check setPermissions on a non-existant file path.
+add_task(function* test_non_existant_file_path_setPermissions() {
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                          "test_osfile_win_async_setPermissions_path.tmp");
+  Assert.rejects(OS.File.setPermissions(path, {winAttributes: {readOnly: true}}),
+                 /The system cannot find the file specified/,
+                 "setPermissions failed as expected on a non-existant file path");
+});
+
+// Test application to Check setPermissions on a invalid file handle.
+add_task(function* test_closed_file_handle_setPermissions() {
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                          "test_osfile_win_async_setPermissions_path.tmp");
+  yield OS.File.writeAtomic(path, new Uint8Array(1));
+
+  try {
+    let fd = yield OS.File.open(path, { write: true });
+    yield fd.close();
+    Assert.rejects(fd.setPermissions(path, {winAttributes: {readOnly: true}}),
+                   /The handle is invalid/,
+                   "setPermissions failed as expected on a invalid file handle");
+  } 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
@@ -40,8 +40,12 @@ support-files =
 [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_setPermissions.js]
 skip-if = os == "win" || os == "android"
+
+# Windows test
+[test_osfile_win_async_setPermissions.js]
+skip-if = os != "win"