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 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 + def test_cache_update_path_overwrites_existing_file + uri = Gem::URI "http://example/file" + path = File.join @tempdir, "file" - 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 + # Create existing file with old content + File.write(path, "old content") + assert_equal "old content", File.read(path) - raise Gem::RemoteFetcher::FetchError.new("haha!", "") - end - else - def fetcher.fetch_path(arg, *rest) - @test_arg = arg - @test_data - end - end + 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