summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--lib/bundler/shared_helpers.rb3
-rw-r--r--lib/rubygems.rb11
-rw-r--r--lib/rubygems/util/atomic_file_writer.rb67
-rw-r--r--test/rubygems/test_gem_installer.rb34
-rw-r--r--test/rubygems/test_gem_remote_fetcher.rb134
5 files changed, 158 insertions, 91 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 55e727425f..a66e537843 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
diff --git a/test/rubygems/test_gem_installer.rb b/test/rubygems/test_gem_installer.rb
index 293fe1e823..0220a41f88 100644
--- a/test/rubygems/test_gem_installer.rb
+++ b/test/rubygems/test_gem_installer.rb
@@ -2385,25 +2385,31 @@ class TestGemInstaller < Gem::InstallerTestCase
installer = Gem::Installer.for_spec @spec
installer.gem_home = @gemhome
- File.singleton_class.class_eval do
- alias_method :original_binwrite, :binwrite
-
- def binwrite(path, data)
+ assert_raise(Errno::ENOSPC) do
+ Gem::AtomicFileWriter.open(@spec.spec_file) do
raise Errno::ENOSPC
end
end
- assert_raise Errno::ENOSPC do
- installer.write_spec
- end
-
assert_path_not_exist @spec.spec_file
- ensure
- File.singleton_class.class_eval do
- remove_method :binwrite
- alias_method :binwrite, :original_binwrite
- remove_method :original_binwrite
- end
+ end
+
+ def test_write_default_spec
+ @spec = setup_base_spec
+ @spec.files = %w[a.rb b.rb c.rb]
+
+ installer = Gem::Installer.for_spec @spec
+ installer.gem_home = @gemhome
+
+ installer.write_default_spec
+
+ assert_path_exist installer.default_spec_file
+
+ loaded = Gem::Specification.load installer.default_spec_file
+
+ assert_equal @spec.files, loaded.files
+ assert_equal @spec.name, loaded.name
+ assert_equal @spec.version, loaded.version
end
def test_dir
diff --git a/test/rubygems/test_gem_remote_fetcher.rb b/test/rubygems/test_gem_remote_fetcher.rb
index 5c1d89fad6..9badd75b42 100644
--- a/test/rubygems/test_gem_remote_fetcher.rb
+++ b/test/rubygems/test_gem_remote_fetcher.rb
@@ -60,7 +60,7 @@ class TestGemRemoteFetcher < Gem::TestCase
uri = Gem::URI "http://example/file"
path = File.join @tempdir, "file"
- fetcher = util_fuck_with_fetcher "hello"
+ fetcher = fake_fetcher(uri.to_s, "hello")
data = fetcher.cache_update_path uri, path
@@ -75,7 +75,7 @@ class TestGemRemoteFetcher < Gem::TestCase
path = File.join @tempdir, "file"
data = String.new("\xC8").force_encoding(Encoding::BINARY)
- fetcher = util_fuck_with_fetcher data
+ fetcher = fake_fetcher(uri.to_s, data)
written_data = fetcher.cache_update_path uri, path
@@ -88,7 +88,7 @@ class TestGemRemoteFetcher < Gem::TestCase
uri = Gem::URI "http://example/file"
path = File.join @tempdir, "file"
- fetcher = util_fuck_with_fetcher "hello"
+ fetcher = fake_fetcher(uri.to_s, "hello")
data = fetcher.cache_update_path uri, path, false
@@ -97,103 +97,79 @@ class TestGemRemoteFetcher < Gem::TestCase
assert_path_not_exist path
end
- def util_fuck_with_fetcher(data, blow = false)
- fetcher = Gem::RemoteFetcher.fetcher
- fetcher.instance_variable_set :@test_data, data
-
- if blow
- def fetcher.fetch_path(arg, *rest)
- # OMG I'm such an ass
- class << self; remove_method :fetch_path; end
- def self.fetch_path(arg, *rest)
- @test_arg = arg
- @test_data
- end
+ def test_cache_update_path_overwrites_existing_file
+ uri = Gem::URI "http://example/file"
+ path = File.join @tempdir, "file"
- raise Gem::RemoteFetcher::FetchError.new("haha!", "")
- end
- else
- def fetcher.fetch_path(arg, *rest)
- @test_arg = arg
- @test_data
- end
- end
+ # Create existing file with old content
+ File.write(path, "old content")
+ assert_equal "old content", File.read(path)
+
+ fetcher = fake_fetcher(uri.to_s, "new content")
- fetcher
+ data = fetcher.cache_update_path uri, path
+
+ assert_equal "new content", data
+ assert_equal "new content", File.read(path)
end
def test_download
- a1_data = nil
- File.open @a1_gem, "rb" do |fp|
- a1_data = fp.read
- end
+ a1_data = File.open @a1_gem, "rb", &:read
+ a1_url = "http://gems.example.com/gems/a-1.gem"
- fetcher = util_fuck_with_fetcher a1_data
+ fetcher = fake_fetcher(a1_url, a1_data)
a1_cache_gem = @a1.cache_file
assert_equal a1_cache_gem, fetcher.download(@a1, "http://gems.example.com")
- assert_equal("http://gems.example.com/gems/a-1.gem",
- fetcher.instance_variable_get(:@test_arg).to_s)
+ assert_equal a1_url, fetcher.paths.last
assert File.exist?(a1_cache_gem)
end
def test_download_with_auth
- a1_data = nil
- File.open @a1_gem, "rb" do |fp|
- a1_data = fp.read
- end
+ a1_data = File.open @a1_gem, "rb", &:read
+ a1_url = "http://user:password@gems.example.com/gems/a-1.gem"
- fetcher = util_fuck_with_fetcher a1_data
+ fetcher = fake_fetcher(a1_url, a1_data)
a1_cache_gem = @a1.cache_file
assert_equal a1_cache_gem, fetcher.download(@a1, "http://user:password@gems.example.com")
- assert_equal("http://user:password@gems.example.com/gems/a-1.gem",
- fetcher.instance_variable_get(:@test_arg).to_s)
+ assert_equal a1_url, fetcher.paths.last
assert File.exist?(a1_cache_gem)
end
def test_download_with_token
- a1_data = nil
- File.open @a1_gem, "rb" do |fp|
- a1_data = fp.read
- end
+ a1_data = File.open @a1_gem, "rb", &:read
+ a1_url = "http://token@gems.example.com/gems/a-1.gem"
- fetcher = util_fuck_with_fetcher a1_data
+ fetcher = fake_fetcher(a1_url, a1_data)
a1_cache_gem = @a1.cache_file
assert_equal a1_cache_gem, fetcher.download(@a1, "http://token@gems.example.com")
- assert_equal("http://token@gems.example.com/gems/a-1.gem",
- fetcher.instance_variable_get(:@test_arg).to_s)
+ assert_equal a1_url, fetcher.paths.last
assert File.exist?(a1_cache_gem)
end
def test_download_with_x_oauth_basic
- a1_data = nil
- File.open @a1_gem, "rb" do |fp|
- a1_data = fp.read
- end
+ a1_data = File.open @a1_gem, "rb", &:read
+ a1_url = "http://token:x-oauth-basic@gems.example.com/gems/a-1.gem"
- fetcher = util_fuck_with_fetcher a1_data
+ fetcher = fake_fetcher(a1_url, a1_data)
a1_cache_gem = @a1.cache_file
assert_equal a1_cache_gem, fetcher.download(@a1, "http://token:x-oauth-basic@gems.example.com")
- assert_equal("http://token:x-oauth-basic@gems.example.com/gems/a-1.gem",
- fetcher.instance_variable_get(:@test_arg).to_s)
+ assert_equal a1_url, fetcher.paths.last
assert File.exist?(a1_cache_gem)
end
def test_download_with_encoded_auth
- a1_data = nil
- File.open @a1_gem, "rb" do |fp|
- a1_data = fp.read
- end
+ a1_data = File.open @a1_gem, "rb", &:read
+ a1_url = "http://user:%25pas%25sword@gems.example.com/gems/a-1.gem"
- fetcher = util_fuck_with_fetcher a1_data
+ fetcher = fake_fetcher(a1_url, a1_data)
a1_cache_gem = @a1.cache_file
assert_equal a1_cache_gem, fetcher.download(@a1, "http://user:%25pas%25sword@gems.example.com")
- assert_equal("http://user:%25pas%25sword@gems.example.com/gems/a-1.gem",
- fetcher.instance_variable_get(:@test_arg).to_s)
+ assert_equal a1_url, fetcher.paths.last
assert File.exist?(a1_cache_gem)
end
@@ -235,8 +211,9 @@ class TestGemRemoteFetcher < Gem::TestCase
def test_download_install_dir
a1_data = File.open @a1_gem, "rb", &:read
+ a1_url = "http://gems.example.com/gems/a-1.gem"
- fetcher = util_fuck_with_fetcher a1_data
+ fetcher = fake_fetcher(a1_url, a1_data)
install_dir = File.join @tempdir, "more_gems"
@@ -245,8 +222,7 @@ class TestGemRemoteFetcher < Gem::TestCase
actual = fetcher.download(@a1, "http://gems.example.com", install_dir)
assert_equal a1_cache_gem, actual
- assert_equal("http://gems.example.com/gems/a-1.gem",
- fetcher.instance_variable_get(:@test_arg).to_s)
+ assert_equal a1_url, fetcher.paths.last
assert File.exist?(a1_cache_gem)
end
@@ -282,7 +258,12 @@ class TestGemRemoteFetcher < Gem::TestCase
FileUtils.chmod 0o555, @a1.cache_dir
FileUtils.chmod 0o555, @gemhome
- fetcher = util_fuck_with_fetcher File.read(@a1_gem)
+ fetcher = Gem::RemoteFetcher.fetcher
+ def fetcher.fetch_path(uri, *rest)
+ File.read File.join(@test_gem_dir, "a-1.gem")
+ end
+ fetcher.instance_variable_set(:@test_gem_dir, File.dirname(@a1_gem))
+
fetcher.download(@a1, "http://gems.example.com")
a1_cache_gem = File.join Gem.user_dir, "cache", @a1.file_name
assert File.exist? a1_cache_gem
@@ -301,19 +282,21 @@ class TestGemRemoteFetcher < Gem::TestCase
end
e1.loaded_from = File.join(@gemhome, "specifications", e1.full_name)
- e1_data = nil
- File.open e1_gem, "rb" do |fp|
- e1_data = fp.read
- end
+ e1_data = File.open e1_gem, "rb", &:read
- fetcher = util_fuck_with_fetcher e1_data, :blow_chunks
+ fetcher = Gem::RemoteFetcher.fetcher
+ def fetcher.fetch_path(uri, *rest)
+ @call_count ||= 0
+ @call_count += 1
+ raise Gem::RemoteFetcher::FetchError.new("error", uri) if @call_count == 1
+ @test_data
+ end
+ fetcher.instance_variable_set(:@test_data, e1_data)
e1_cache_gem = e1.cache_file
assert_equal e1_cache_gem, fetcher.download(e1, "http://gems.example.com")
- assert_equal("http://gems.example.com/gems/#{e1.original_name}.gem",
- fetcher.instance_variable_get(:@test_arg).to_s)
assert File.exist?(e1_cache_gem)
end
@@ -592,6 +575,8 @@ class TestGemRemoteFetcher < Gem::TestCase
end
end
+ private
+
def assert_error(exception_class = Exception)
got_exception = false
@@ -603,4 +588,13 @@ class TestGemRemoteFetcher < Gem::TestCase
assert got_exception, "Expected exception conforming to #{exception_class}"
end
+
+ def fake_fetcher(url, data)
+ original_fetcher = Gem::RemoteFetcher.fetcher
+ fetcher = Gem::FakeFetcher.new
+ fetcher.data[url] = data
+ Gem::RemoteFetcher.fetcher = fetcher
+ ensure
+ Gem::RemoteFetcher.fetcher = original_fetcher
+ end
end