Bug 1009465 - Set the read-only attribute for temporary downloads on Windows. r=paolo, a=sledru
authorGanesh Sahukari <sahukariganesh2@gmail.com>
Wed, 01 Apr 2015 23:43:28 +0530
changeset 260249 b7d8d79c1ee5
parent 260248 97d33db56113
child 260250 6b096f9b31d3
push id727
push userryanvm@gmail.com
push date2015-04-23 14:45 +0000
treeherdermozilla-release@7c66212e4c09 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaolo, sledru
bugs1009465
milestone38.0
Bug 1009465 - Set the read-only attribute for temporary downloads on Windows. r=paolo, a=sledru
toolkit/components/jsdownloads/src/DownloadIntegration.jsm
toolkit/components/jsdownloads/src/DownloadPlatform.cpp
toolkit/components/jsdownloads/test/unit/common_test_Download.js
--- a/toolkit/components/jsdownloads/src/DownloadIntegration.jsm
+++ b/toolkit/components/jsdownloads/src/DownloadIntegration.jsm
@@ -640,20 +640,25 @@ this.DownloadIntegration = {
         // is due to the fact that "deleteTempFileOnExit" is false on Mac, where
         // downloads to be opened with external applications are preserved in
         // the "Downloads" folder like normal downloads.
         let isTemporaryDownload =
           aDownload.launchWhenSucceeded && (aDownload.source.isPrivate ||
           Services.prefs.getBoolPref("browser.helperApps.deleteTempFileOnExit"));
         // Permanently downloaded files are made accessible by other users on
         // this system, while temporary downloads are marked as read-only.
-        let unixMode = isTemporaryDownload ? 0o400 : 0o666;
-        // On Unix, the umask of the process is respected.  This call has no
-        // effect on Windows.
-        yield OS.File.setPermissions(aDownload.target.path, { unixMode });
+        let options = {};
+        if (isTemporaryDownload) {
+          options.unixMode = 0o400;
+          options.winAttributes = {readOnly: true};
+        } else {
+          options.unixMode = 0o666;
+        }
+        // On Unix, the umask of the process is respected.
+        yield 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.
         // 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) {
--- a/toolkit/components/jsdownloads/src/DownloadPlatform.cpp
+++ b/toolkit/components/jsdownloads/src/DownloadPlatform.cpp
@@ -139,35 +139,16 @@ nsresult DownloadPlatform::DownloadDone(
         if (NS_SUCCEEDED(pathString->SetData(path))) {
           (void)obs->NotifyObservers(pathString, "download-watcher-notify",
                                      MOZ_UTF16("modified"));
         }
       }
     }
   }
 
-#ifdef XP_WIN
-  // Adjust file attributes so that by default, new files are indexed by
-  // desktop search services. Skip off those that land in the temp folder.
-  nsCOMPtr<nsIFile> tempDir, fileDir;
-  nsresult rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(tempDir));
-  NS_ENSURE_SUCCESS(rv, rv);
-  aTarget->GetParent(getter_AddRefs(fileDir));
-
-  bool isTemp = false;
-  if (fileDir) {
-    fileDir->Equals(tempDir, &isTemp);
-  }
-
-  nsCOMPtr<nsILocalFileWin> localFileWin(do_QueryInterface(aTarget));
-  if (!isTemp && localFileWin) {
-    localFileWin->SetFileAttributesWin(nsILocalFileWin::WFA_SEARCH_INDEXED);
-  }
-#endif
-
 #endif
 
   return NS_OK;
 }
 
 nsresult DownloadPlatform::MapUrlToZone(const nsAString& aURL,
                                         uint32_t* aZone)
 {
--- a/toolkit/components/jsdownloads/test/unit/common_test_Download.js
+++ b/toolkit/components/jsdownloads/test/unit/common_test_Download.js
@@ -188,18 +188,19 @@ add_task(function test_basic_tryToKeepPa
   do_check_eq(32, download.saver.getSha256Hash().length);
 });
 
 /**
  * Tests the permissions of the final target file once the download finished.
  */
 add_task(function test_unix_permissions()
 {
-  // This test is only executed on Linux and Mac.
-  if (Services.appinfo.OS != "Darwin" && Services.appinfo.OS != "Linux") {
+  // This test is only executed on some Desktop systems.
+  if (Services.appinfo.OS != "Darwin" && Services.appinfo.OS != "Linux" &&
+      Services.appinfo.OS != "WINNT") {
     do_print("Skipping test.");
     return;
   }
 
   let launcherPath = getTempFile("app-launcher").path;
 
   for (let autoDelete of [false, true]) {
     for (let isPrivate of [false, true]) {
@@ -223,22 +224,30 @@ add_task(function test_unix_permissions(
           download = yield promiseStartLegacyDownload(httpUrl("source.txt"), {
             isPrivate,
             launchWhenSucceeded,
             launcherPath: launchWhenSucceeded && launcherPath,
           });
           yield promiseDownloadStopped(download);
         }
 
-        // Temporary downloads should be read-only and not accessible to other
-        // users, while permanently downloaded files should be readable and
-        // writable as specified by the system umask.
         let isTemporary = launchWhenSucceeded && (autoDelete || isPrivate);
-        do_check_eq((yield OS.File.stat(download.target.path)).unixMode,
-                    isTemporary ? 0o400 : (0o666 & ~OS.Constants.Sys.umask));
+        let stat = yield OS.File.stat(download.target.path);
+        if (Services.appinfo.OS == "WINNT") {
+          // On Windows
+          // Temporary downloads should be read-only
+          do_check_eq(stat.winAttributes.readOnly, isTemporary ? true : false);
+        } else {
+          // On Linux, Mac
+          // Temporary downloads should be read-only and not accessible to other
+          // users, while permanently downloaded files should be readable and
+          // writable as specified by the system umask.
+          do_check_eq(stat.unixMode,
+                      isTemporary ? 0o400 : (0o666 & ~OS.Constants.Sys.umask));
+        }
       }
     }
   }
 
   // Clean up the changes to the preference.
   Services.prefs.clearUserPref(kDeleteTempFileOnExit);
 });