win: fix unlink of readonly files

Unix unlinks files even if they are not writable, windows version should behave similarly. Fixes: nodejs/node-v0.x-archive#3006 PR-URL: #327 Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
libuv · Apr 30, 2015 · 0db81a9 · 0db81a9
1 parent 03df7e7
commit 0db81a9
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 1 deletion.
diff --git a/src/win/fs.c b/src/win/fs.c
@@ -661,7 +661,7 @@ void fs__unlink(uv_fs_t* req) {
   NTSTATUS status;
 
   handle = CreateFileW(pathw,
-                       FILE_READ_ATTRIBUTES | DELETE,
+                       FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES | DELETE,
                        FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
                        NULL,
                        OPEN_EXISTING,
@@ -703,6 +703,24 @@ void fs__unlink(uv_fs_t* req) {
     }
   }
 
+  if (info.dwFileAttributes & FILE_ATTRIBUTE_READONLY) {
+    /* Remove read-only attribute */
+    FILE_BASIC_INFORMATION basic = { 0 };
+
+    basic.FileAttributes = info.dwFileAttributes & ~(FILE_ATTRIBUTE_READONLY);
+
+    status = pNtSetInformationFile(handle,
+                                   &iosb,
+                                   &basic,
+                                   sizeof basic,
+                                   FileBasicInformation);
+    if (!NT_SUCCESS(status)) {
+      SET_REQ_WIN32_ERROR(req, pRtlNtStatusToDosError(status));
+      CloseHandle(handle);
+      return;
+    }
+  }
+
   /* Try to set the delete flag. */
   disposition.DeleteFile = TRUE;
   status = pNtSetInformationFile(handle,

diff --git a/test/test-fs.c b/test/test-fs.c
@@ -1323,6 +1323,65 @@ TEST_IMPL(fs_chmod) {
 }
 
 
+TEST_IMPL(fs_unlink_readonly) {
+  int r;
+  uv_fs_t req;
+  uv_file file;
+
+  /* Setup. */
+  unlink("test_file");
+
+  loop = uv_default_loop();
+
+  r = uv_fs_open(loop,
+                 &req,
+                 "test_file",
+                 O_RDWR | O_CREAT,
+                 S_IWUSR | S_IRUSR,
+                 NULL);
+  ASSERT(r >= 0);
+  ASSERT(req.result >= 0);
+  file = req.result;
+  uv_fs_req_cleanup(&req);
+
+  iov = uv_buf_init(test_buf, sizeof(test_buf));
+  r = uv_fs_write(loop, &req, file, &iov, 1, -1, NULL);
+  ASSERT(r == sizeof(test_buf));
+  ASSERT(req.result == sizeof(test_buf));
+  uv_fs_req_cleanup(&req);
+
+  close(file);
+
+  /* Make the file read-only */
+  r = uv_fs_chmod(loop, &req, "test_file", 0400, NULL);
+  ASSERT(r == 0);
+  ASSERT(req.result == 0);
+  uv_fs_req_cleanup(&req);
+
+  check_permission("test_file", 0400);
+
+  /* Try to unlink the file */
+  r = uv_fs_unlink(loop, &req, "test_file", NULL);
+  ASSERT(r == 0);
+  ASSERT(req.result == 0);
+  uv_fs_req_cleanup(&req);
+
+  /*
+  * Run the loop just to check we don't have make any extraneous uv_ref()
+  * calls. This should drop out immediately.
+  */
+  uv_run(loop, UV_RUN_DEFAULT);
+
+  /* Cleanup. */
+  uv_fs_chmod(loop, &req, "test_file", 0600, NULL);
+  uv_fs_req_cleanup(&req);
+  unlink("test_file");
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
+
+
 TEST_IMPL(fs_chown) {
   int r;
   uv_fs_t req;

diff --git a/test/test-list.h b/test/test-list.h
@@ -233,6 +233,7 @@ TEST_DECLARE   (fs_mkdtemp)
 TEST_DECLARE   (fs_fstat)
 TEST_DECLARE   (fs_access)
 TEST_DECLARE   (fs_chmod)
+TEST_DECLARE   (fs_unlink_readonly)
 TEST_DECLARE   (fs_chown)
 TEST_DECLARE   (fs_link)
 TEST_DECLARE   (fs_readlink)
@@ -623,6 +624,7 @@ TASK_LIST_START
   TEST_ENTRY  (fs_fstat)
   TEST_ENTRY  (fs_access)
   TEST_ENTRY  (fs_chmod)
+  TEST_ENTRY  (fs_unlink_readonly)
   TEST_ENTRY  (fs_chown)
   TEST_ENTRY  (fs_utime)
   TEST_ENTRY  (fs_futime)