Skip to content

Commit b7dbdfe

Browse files
eileencodesmatzbot
authored andcommitted
[ruby/rubygems] Refactor atomic file write
This refactoring is based off the changes in test/rubygems/test_gem_remote_fetcher.rb. It no longer uses tempfile as a result. ruby/rubygems@be6fd6550b
1 parent 60cf859 commit b7dbdfe

File tree

1 file changed

+46
-38
lines changed

1 file changed

+46
-38
lines changed

lib/rubygems/util/atomic_file_writer.rb

Lines changed: 46 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,55 +12,63 @@ class AtomicFileWriter
1212
# want other processes or threads to see half-written files.
1313

1414
def self.open(file_name)
15-
temp_dir = File.dirname(file_name)
16-
require "tempfile" unless defined?(Tempfile)
15+
require "securerandom" unless defined?(SecureRandom)
1716

18-
Tempfile.create(".#{File.basename(file_name)}", temp_dir) do |temp_file|
19-
temp_file.binmode
20-
return_value = yield temp_file
21-
temp_file.close
17+
old_stat = begin
18+
File.stat(file_name)
19+
rescue SystemCallError
20+
nil
21+
end
2222

23-
original_permissions = if File.exist?(file_name)
24-
File.stat(file_name)
25-
else
26-
# If not possible, probe which are the default permissions in the
27-
# destination directory.
28-
probe_permissions_in(File.dirname(file_name))
29-
end
23+
# Names can't be longer than 255B
24+
tmp_suffix = ".tmp.#{SecureRandom.hex}"
25+
dirname = File.dirname(file_name)
26+
basename = File.basename(file_name)
27+
tmp_path = File.join(dirname, ".#{basename.byteslice(0, 254 - tmp_suffix.bytesize)}#{tmp_suffix}")
28+
29+
flags = File::RDWR | File::CREAT | File::EXCL | File::BINARY
30+
flags |= File::SHARE_DELETE if defined?(File::SHARE_DELETE)
3031

31-
# Set correct permissions on new file
32-
if original_permissions
32+
File.open(tmp_path, flags) do |temp_file|
33+
if old_stat
34+
# Set correct permissions on new file
3335
begin
34-
File.chown(original_permissions.uid, original_permissions.gid, temp_file.path)
35-
File.chmod(original_permissions.mode, temp_file.path)
36+
File.chown(old_stat.uid, old_stat.gid, temp_file.path)
37+
# This operation will affect filesystem ACL's
38+
File.chmod(old_stat.mode, temp_file.path)
3639
rescue Errno::EPERM, Errno::EACCES
3740
# Changing file ownership failed, moving on.
3841
end
3942
end
4043

41-
# Overwrite original file with temp file
42-
File.rename(temp_file.path, file_name)
43-
return_value
44-
end
45-
end
44+
return_val = yield temp_file
45+
rescue StandardError => error
46+
begin
47+
temp_file.close
48+
rescue StandardError
49+
nil
50+
end
4651

47-
def self.probe_permissions_in(dir) # :nodoc:
48-
basename = [
49-
".permissions_check",
50-
Thread.current.object_id,
51-
Process.pid,
52-
rand(1_000_000),
53-
].join(".")
52+
begin
53+
File.unlink(temp_file.path)
54+
rescue StandardError
55+
nil
56+
end
57+
58+
raise error
59+
else
60+
begin
61+
File.rename(temp_file.path, file_name)
62+
rescue StandardError
63+
begin
64+
File.unlink(temp_file.path)
65+
rescue StandardError
66+
end
67+
68+
raise
69+
end
5470

55-
file_name = File.join(dir, basename)
56-
File.open(file_name, "w") {}
57-
File.stat(file_name)
58-
rescue Errno::ENOENT
59-
nil
60-
ensure
61-
begin
62-
File.unlink(file_name) if File.exist?(file_name)
63-
rescue SystemCallError
71+
return_val
6472
end
6573
end
6674
end

0 commit comments

Comments
 (0)