Project

General

Profile

Actions

Patch #35721

closed

Unlink files after they're closed

Added by Pavel Rosický over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
Attachments
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Tests are trying to unlink the file inside the block while the file descriptor is still active.

they were introduced by #35539

fixes Errno::EACCES: Permission denied

diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb
index 2aee87964..a870e3697 100644
--- a/test/unit/attachment_test.rb
+++ b/test/unit/attachment_test.rb
@@ -276,27 +276,31 @@ class AttachmentTest < ActiveSupport::TestCase
   end

   def test_create_diskfile
+    path = nil
     Attachment.create_diskfile("test_file.txt") do |f|
       path = f.path
       assert_match(/^\d{12}_test_file.txt$/, File.basename(path))
       assert_equal 'test_file.txt', File.basename(path)[13..-1]
-      File.unlink f.path
     end
+    File.unlink path

     Attachment.create_diskfile("test_accentué.txt") do |f|
+      path = f.path
       assert_equal '770c509475505f37c2b8fb6030434d6b.txt', File.basename(f.path)[13..-1]
-      File.unlink f.path
     end
+    File.unlink path

     Attachment.create_diskfile("test_accentué") do |f|
+      path = f.path
       assert_equal 'f8139524ebb8f32e51976982cd20a85d', File.basename(f.path)[13..-1]
-      File.unlink f.path
     end
+    File.unlink path

     Attachment.create_diskfile("test_accentué.ça") do |f|
+      path = f.path
       assert_equal 'cbb5b0f30978ba03731d61f9f6d10011', File.basename(f.path)[13..-1]
-      File.unlink f.path
     end
+    File.unlink path
   end

   def test_title


Related issues

Related to Redmine - Defect #35539: Race condition (possible filename collision) in Attachment.disk_filenameClosedGo MAEDA

Actions
Actions #1

Updated by Go MAEDA over 2 years ago

  • Related to Defect #35539: Race condition (possible filename collision) in Attachment.disk_filename added
Actions #2

Updated by Holger Just over 2 years ago

The tests were specifically written to simulate the race condition. On Filesystems following the usual POSIX semantics, it is allowed to unlink files while they are still opened by any process. They will only actually be removed from the filesystem when the last filehandle is closed by the kernel.

On Windows / NTFS, this is stricter. Here, the filesystem denies moving / deleting any file for which there are open file handles (that is, unless all file handles have the ::File::SHARE_DELETE flag set). Thus, on windows the test might fail.

In any case, I think this patch of the tests is safe and can be applied.

Actions #3

Updated by Mischa The Evil over 2 years ago

  • Description updated (diff)
Actions #4

Updated by Go MAEDA over 2 years ago

  • Status changed from New to Closed

Committed the fix as a part of #35539. Thank you.

Actions

Also available in: Atom PDF