diff options
| author | eileencodes <eileencodes@gmail.com> | 2025-11-25 14:17:03 -0500 |
|---|---|---|
| committer | Hiroshi SHIBATA <hsbt@ruby-lang.org> | 2025-12-26 11:00:51 +0900 |
| commit | bdbe8d50150447748eaa92a0cce7327d8dec9903 (patch) | |
| tree | 434fe58039a0c54819d314e65adce44d3ac17976 /lib | |
| parent | c5376a3a167cbb90023e7610a4fafda22a5c381c (diff) | |
[ruby/rubygems] Write gem files atomically
This change updates `write_binary` to use a new class,
`AtomicFileWriter.open` to write the gem's files. This implementation
is borrowed from Active Support's [`atomic_write`](https://github.com/rails/rails/blob/main/activesupport/lib/active_support/core_ext/file/atomic.rb).
Atomic write will write the files to a temporary file and then once
created, sets permissions and renames the file. If the file is corrupted
- ie on failed download, an error occurs, or for some other reason, the
real file will not be created. The changes made here make `verify_gz`
obsolete, we don't need to verify it if we have successfully created the
file atomically. If it exists, it is not corrupt. If it is corrupt, the
file won't exist on disk.
While writing tests for this functionality I replaced the
`RemoteFetcher` stub with `FakeFetcher` except for where we really do
need to overwrite the `RemoteFetcher`. The new test implementation is much
clearer on what it's trying to accomplish versus the prior test
implementation.
https://github.com/ruby/rubygems/commit/0cd4b54291
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/bundler/shared_helpers.rb | 3 | ||||
| -rw-r--r-- | lib/rubygems.rb | 11 | ||||
| -rw-r--r-- | lib/rubygems/util/atomic_file_writer.rb | 67 |
3 files changed, 74 insertions, 7 deletions
diff --git a/lib/bundler/shared_helpers.rb b/lib/bundler/shared_helpers.rb index 6419e42997..2aa8abe0a0 100644 --- a/lib/bundler/shared_helpers.rb +++ b/lib/bundler/shared_helpers.rb @@ -105,7 +105,8 @@ module Bundler def filesystem_access(path, action = :write, &block) yield(path.dup) rescue Errno::EACCES => e - raise unless e.message.include?(path.to_s) || action == :create + path_basename = File.basename(path.to_s) + raise unless e.message.include?(path_basename) || action == :create raise PermissionError.new(path, action) rescue Errno::EAGAIN diff --git a/lib/rubygems.rb b/lib/rubygems.rb index e99176fec0..41b39a808b 100644 --- a/lib/rubygems.rb +++ b/lib/rubygems.rb @@ -17,6 +17,7 @@ require_relative "rubygems/deprecate" require_relative "rubygems/errors" require_relative "rubygems/target_rbconfig" require_relative "rubygems/win_platform" +require_relative "rubygems/util/atomic_file_writer" ## # RubyGems is the Ruby standard for publishing and managing third party @@ -833,14 +834,12 @@ An Array (#{env.inspect}) was passed in from #{caller[3]} end ## - # Safely write a file in binary mode on all platforms. + # Atomically write a file in binary mode on all platforms. def self.write_binary(path, data) - File.binwrite(path, data) - rescue Errno::ENOSPC - # If we ran out of space but the file exists, it's *guaranteed* to be corrupted. - File.delete(path) if File.exist?(path) - raise + Gem::AtomicFileWriter.open(path) do |file| + file.write(data) + end end ## diff --git a/lib/rubygems/util/atomic_file_writer.rb b/lib/rubygems/util/atomic_file_writer.rb new file mode 100644 index 0000000000..7d1d6a7416 --- /dev/null +++ b/lib/rubygems/util/atomic_file_writer.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +# Based on ActiveSupport's AtomicFile implementation +# Copyright (c) David Heinemeier Hansson +# https://github.com/rails/rails/blob/main/activesupport/lib/active_support/core_ext/file/atomic.rb +# Licensed under the MIT License + +module Gem + class AtomicFileWriter + ## + # Write to a file atomically. Useful for situations where you don't + # want other processes or threads to see half-written files. + + def self.open(file_name) + temp_dir = File.dirname(file_name) + require "tempfile" unless defined?(Tempfile) + + Tempfile.create(".#{File.basename(file_name)}", temp_dir) do |temp_file| + temp_file.binmode + return_value = yield temp_file + temp_file.close + + original_permissions = if File.exist?(file_name) + File.stat(file_name) + else + # If not possible, probe which are the default permissions in the + # destination directory. + probe_permissions_in(File.dirname(file_name)) + end + + # Set correct permissions on new file + if original_permissions + begin + File.chown(original_permissions.uid, original_permissions.gid, temp_file.path) + File.chmod(original_permissions.mode, temp_file.path) + rescue Errno::EPERM, Errno::EACCES + # Changing file ownership failed, moving on. + end + end + + # Overwrite original file with temp file + File.rename(temp_file.path, file_name) + return_value + end + end + + def self.probe_permissions_in(dir) # :nodoc: + basename = [ + ".permissions_check", + Thread.current.object_id, + Process.pid, + rand(1_000_000), + ].join(".") + + file_name = File.join(dir, basename) + File.open(file_name, "w") {} + File.stat(file_name) + rescue Errno::ENOENT + nil + ensure + begin + File.unlink(file_name) if File.exist?(file_name) + rescue SystemCallError + end + end + end +end |
