[rubygems/rubygems] Properly protect writing binstubs with a file lock

There's an issue when multiple processes try to write the same binstub.
The problem is that our file locking mechanism is incorrect because
files are truncated _before_ they are locked. So it can happen that:

* Process A truncates binstub FOO.
* Process B truncates binstub FOO.
* Process A writes binstub FOO for gem BAR from the beginning of file.
* Process B writes binstub FOO for gem BAZ from the beginning of file.

If binstub FOO for gem BAR is bigger than binstub FOO for gem BAZ, then
some bytes will be left around at the end of the binstub, making it
corrupt.

This was not a problem in our specs until the spec testing binstubs with
the same name coming from different gems changed from using gems named
"fake" and "rack" to using gems named "fake" and "myrack". Because of
the difference in gem name length, the generated binstub for gem
"myrack" is now longer, causing the above problem if binstub for gem
myrack is written first.

The solution is to make sure when using flock to always use modes that
DON'T truncate the file when opening it. So, we use `r+` if the file
exists previously (it requires the file to exist previously), otherwise
we use `a+`.

https://github.com/rubygems/rubygems/commit/ce8bcba90f
This commit is contained in:
David Rodríguez 2024-07-02 21:32:31 +02:00 committed by Hiroshi SHIBATA
parent da12d63431
commit d90a930ede
2 changed files with 9 additions and 16 deletions

View File

@ -792,21 +792,18 @@ An Array (#{env.inspect}) was passed in from #{caller[3]}
end
##
# Open a file with given flags. It requires special logic on Windows, like
# protecting access with flock
# Open a file with given flags
def self.open_file(path, flags, &block)
if !java_platform? && win_platform?
open_file_with_flock(path, flags, &block)
else
open_file_without_flock(path, flags, &block)
end
File.open(path, flags, &block)
end
##
# Open a file with given flags, and protect access with flock
def self.open_file_with_flock(path, flags, &block)
def self.open_file_with_flock(path, &block)
flags = File.exist?(path) ? "r+" : "a+"
File.open(path, flags) do |io|
begin
io.flock(File::LOCK_EX)
@ -817,7 +814,7 @@ An Array (#{env.inspect}) was passed in from #{caller[3]}
if Thread.main != Thread.current
raise
else
open_file_without_flock(path, flags, &block)
open_file(path, flags, &block)
end
end
end
@ -1318,10 +1315,6 @@ An Array (#{env.inspect}) was passed in from #{caller[3]}
private
def open_file_without_flock(path, flags, &block)
File.open(path, flags, &block)
end
def already_loaded?(file)
$LOADED_FEATURES.any? do |feature_path|
feature_path.end_with?(file) && default_gem_load_paths.any? {|load_path_entry| feature_path == "#{load_path_entry}/#{file}" }

View File

@ -222,7 +222,7 @@ class Gem::Installer
ruby_executable = false
existing = nil
Gem.open_file_with_flock generated_bin, "rb+" do |io|
File.open generated_bin, "rb" do |io|
line = io.gets
shebang = /^#!.*ruby/o
@ -541,8 +541,8 @@ class Gem::Installer
require "fileutils"
FileUtils.rm_f bin_script_path # prior install may have been --no-wrappers
File.open bin_script_path, "wb", 0o755 do |file|
file.print app_script_text(filename)
Gem.open_file_with_flock(bin_script_path) do |file|
file.write app_script_text(filename)
file.chmod(options[:prog_mode] || 0o755)
end