diff --git a/lib/rubygems/config_file.rb b/lib/rubygems/config_file.rb index e9cd2c6c3554..853de9e8ecd1 100644 --- a/lib/rubygems/config_file.rb +++ b/lib/rubygems/config_file.rb @@ -191,7 +191,12 @@ class Gem::ConfigFile ## # Use a global cache for .gem files shared across all Ruby installations. - # When enabled, gems are cached to ~/.cache/gem/gems (or XDG_CACHE_HOME/gem/gems). + # When enabled, gems fetched from a remote source are cached to + # ~/.cache/gem/gems (or XDG_CACHE_HOME/gem/gems). Gems installed from a + # local path are not, since a cached copy is later reused without being + # verified again. gem fetch still writes to the working directory, + # and an unwritable cache directory falls back to the cache of the + # installation. attr_accessor :global_gem_cache diff --git a/lib/rubygems/remote_fetcher.rb b/lib/rubygems/remote_fetcher.rb index 5b83dc6f6f20..d3ab256029e8 100644 --- a/lib/rubygems/remote_fetcher.rb +++ b/lib/rubygems/remote_fetcher.rb @@ -47,6 +47,10 @@ class UnknownHostError < FetchError end deprecate_constant(:UnknownHostError) + # Schemes fetched over the network, as opposed to copied from a local path. + REMOTE_SCHEMES = %w[http https s3].freeze + private_constant :REMOTE_SCHEMES + @fetcher = nil ## @@ -113,12 +117,21 @@ def download_to_cache(dependency) def download(spec, source_uri, install_dir = Gem.dir) gem_file_name = File.basename spec.cache_file + source_uri = Gem::Uri.new(source_uri) + + scheme = source_uri.scheme + + # Gem::URI.parse gets confused by MS Windows paths with forward slashes. + scheme = nil if /^[a-z]$/i.match?(scheme) + + remote_source = REMOTE_SCHEMES.include?(scheme) + install_cache_dir = File.join install_dir, "cache" cache_dir = - if Gem.configuration.global_gem_cache - Gem.global_gem_cache_path - elsif Dir.pwd == install_dir # see fetch_command + if File.identical?(".", install_dir) # gem fetch asks for it this way install_dir + elsif Gem.configuration.global_gem_cache && remote_source && ensure_writable_cache_dir(Gem.global_gem_cache_path) + Gem.global_gem_cache_path elsif File.writable?(install_cache_dir) || (File.writable?(install_dir) && !File.exist?(install_cache_dir)) install_cache_dir else @@ -134,20 +147,15 @@ def download(spec, source_uri, install_dir = Gem.dir) nil end unless File.exist? cache_dir - source_uri = Gem::Uri.new(source_uri) - - scheme = source_uri.scheme - - # Gem::URI.parse gets confused by MS Windows paths with forward slashes. - scheme = nil if /^[a-z]$/i.match?(scheme) - # REFACTOR: split this up and dispatch on scheme (eg download_http) # REFACTOR: be sure to clean up fake fetcher when you do this... cleaner case scheme - when "http", "https", "s3" then - unless File.exist? local_gem_path + when *REMOTE_SCHEMES then + if File.exist? local_gem_path + verbose "Using local gem #{local_gem_path}" + else begin - verbose "Downloading gem #{gem_file_name}" + verbose "Downloading gem #{gem_file_name} to #{cache_dir}" remote_gem_path = source_uri + "gems/#{gem_file_name}" @@ -157,7 +165,7 @@ def download(spec, source_uri, install_dir = Gem.dir) alternate_name = "#{spec.original_name}.gem" - verbose "Failed, downloading gem #{alternate_name}" + verbose "Failed, downloading gem #{alternate_name} to #{cache_dir}" remote_gem_path = source_uri + "gems/#{alternate_name}" @@ -171,7 +179,7 @@ def download(spec, source_uri, install_dir = Gem.dir) remote_gem_path = Gem::Util.correct_for_windows_path(File.join(path, "gems", gem_file_name)) - FileUtils.cp(remote_gem_path, local_gem_path) + atomic_copy(remote_gem_path, local_gem_path) rescue Errno::EACCES local_gem_path = source_uri.to_s end @@ -188,7 +196,7 @@ def download(spec, source_uri, install_dir = Gem.dir) source_path = Gem::UriFormatter.new(source_path).unescape begin - FileUtils.cp source_path, local_gem_path unless + atomic_copy(source_path, local_gem_path) unless File.identical?(source_path, local_gem_path) rescue Errno::EACCES local_gem_path = source_uri.to_s @@ -338,6 +346,40 @@ def close_all private + # Creates +cache_dir+ so its writability can be probed, since File.writable? + # is false for a path that does not exist yet. + + def ensure_writable_cache_dir(cache_dir) + require "fileutils" + begin + FileUtils.mkdir_p cache_dir + rescue SystemCallError + return false + end + + File.writable?(cache_dir) + end + + def atomic_copy(source_path, destination_path) + File.open(source_path, "rb") do |source| + # FileUtils.cp passed the source mode to File.open, so it only reached a + # file being created and the umask still applied to it. The writer + # already carries over the mode of a file it replaces. + mode = source.stat.mode & 0o777 & ~File.umask + replacing = File.exist?(destination_path) + + Gem::AtomicFileWriter.open(destination_path) do |io| + IO.copy_stream(source, io) + + begin + io.chmod(mode) unless replacing + rescue Errno::EPERM, Errno::EACCES + # the filesystem does not carry permissions + end + end + end + end + def proxy_for(proxy, uri) Gem::Request.proxy_uri(proxy || Gem::Request.get_proxy_from_env(uri.scheme)) end diff --git a/spec/install/global_cache_spec.rb b/spec/install/global_cache_spec.rb index 5e5494dbe9d3..ce0564ee7851 100644 --- a/spec/install/global_cache_spec.rb +++ b/spec/install/global_cache_spec.rb @@ -9,11 +9,11 @@ let(:source2) { "http://gemserver.example.org" } def cache_base - # Use the unified global gem cache path if the RubyGems under test - # provides it, otherwise fall back to the Bundler-specific cache - # location that Bundler uses on RubyGems older than 4.0 - if exercised_rubygems_version >= Gem::Version.new("4.0.0.a") - Pathname.new(Gem.global_gem_cache_path) + # Gem.global_gem_cache_path first ships in RubyGems 4.1. Older RubyGems + # fall back to the Bundler-specific cache location. The suite clears + # XDG_CACHE_HOME, so the path resolves to the ~/.cache default. + if exercised_rubygems_version >= Gem::Version.new("4.1.0.a") + home(".cache", "gem", "gems") else home(".bundle", "cache", "gems") end diff --git a/test/rubygems/test_gem_remote_fetcher.rb b/test/rubygems/test_gem_remote_fetcher.rb index c35da2fc5ae2..2c599e00986b 100644 --- a/test/rubygems/test_gem_remote_fetcher.rb +++ b/test/rubygems/test_gem_remote_fetcher.rb @@ -643,6 +643,234 @@ def fetcher.fetch_path(uri, *rest) end end + def test_download_with_global_gem_cache_fetches_to_current_directory + test_cache_dir = File.join(@tempdir, "global_gem_cache_test") + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + fetcher = Gem::RemoteFetcher.fetcher + def fetcher.fetch_path(uri, *rest) + File.binread File.join(@test_gem_dir, "a-1.gem") + end + fetcher.instance_variable_set(:@test_gem_dir, File.dirname(@a1_gem)) + + fetch_dir = File.join @tempdir, "fetch_dir" + FileUtils.mkdir_p fetch_dir + + # gem fetch downloads into the current directory, see fetch_command + fetched_gem = Dir.chdir fetch_dir do + fetcher.download(@a1, "http://gems.example.com", fetch_dir) + end + + assert_equal File.join(fetch_dir, @a1.file_name), fetched_gem + assert File.exist?(fetched_gem) + refute File.exist?(test_cache_dir), + "gem fetch output should not be diverted to the global cache" + end + ensure + Gem.configuration.global_gem_cache = false + end + + def test_download_local_takes_the_source_permissions_through_the_umask + omit "File.chmod doesn't work on Windows" if Gem.win_platform? + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + FileUtils.chmod 0o666, local_path + inst = nil + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + assert_equal @a1.cache_file, inst.download(@a1, local_path) + assert_equal 0o666 & ~File.umask, File.stat(@a1.cache_file).mode & 0o777 + end + + def test_download_local_keeps_a_restrictive_source_permission + omit "File.chmod doesn't work on Windows" if Gem.win_platform? + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + FileUtils.chmod 0o600, local_path + inst = nil + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + # a mode the writer would not produce on its own, so dropping the chmod + # would show up here + assert_equal @a1.cache_file, inst.download(@a1, local_path) + assert_equal 0o600, File.stat(@a1.cache_file).mode & 0o777 + end + + def test_download_local_keeps_the_replaced_cache_file_permissions + omit "File.chmod doesn't work on Windows" if Gem.win_platform? + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + FileUtils.chmod 0o666, local_path + inst = nil + + FileUtils.mkdir_p File.dirname(@a1.cache_file) + FileUtils.touch @a1.cache_file + FileUtils.chmod 0o640, @a1.cache_file + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + assert_equal @a1.cache_file, inst.download(@a1, local_path) + assert_equal 0o640, File.stat(@a1.cache_file).mode & 0o777 + end + + def test_download_to_current_directory_reached_through_a_symlink + omit "symlinks are not usable on Windows" if Gem.win_platform? + + fetch_dir = File.join @tempdir, "fetch_dir" + FileUtils.mkdir_p fetch_dir + linked_dir = File.join @tempdir, "linked_dir" + File.symlink fetch_dir, linked_dir + + fetcher = Gem::RemoteFetcher.fetcher + def fetcher.fetch_path(uri, *rest) + File.binread File.join(@test_gem_dir, "a-1.gem") + end + fetcher.instance_variable_set(:@test_gem_dir, File.dirname(@a1_gem)) + + # gem fetch passes the working directory as install_dir, and the two can + # name the same directory through different paths + fetched_gem = Dir.chdir fetch_dir do + fetcher.download(@a1, "http://gems.example.com", linked_dir) + end + + assert_equal File.join(linked_dir, @a1.file_name), fetched_gem + assert File.exist?(fetched_gem) + end + + def test_download_local_with_global_gem_cache + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + test_cache_dir = File.join(@tempdir, "global_gem_cache_test") + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + inst = nil + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + assert_equal @a1.cache_file, inst.download(@a1, local_path) + refute File.exist?(test_cache_dir), + "local gems should not be copied to the global cache" + end + ensure + Gem.configuration.global_gem_cache = false + end + + def test_download_file_scheme_with_global_gem_cache + test_cache_dir = File.join(@tempdir, "global_gem_cache_test") + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + repo_dir = File.join @tempdir, "repo" + FileUtils.mkdir_p File.join(repo_dir, "gems") + FileUtils.cp @a1_gem, File.join(repo_dir, "gems", @a1.file_name) + + uri_path = repo_dir.start_with?("/") ? repo_dir : "/#{repo_dir}" + inst = Gem::RemoteFetcher.fetcher + + assert_equal @a1.cache_file, inst.download(@a1, "file://#{uri_path}") + assert File.exist?(@a1.cache_file) + refute File.exist?(test_cache_dir), + "local gems should not be copied to the global cache" + end + ensure + Gem.configuration.global_gem_cache = false + end + + unless Gem.win_platform? || Process.uid.zero? # File.chmod doesn't work + def test_download_with_global_gem_cache_not_writable + test_cache_dir = File.join(@tempdir, "global_gem_cache_test") + FileUtils.mkdir_p test_cache_dir + FileUtils.chmod 0o555, test_cache_dir + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + fetcher = Gem::RemoteFetcher.fetcher + def fetcher.fetch_path(uri, *rest) + File.binread File.join(@test_gem_dir, "a-1.gem") + end + fetcher.instance_variable_set(:@test_gem_dir, File.dirname(@a1_gem)) + + a1_cache_gem = @a1.cache_file + assert_equal a1_cache_gem, fetcher.download(@a1, "http://gems.example.com") + assert File.exist?(a1_cache_gem) + assert_empty Dir.children(test_cache_dir) + end + ensure + FileUtils.chmod 0o755, test_cache_dir if File.exist?(test_cache_dir) + Gem.configuration.global_gem_cache = false + end + + def test_download_with_global_gem_cache_not_creatable + parent_dir = File.join(@tempdir, "global_gem_cache_parent") + FileUtils.mkdir_p parent_dir + FileUtils.chmod 0o555, parent_dir + test_cache_dir = File.join(parent_dir, "gems") + + Gem.stub :global_gem_cache_path, test_cache_dir do + Gem.configuration.global_gem_cache = true + + fetcher = Gem::RemoteFetcher.fetcher + def fetcher.fetch_path(uri, *rest) + File.binread File.join(@test_gem_dir, "a-1.gem") + end + fetcher.instance_variable_set(:@test_gem_dir, File.dirname(@a1_gem)) + + a1_cache_gem = @a1.cache_file + assert_equal a1_cache_gem, fetcher.download(@a1, "http://gems.example.com") + assert File.exist?(a1_cache_gem) + refute File.exist?(test_cache_dir) + end + ensure + FileUtils.chmod 0o755, parent_dir if File.exist?(parent_dir) + Gem.configuration.global_gem_cache = false + end + + def test_download_local_replaces_read_only_cache_file + omit "doesn't work if tempdir has +" if @tempdir.include?("+") + FileUtils.mv @a1_gem, @tempdir + local_path = File.join @tempdir, @a1.file_name + inst = nil + + FileUtils.mkdir_p File.dirname(@a1.cache_file) + FileUtils.touch @a1.cache_file + FileUtils.chmod 0o444, @a1.cache_file + + Dir.chdir @tempdir do + inst = Gem::RemoteFetcher.fetcher + end + + # the atomic replacement of the cache copy must not depend on the + # permissions of the previous file + assert_equal @a1.cache_file, inst.download(@a1, local_path) + assert_equal File.binread(local_path), File.binread(@a1.cache_file) + ensure + FileUtils.chmod 0o644, @a1.cache_file if File.exist?(@a1.cache_file) + end + end + def test_fetch_http_with_custom_error_header fetcher = Gem::RemoteFetcher.new nil @fetcher = fetcher