diff --git a/.github/workflows/check_sast.yml b/.github/workflows/check_sast.yml index 669261535b13a0..90ed8a5bc9767f 100644 --- a/.github/workflows/check_sast.yml +++ b/.github/workflows/check_sast.yml @@ -45,7 +45,7 @@ jobs: persist-credentials: false - name: Run zizmor - uses: zizmorcore/zizmor-action@3dc1ecc9bcb9e94e9b2c709687979e1298497054 # v0.6.2 + uses: zizmorcore/zizmor-action@70fb788f84895a7701f5643d103d587e460b5c99 # v0.6.3 continue-on-error: true analyze: diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index e0cc2eff496ab7..0481646faa75fc 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -98,7 +98,7 @@ jobs: rustup install ${{ matrix.rust_version }} --profile minimal rustup default ${{ matrix.rust_version }} - - uses: taiki-e/install-action@742a3317eac7bd62f91cd888b4eead5e784ba833 # v2.87.1 + - uses: taiki-e/install-action@1ed6d7be6168f6c9046541087ff549b6bc581fdf # v2.87.2 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 4b57137ba7f42c..2443e3aaa3446a 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -152,7 +152,7 @@ jobs: ruby-version: '3.1' bundler: none - - uses: taiki-e/install-action@742a3317eac7bd62f91cd888b4eead5e784ba833 # v2.87.1 + - uses: taiki-e/install-action@1ed6d7be6168f6c9046541087ff549b6bc581fdf # v2.87.2 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/file.c b/file.c index 8f860f3dc10f74..7193bc28dd99f2 100644 --- a/file.c +++ b/file.c @@ -2482,21 +2482,29 @@ rb_file_sticky_p(VALUE obj, VALUE fname) /* * call-seq: - * File.identical?(file_1, file_2) -> true or false + * File.identical?(object_0, object_1) -> true or false * - * Returns true if the named files are identical. + * Returns whether the given objects represent filesystem entries that are identical; + * each object may be a string path or an IO object: * - * _file_1_ and _file_2_ can be an IO object. + * # Paths. + * File.identical?('README.md', 'README.md') # => true # Same path. + * File.identical?('README.md', './README.md') # => true # Same entry. + * File.identical?('.', '.') # => true # Directory. + * File.identical?('README.md', 'LEGAL') # => false + * File.identical?('README.md', 'nosuch') # => false # Non-existent entry. + * # Links and File object. + * File.link('README.md', 'link') # Symbolic link. + * File.symlink('README.md', 'symlink') # Hard link. + * file = File.open('README.md', 'r') # File object. + * File.identical?('README.md', 'link') # => true + * File.identical?('README.md', 'symlink') # => true + * File.identical?('README.md', file) # => true + * # Clean up. + * File.unlink('link') + * File.unlink('symlink') + * file.close * - * open("a", "w") {} - * p File.identical?("a", "a") #=> true - * p File.identical?("a", "./a") #=> true - * File.link("a", "b") - * p File.identical?("a", "b") #=> true - * File.symlink("a", "c") - * p File.identical?("a", "c") #=> true - * open("d", "w") {} - * p File.identical?("a", "d") #=> false */ static VALUE diff --git a/lib/bundler/cli/exec.rb b/lib/bundler/cli/exec.rb index 2fdc4162868abe..20ed1d8bc62c38 100644 --- a/lib/bundler/cli/exec.rb +++ b/lib/bundler/cli/exec.rb @@ -23,7 +23,7 @@ def run bin_path.delete_suffix!(".bat") if Gem.win_platform? kernel_load(bin_path, *args) else - bin_path = "./" + bin_path unless File.absolute_path?(bin_path) + bin_path = explicit_path(bin_path) kernel_exec(bin_path, *args) end else @@ -71,6 +71,20 @@ def process_title(file, args) "#{file} #{args.join(" ")}".strip end + def explicit_path(path) + if File.absolute_path?(path) || explicit_relative_path?(path) + path + else + ".#{File::SEPARATOR}#{path}" + end + end + + def explicit_relative_path?(path) + [File::SEPARATOR, File::ALT_SEPARATOR].compact.any? do |separator| + path.start_with?(".#{separator}", "..#{separator}") + end + end + def directly_loadable?(file) if Gem.win_platform? script_wrapper?(file) diff --git a/lib/bundler/source/git/git_proxy.rb b/lib/bundler/source/git/git_proxy.rb index 3df37e487bdb91..42f1e9dd71ffb2 100644 --- a/lib/bundler/source/git/git_proxy.rb +++ b/lib/bundler/source/git/git_proxy.rb @@ -193,7 +193,13 @@ def git_remote_fetch(args) return out if status.success? if err.include?("couldn't find remote ref") || err.include?("not our ref") - raise MissingGitRevisionError.new(command_with_no_credentials, path, commit || explicit_ref, credential_filtered_uri) + default_branch = renamed_remote_default_branch if tracking_remote_default_branch? + if default_branch + out = follow_remote_default_branch(args, default_branch) + return out if out + end + + raise MissingGitRevisionError.new(command_with_no_credentials, path, commit || explicit_ref || current_branch, credential_filtered_uri) else if shallow? args -= depth_args @@ -298,6 +304,61 @@ def not_pinned? branch_option || ref.nil? end + def tracking_remote_default_branch? + explicit_ref.nil? && commit.nil? + end + + # Returns nil on any failure, leaving HEAD untouched so the caller reports + # the original fetch failure. Runs inside the retry block, so it must not + # touch the caller's command locals. + def follow_remote_default_branch(args, default_branch) + reference = "refs/heads/#{default_branch}" + command = fetch_command(args, "#{reference}:#{reference}") + check_allowed(command) + + out, err, status = capture(command, path) + unless status.success? + Bundler.ui.debug "Could not fetch #{reference} from #{credential_filtered_uri}: #{err}" + return + end + + previous_branch = current_branch + begin + git "symbolic-ref", "HEAD", reference, dir: path + rescue GitError => e + Bundler.ui.debug "Could not repoint the cached clone at #{reference}: #{e.message}" + return + end + @current_branch = nil + Bundler.ui.warn "#{credential_filtered_uri} no longer has #{previous_branch}, " \ + "now following its default branch #{default_branch}" + out + end + + # The cached clone's HEAD branch is gone from the remote, so the remote's + # own idea of its default branch is the only thing left to follow. + def renamed_remote_default_branch + default_branch = remote_default_branch + return if default_branch.nil? || default_branch == current_branch + + default_branch + end + + def remote_default_branch + command = ["ls-remote", "--symref", "--", configured_uri, "HEAD"] + check_allowed(command) + + out, err, status = capture(command, path) + unless status.success? + Bundler.ui.debug "Could not ask #{credential_filtered_uri} for its default branch: #{err}" + return + end + + # A remote is free to advertise a ref name that is not valid UTF-8, and + # matching that as text raises out of the GitError family. + out.b[%r{^ref:\s+refs/heads/(.+?)\s+HEAD}, 1] + end + def pinned_to_full_sha? full_sha_revision?(ref) end @@ -465,8 +526,8 @@ def extra_clone_args args end - def fetch_command(args) - ["fetch", "--force", "--quiet", "--no-tags", *args, "--", configured_uri, refspec].compact + def fetch_command(args, spec = refspec) + ["fetch", "--force", "--quiet", "--no-tags", *args, "--", configured_uri, spec].compact end def clone_command(args) diff --git a/lib/rubygems/basic_specification.rb b/lib/rubygems/basic_specification.rb index 61d35307f4ea13..f1a04fc180e85d 100644 --- a/lib/rubygems/basic_specification.rb +++ b/lib/rubygems/basic_specification.rb @@ -182,17 +182,6 @@ def full_require_paths end end - ## - # The path to the data directory for this gem. - - def datadir - # TODO: drop the extra ", gem_name" which is uselessly redundant - File.expand_path(File.join(gems_dir, full_name, "data", name)) - end - - extend Gem::Deprecate - rubygems_deprecate :datadir, :none, "4.1" - ## # Full path of the target library file. # If the file is not in this gem, return nil. diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb index 494525d661af2b..ba2cc420c85832 100644 --- a/lib/rubygems/commands/push_command.rb +++ b/lib/rubygems/commands/push_command.rb @@ -48,7 +48,8 @@ def initialize end add_option("--attestation FILE", - "Push with sigstore attestations") do |value, options| + "Push with sigstore attestations", + " (FILE must be a JSON sigstore bundle)") do |value, options| options[:attestations] << value end @@ -96,7 +97,7 @@ def send_gem(name) def send_push_request(name, args) # Always honor explicit --attestation option # Auto-attestation is only supported on rubygems.org with GitHub Actions (not JRuby) - if options[:attestations].any? || (RUBY_ENGINE != "jruby" && attestation_supported_host? && ENV["GITHUB_ACTIONS"]) + if options[:attestations].any? || (RUBY_ENGINE != "jruby" && attestation_supported_host? && ENV["GITHUB_ACTIONS"] == "true") send_push_request_with_attestation(name, args) else send_push_request_without_attestation(name, args) @@ -117,14 +118,23 @@ def send_push_request_without_attestation(name, args) def send_push_request_with_attestation(name, args) attestations = if options[:attestations].any? options[:attestations].map do |attestation| - Gem.read_binary(attestation) + load_attestation(attestation) end else - bundle_path = attest!(name) + # Only the opportunistic signing step falls back. The request below stays + # outside this rescue because once the server may have seen the attested + # push, a network error must not trigger an unattested retry. begin - [Gem.read_binary(bundle_path)] - ensure - File.unlink(bundle_path) if bundle_path && File.exist?(bundle_path) + [attest!(name)] + rescue StandardError => e + message = "Failed to create an attestation, pushing without one.\n" + message += if Gem.configuration.really_verbose + e.full_message + else + e.message + end + alert_warning message + return send_push_request_without_attestation(name, args) end end bundles = "[" + attestations.join(",") + "]" @@ -136,15 +146,27 @@ def send_push_request_with_attestation(name, args) ], "multipart/form-data") request.add_field "Authorization", api_key end - rescue StandardError => e - message = "Failed to push with attestation, retrying without attestation.\n" - message += if Gem.configuration.really_verbose - e.full_message - else - e.message + end + + def load_attestation(file) + data = begin + Gem.read_binary(file) + rescue SystemCallError, IOError, ArgumentError => e + raise Gem::Exception, "Failed to read attestation #{file}: #{e.message}" + end + validate_attestation_json(data, file) + end + + def validate_attestation_json(data, source) + require "json" + + parsed = begin + JSON.parse(data) + rescue JSON::ParserError => e + raise Gem::Exception, "Attestation #{source} is not valid JSON: #{e.message}" end - alert_warning message - send_push_request_without_attestation(name, args) + raise Gem::Exception, "Attestation #{source} is not a JSON object" unless parsed.is_a?(Hash) + data end def attest!(name) @@ -152,22 +174,24 @@ def attest!(name) require "shellwords" require "tempfile" - tempfile = Tempfile.new([File.basename(name, ".*"), ".sigstore.json"]) - bundle = tempfile.path - tempfile.close(false) - env = defined?(Bundler.unbundled_env) ? Bundler.unbundled_env : ENV.to_h - # Gem.ruby is quoted if it contains whitespace, so split it into argv - # elements to keep the quotes out of the spawned command. - out, st = Open3.capture2e( - env, - *Shellwords.split(Gem.ruby), "-S", "gem", "exec", "--conservative", - "sigstore-cli", "sign", name, "--bundle", bundle, - unsetenv_others: true - ) - raise Gem::Exception, "Failed to sign gem:\n\n#{out}" unless st.success? - - bundle + + Tempfile.create([File.basename(name, ".*"), ".sigstore.json"]) do |tempfile| + tempfile.close + bundle = tempfile.path + + # Gem.ruby is quoted if it contains whitespace, so split it into argv + # elements to keep the quotes out of the spawned command. + out, st = Open3.capture2e( + env, + *Shellwords.split(Gem.ruby), "-S", "gem", "exec", "--conservative", + "sigstore-cli", "sign", name, "--bundle", bundle, + unsetenv_others: true + ) + raise Gem::Exception, "Failed to sign gem:\n\n#{out}" unless st.success? + + validate_attestation_json(Gem.read_binary(bundle), "generated by sigstore-cli") + end end def get_hosts_for(name) diff --git a/lib/rubygems/config_file.rb b/lib/rubygems/config_file.rb index e9cd2c6c355429..853de9e8ecd143 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 5b83dc6f6f2030..d3ab256029e851 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/lib/rubygems/safe_yaml.rb b/lib/rubygems/safe_yaml.rb index f4bba001365fea..6ecdd1d50b041e 100644 --- a/lib/rubygems/safe_yaml.rb +++ b/lib/rubygems/safe_yaml.rb @@ -35,7 +35,8 @@ def self.aliases_enabled? # :nodoc: end def self.safe_load(input) - if Gem.use_psych? + # Psych rejects legacy metadata bytes, so preserve them with the internal parser. + if Gem.use_psych? && valid_encoding?(input) ::Psych.safe_load(input, permitted_classes: PERMITTED_CLASSES, permitted_symbols: PERMITTED_SYMBOLS, aliases: @aliases_enabled) else @@ -51,5 +52,11 @@ def self.safe_load(input) class << self alias_method :load, :safe_load end + + private_class_method def self.valid_encoding?(input) + return true unless input.is_a?(String) + + input.dup.force_encoding(Encoding::UTF_8).valid_encoding? + end end end diff --git a/lib/time.rb b/lib/time.rb index 577d0060327212..b157e60863354e 100644 --- a/lib/time.rb +++ b/lib/time.rb @@ -510,7 +510,7 @@ def strptime(date, format, now=self.now) # require 'time' # # Time.rfc2822("Wed, 05 Oct 2011 22:26:12 -0400") - # #=> 2010-10-05 22:26:12 -0400 + # #=> 2011-10-05 22:26:12 -0400 # # You must require 'time' to use this method. # diff --git a/parse.y b/parse.y index 7a566681964070..78d3f4ab4c3baf 100644 --- a/parse.y +++ b/parse.y @@ -9885,6 +9885,7 @@ parse_numeric(struct parser_params *p, int c) type = tRATIONAL; } else { + errno = 0; strtod(tok(p), 0); if (errno == ERANGE) { rb_warning1("Float %s out of range", WARN_S(tok(p))); diff --git a/spec/bundler/bundler/cli/exec_spec.rb b/spec/bundler/bundler/cli/exec_spec.rb new file mode 100644 index 00000000000000..c1b0db6176c59e --- /dev/null +++ b/spec/bundler/bundler/cli/exec_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require "bundler/cli" +require "bundler/cli/exec" + +RSpec.describe Bundler::CLI::Exec do + subject(:command) { described_class.new(options, ["script"]) } + + let(:options) { double("options", keep_file_descriptors?: false) } + + before do + allow(Bundler.current_ruby).to receive(:jruby?).and_return(false) + allow(Bundler::SharedHelpers).to receive(:set_bundle_environment) + allow(Bundler).to receive(:settings).and_return({ disable_exec_load: true }) + end + + def expect_exec_path(path, expected_path) + allow(Bundler).to receive(:which).with("script").and_return(path) + expect(command).to receive(:kernel_exec).with(expected_path, kind_of(Hash)) + command.run + end + + it "preserves explicit relative paths using the primary path separator" do + path = ".#{File::SEPARATOR}script" + expect_exec_path(path, path) + end + + it "preserves parent-relative paths using the primary path separator" do + path = "..#{File::SEPARATOR}script" + expect_exec_path(path, path) + end + + it "preserves explicit relative paths using the alternative path separator" do + stub_const("File::ALT_SEPARATOR", "\\") + + expect_exec_path(".\\script", ".\\script") + end + + it "preserves parent-relative paths using the alternative path separator" do + stub_const("File::ALT_SEPARATOR", "\\") + + expect_exec_path("..\\script", "..\\script") + end + + it "prepends the primary path separator to other relative paths" do + expect_exec_path(".script", ".#{File::SEPARATOR}.script") + end + + it "treats a backslash as part of a filename when it is not a path separator" do + stub_const("File::ALT_SEPARATOR", nil) + + expect_exec_path(".\\script", ".#{File::SEPARATOR}.\\script") + end +end diff --git a/spec/bundler/bundler/source/git/git_proxy_spec.rb b/spec/bundler/bundler/source/git/git_proxy_spec.rb index 9633c910171043..760819c7e51ff4 100644 --- a/spec/bundler/bundler/source/git/git_proxy_spec.rb +++ b/spec/bundler/bundler/source/git/git_proxy_spec.rb @@ -438,6 +438,36 @@ end end + context "when the remote no longer has the branch HEAD points at" do + let(:cached_branch) { "main" } + let(:missing_ref) { ["", "fatal: couldn't find remote ref refs/heads/#{cached_branch}", fail_result] } + let(:symref_advertisement) { ["ref: refs/heads/renamed\tHEAD\n", "", clone_result] } + + before do + allow(git_proxy).to receive(:git_local).with("--version").and_return("git version 2.14.0") + allow(git_proxy).to receive(:git_local).with("rev-parse", "--abbrev-ref", "HEAD", dir: path).and_return(cached_branch) + allow(git_proxy).to receive(:capture).with([*base_fetch_args, "--", uri, "refs/heads/#{cached_branch}:refs/heads/#{cached_branch}"], path).and_return(missing_ref) + end + + it "follows the branch the remote now points HEAD at" do + expect(git_proxy).to receive(:capture).with(["ls-remote", "--symref", "--", uri, "HEAD"], path).and_return(symref_advertisement) + expect(git_proxy).to receive(:capture).with([*base_fetch_args, "--", uri, "refs/heads/renamed:refs/heads/renamed"], path).and_return(["", "", clone_result]) + expect(git_proxy).to receive(:git).with("symbolic-ref", "HEAD", "refs/heads/renamed", dir: path) + subject.checkout + end + + context "and a revision is locked" do + let(:revision) { Digest::SHA1.hexdigest("ruby") } + + it "does not ask the remote for its default branch" do + expect(git_proxy).to receive(:git).with("cat-file", "-e", revision, dir: path).and_raise(Bundler::GitError) + expect(git_proxy).to receive(:capture).with([*base_fetch_args, "--", uri, "#{revision}:refs/#{revision}-sha"], path).and_return(missing_ref) + expect(git_proxy).not_to receive(:capture).with(["ls-remote", "--symref", "--", uri, "HEAD"], path) + expect { subject.checkout }.to raise_error(Bundler::Source::Git::MissingGitRevisionError) + end + end + end + context "URI is HTTP" do let(:uri) { "http://github.com/ruby/rubygems.git" } diff --git a/spec/bundler/commands/exec_spec.rb b/spec/bundler/commands/exec_spec.rb index e7a294b42932fa..5a381e5073dde5 100644 --- a/spec/bundler/commands/exec_spec.rb +++ b/spec/bundler/commands/exec_spec.rb @@ -343,6 +343,32 @@ expect(out).to include(rubylib) end + it "does not duplicate a relative path prefix when exec'ing to a relative path" do + skip "https://github.com/ruby/rubygems/issues/3351" if Gem.win_platform? + + create_file("script", "#!/usr/bin/env ruby\nputs $0") + + install_gemfile <<-G + source "https://gem.repo1" + G + + bundle "exec ./script", env: { "BUNDLE_DISABLE_EXEC_LOAD" => "true" } + expect(out).to eq("./script") + end + + it "prepends a relative path prefix when exec'ing to a hidden file in the current directory" do + skip "https://github.com/ruby/rubygems/issues/3351" if Gem.win_platform? + + create_file(".script", "#!/usr/bin/env ruby\nputs $0") + + install_gemfile <<-G + source "https://gem.repo1" + G + + bundle "exec .script", env: { "BUNDLE_DISABLE_EXEC_LOAD" => "true" } + expect(out).to eq("./.script") + end + it "errors nicely when the argument doesn't exist" do install_gemfile <<-G source "https://gem.repo1" diff --git a/spec/bundler/install/global_cache_spec.rb b/spec/bundler/install/global_cache_spec.rb index 5e5494dbe9d349..ce0564ee785136 100644 --- a/spec/bundler/install/global_cache_spec.rb +++ b/spec/bundler/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/spec/bundler/support/shards.rb b/spec/bundler/support/shards.rb index ccadff75faa97e..93e1ccb9946275 100644 --- a/spec/bundler/support/shards.rb +++ b/spec/bundler/support/shards.rb @@ -144,6 +144,7 @@ module Shards "spec/bundler/ci_detector_spec.rb", ], shard_d: [ + "spec/bundler/cli/exec_spec.rb", "spec/bundler/rubygems_ext_spec.rb", "spec/bundler/resolver/cooldown_spec.rb", "spec/install/cooldown_spec.rb", diff --git a/spec/bundler/update/git_spec.rb b/spec/bundler/update/git_spec.rb index 17955672f542dc..853c07fcc04dcd 100644 --- a/spec/bundler/update/git_spec.rb +++ b/spec/bundler/update/git_spec.rb @@ -2,6 +2,11 @@ RSpec.describe "bundle update" do describe "git sources" do + def cached_head_for(name) + path = default_cache_path("git/#{name}-#{Digest(:SHA1).hexdigest(lib_path(name).to_s)}") + File.read(path.join("HEAD")).strip + end + it "floats on a branch when :branch is used" do build_git "foo", "1.0" update_git "foo", branch: "omg" @@ -22,6 +27,45 @@ expect(the_bundle).to include_gems "foo 1.1" end + it "updates a source with no :branch when its default branch was renamed" do + build_git "foo", "1.0" + + install_gemfile <<-G + source "https://gem.repo1" + gem "foo", :git => "#{lib_path("foo-1.0")}" + G + + git "branch -m renamed", lib_path("foo-1.0") + update_git "foo" do |s| + s.write "lib/foo.rb", "FOO = '1.1'" + end + + bundle "update", all: true + + expect(err).to include("no longer has main, now following its default branch renamed") + expect(the_bundle).to include_gems "foo 1.1" + end + + it "does not follow a renamed default branch when :branch is used" do + build_git "foo", "1.0" + + install_gemfile <<-G + source "https://gem.repo1" + gem "foo", :git => "#{lib_path("foo-1.0")}", :branch => "main" + G + + git "branch -m renamed", lib_path("foo-1.0") + update_git "foo" do |s| + s.write "lib/foo.rb", "FOO = '1.1'" + end + + bundle "update", all: true + + expect(err).to include("Revision main does not exist") + expect(the_bundle).to include_gems "foo 1.0" + expect(cached_head_for("foo-1.0")).to eq("ref: refs/heads/main") + end + it "updates correctly when you have like craziness" do build_lib "activesupport", "3.0", path: lib_path("rails/activesupport") build_git "rails", "3.0", path: lib_path("rails") do |s| diff --git a/string.c b/string.c index 09384e4470b1c7..e4503a96bf1f2d 100644 --- a/string.c +++ b/string.c @@ -9158,11 +9158,10 @@ tr_trans(VALUE str, VALUE src, VALUE repl, int sflag) SIZED_REALLOC_N(buf, unsigned char, max + termlen, old); t = buf + offset; } - if (s != t) { - rb_enc_mbcput(c, t, enc); - if (may_modify && memcmp(s, t, tlen) != 0) { - modify = 1; - } + + rb_enc_mbcput(c, t, enc); + if (may_modify && memcmp(s, t, tlen) != 0) { + modify = 1; } CHECK_IF_ASCII(c); s += clen; diff --git a/test/rubygems/helper.rb b/test/rubygems/helper.rb index 1fe8577b4fc94b..66e1e3e293459c 100644 --- a/test/rubygems/helper.rb +++ b/test/rubygems/helper.rb @@ -391,6 +391,7 @@ def setup ENV["XDG_STATE_HOME"] = nil ENV["MAKEFLAGS"] = nil ENV["SOURCE_DATE_EPOCH"] = nil + ENV["GITHUB_ACTIONS"] = nil ENV["BUNDLER_VERSION"] = nil ENV["BUNDLE_CONFIG"] = nil ENV["BUNDLE_USER_CONFIG"] = nil diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb index f8bb09d60062ff..904fdfcd8e87bf 100644 --- a/test/rubygems/test_gem_commands_push_command.rb +++ b/test/rubygems/test_gem_commands_push_command.rb @@ -106,7 +106,7 @@ def test_execute_attestation @response = "Successfully registered gem: freewill (1.0.0)" @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") - File.write("#{@path}.sigstore.json", "attestation") + File.write("#{@path}.sigstore.json", '{"attestation":true}') @cmd.options[:args] = [@path] @cmd.options[:attestations] = ["#{@path}.sigstore.json"] @@ -118,59 +118,166 @@ def test_execute_attestation assert_attestation_multipart Gem.read_binary("#{@path}.sigstore.json") end + def test_execute_attestation_multiple + @response = "Successfully registered gem: freewill (1.0.0)" + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") + + File.write("#{@path}.a.sigstore.json", '{"attestation":"a"}') + File.write("#{@path}.b.sigstore.json", '{"attestation":"b"}') + @cmd.options[:args] = [@path] + @cmd.options[:attestations] = ["#{@path}.a.sigstore.json", "#{@path}.b.sigstore.json"] + + @cmd.execute + + assert_attestation_multipart '{"attestation":"a"},{"attestation":"b"}' + end + def test_execute_attestation_auto omit if RUBY_ENGINE == "jruby" ENV["GITHUB_ACTIONS"] = "true" - begin - @response = "Successfully registered gem: freewill (1.0.0)" - @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") - attestation_path = "#{@path}.sigstore.json" - attestation_content = "auto-attestation" - File.write(attestation_path, attestation_content) - @cmd.options[:args] = [@path] + @response = "Successfully registered gem: freewill (1.0.0)" + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") + + attestation_content = '{"auto":"attestation"}' + @cmd.options[:args] = [@path] + + @cmd.stub(:attest!, attestation_content) do + @cmd.execute + end - @cmd.stub(:attest!, attestation_path) do + assert_equal Gem::Net::HTTP::Post, @fetcher.last_request.class + content_length = @fetcher.last_request["Content-Length"].to_i + assert_equal content_length, @fetcher.last_request.body.length + assert_attestation_multipart attestation_content + end + + def test_execute_attestation_fallback + omit if RUBY_ENGINE == "jruby" + + ENV["GITHUB_ACTIONS"] = "true" + + @response = "Successfully registered gem: freewill (1.0.0)" + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") + + @cmd.options[:args] = [@path] + + @cmd.stub(:attest!, proc { raise Gem::Exception, "boom" }) do + use_ui @ui do @cmd.execute end + end - assert_equal Gem::Net::HTTP::Post, @fetcher.last_request.class - content_length = @fetcher.last_request["Content-Length"].to_i - assert_equal content_length, @fetcher.last_request.body.length - assert_attestation_multipart attestation_content - ensure - ENV.delete("GITHUB_ACTIONS") + assert_match "Failed to create an attestation, pushing without one.", @ui.error + assert_equal Gem::Net::HTTP::Post, @fetcher.last_request.class + assert_equal Gem.read_binary(@path), @fetcher.last_request.body + assert_equal "application/octet-stream", + @fetcher.last_request["Content-Type"] + end + + def test_execute_attestation_explicit_missing_file + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: "", code: 200, msg: "OK") + + @cmd.options[:args] = [@path] + @cmd.options[:attestations] = ["#{@path}.sigstore.json"] + + e = assert_raise Gem::Exception do + use_ui @ui do + @cmd.execute + end end + + assert_match "Failed to read attestation", e.message + refute_match "pushing without one", @ui.error + assert_nil @fetcher.last_request end - def test_execute_attestation_fallback + def test_execute_attestation_explicit_invalid_json + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: "", code: 200, msg: "OK") + + File.write("#{@path}.sigstore.json", "not json") + @cmd.options[:args] = [@path] + @cmd.options[:attestations] = ["#{@path}.sigstore.json"] + + e = assert_raise Gem::Exception do + use_ui @ui do + @cmd.execute + end + end + + assert_match "is not valid JSON", e.message + refute_match "pushing without one", @ui.error + assert_nil @fetcher.last_request + end + + def test_execute_attestation_explicit_json_scalar + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: "", code: 200, msg: "OK") + + File.write("#{@path}.sigstore.json", "null") + @cmd.options[:args] = [@path] + @cmd.options[:attestations] = ["#{@path}.sigstore.json"] + + e = assert_raise Gem::Exception do + use_ui @ui do + @cmd.execute + end + end + + assert_match "is not a JSON object", e.message + assert_nil @fetcher.last_request + end + + def test_execute_attestation_network_error_not_retried_without_attestation omit if RUBY_ENGINE == "jruby" ENV["GITHUB_ACTIONS"] = "true" - begin - @response = "Successfully registered gem: freewill (1.0.0)" - @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") - @cmd.options[:args] = [@path] + requests = 0 + @fetcher.data["#{Gem.host}/api/v1/gems"] = proc do + requests += 1 + raise Gem::RemoteFetcher::FetchError.new("timed out", "#{Gem.host}/api/v1/gems") + end + + @cmd.options[:args] = [@path] - @cmd.stub(:attest!, proc { raise Gem::Exception, "boom" }) do + assert_raise Gem::RemoteFetcher::FetchError do + @cmd.stub(:attest!, '{"auto":"attestation"}') do use_ui @ui do @cmd.execute end end + end - assert_match "Failed to push with attestation, retrying without attestation.", @ui.error - assert_equal Gem::Net::HTTP::Post, @fetcher.last_request.class - assert_equal Gem.read_binary(@path), @fetcher.last_request.body - assert_equal "application/octet-stream", - @fetcher.last_request["Content-Type"] - ensure - ENV.delete("GITHUB_ACTIONS") + assert_equal 1, requests + refute_match "pushing without one", @ui.error + end + + def test_execute_attestation_auto_skipped_unless_github_actions_true + omit if RUBY_ENGINE == "jruby" + + ENV["GITHUB_ACTIONS"] = "false" + + @response = "Successfully registered gem: freewill (1.0.0)" + @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") + + @cmd.options[:args] = [@path] + + attest_called = false + @cmd.stub(:attest!, proc { attest_called = true }) do + @cmd.execute end + + refute attest_called, "attest! should not be called when GITHUB_ACTIONS is not \"true\"" + assert_equal "application/octet-stream", + @fetcher.last_request["Content-Type"] end def test_execute_attestation_skipped_on_non_rubygems_host + omit if RUBY_ENGINE == "jruby" + + ENV["GITHUB_ACTIONS"] = "true" + @spec, @path = util_gem "freebird", "1.0.1" do |spec| spec.metadata["allowed_push_host"] = "https://privategemserver.example" end @@ -193,6 +300,8 @@ def test_execute_attestation_skipped_on_non_rubygems_host end def test_execute_attestation_skipped_on_jruby + ENV["GITHUB_ACTIONS"] = "true" + @response = "Successfully registered gem: freewill (1.0.0)" @fetcher.data["#{Gem.host}/api/v1/gems"] = HTTPResponseFactory.create(body: @response, code: 200, msg: "OK") @@ -234,6 +343,7 @@ def fake_status.success? captured = nil capture_stub = lambda do |*args, **_kwargs| captured = args + File.write(args[args.index("--bundle") + 1], "{}") ["", fake_status] end Gem.stub(:ruby, '"/path with space/bin/ruby"') do @@ -249,6 +359,77 @@ def fake_status.success? assert_equal "-S", captured[2] end + def test_attest_aborts_when_signing_fails + require "open3" + + fake_status = Object.new + def fake_status.success? + false + end + + bundle_path = nil + capture_stub = lambda do |*args, **_kwargs| + bundle_path = args[args.index("--bundle") + 1] + ["sigstore-cli: no identity token available", fake_status] + end + + e = assert_raise Gem::Exception do + Open3.stub(:capture2e, capture_stub) do + @cmd.send(:attest!, @path) + end + end + + assert_match "Failed to sign gem", e.message + assert_match "no identity token available", e.message + refute_nil bundle_path, "signing command should have been spawned" + refute File.exist?(bundle_path), "bundle tempfile should be removed" + end + + def test_attest_rejects_a_bundle_that_is_not_json + require "open3" + + fake_status = Object.new + def fake_status.success? + true + end + + capture_stub = lambda do |*args, **_kwargs| + File.write(args[args.index("--bundle") + 1], "not json") + ["", fake_status] + end + + e = assert_raise Gem::Exception do + Open3.stub(:capture2e, capture_stub) do + @cmd.send(:attest!, @path) + end + end + + assert_match "is not valid JSON", e.message + end + + def test_attest_returns_bundle_content_and_removes_tempfile + require "open3" + + fake_status = Object.new + def fake_status.success? + true + end + + bundle_path = nil + capture_stub = lambda do |*args, **_kwargs| + bundle_path = args[args.index("--bundle") + 1] + File.write(bundle_path, '{"signed":true}') + ["", fake_status] + end + + content = Open3.stub(:capture2e, capture_stub) do + @cmd.send(:attest!, @path) + end + + assert_equal '{"signed":true}', content + refute File.exist?(bundle_path), "bundle tempfile should be removed" + end + def test_execute_allowed_push_host @spec, @path = util_gem "freebird", "1.0.1" do |spec| spec.metadata["allowed_push_host"] = "https://privategemserver.example" diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index 7b8ac4736d2f46..fdc0c22f45ab1b 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -944,6 +944,26 @@ def entry.full_name assert_equal @spec, spec end + def test_load_spec_from_metadata_with_legacy_encodings + { + "Based on Mauricio Fern\u00E1ndez's implementation" => Encoding::ISO_8859_1, + "\u65E5\u672C\u8A9E" => Encoding::EUC_JP, + }.each do |description, encoding| + @spec.description = description + metadata = @spec.to_yaml.encode encoding + entry = StringIO.new Gem::Util.gzip metadata + def entry.full_name + "metadata.gz" + end + + package = Gem::Package.new "nonexistent.gem" + spec = package.load_spec_from_metadata entry + + assert_equal @spec, spec + assert_equal description.encode(encoding).b, spec.description.b + end + end + def test_verify package = Gem::Package.new @gem diff --git a/test/rubygems/test_gem_remote_fetcher.rb b/test/rubygems/test_gem_remote_fetcher.rb index c35da2fc5ae273..2c599e00986b90 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 diff --git a/vm.c b/vm.c index d1084a851bfb4b..5454bff570e974 100644 --- a/vm.c +++ b/vm.c @@ -470,14 +470,14 @@ static VALUE vm_invoke_proc(rb_execution_context_t *ec, rb_proc_t *proc, VALUE s #if USE_YJIT // Counter to serve as a proxy for execution time, total number of calls -static uint64_t yjit_total_entry_hits = 0; +static unsigned int yjit_total_entry_hits = 0; // Number of calls used to estimate how hot an ISEQ is #define YJIT_CALL_COUNT_INTERV 20u /// Test whether we are ready to compile an ISEQ or not static inline bool -rb_yjit_threshold_hit(const rb_iseq_t *iseq, uint64_t entry_calls) +rb_yjit_threshold_hit(const rb_iseq_t *iseq, unsigned int entry_calls) { yjit_total_entry_hits += 1; @@ -494,7 +494,7 @@ rb_yjit_threshold_hit(const rb_iseq_t *iseq, uint64_t entry_calls) return true; } - uint64_t num_calls = yjit_total_entry_hits - ISEQ_BODY(iseq)->yjit_calls_at_interv; + unsigned int num_calls = yjit_total_entry_hits - ISEQ_BODY(iseq)->yjit_calls_at_interv; // Reject ISEQs that don't get called often enough if (num_calls > rb_yjit_cold_threshold) { diff --git a/vm_core.h b/vm_core.h index a1e6bfac431ff6..8843b7ac124ead 100644 --- a/vm_core.h +++ b/vm_core.h @@ -566,20 +566,20 @@ struct rb_iseq_constant_body { const rb_iseq_t *mandatory_only_iseq; #if USE_YJIT || USE_ZJIT + // Number of calls on jit_exec() + unsigned int jit_entry_calls; + // Number of calls on jit_exec_exception() + unsigned int jit_exception_calls; // Function pointer for JIT code on jit_exec() rb_jit_func_t jit_entry; - // Number of calls on jit_exec() - long unsigned jit_entry_calls; // Function pointer for JIT code on jit_exec_exception() rb_jit_func_t jit_exception; - // Number of calls on jit_exec_exception() - long unsigned jit_exception_calls; void *jit_payload; #endif #if USE_YJIT // Used to estimate how frequently this ISEQ gets called - uint64_t yjit_calls_at_interv; + unsigned int yjit_calls_at_interv; #endif // Hash of the source this iseq was compiled from, or 0 if it is diff --git a/vm_insnhelper.c b/vm_insnhelper.c index ea4deb17727959..0542221f58d33e 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1881,6 +1881,8 @@ rb_vm_throw(const rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_nu return vm_throw(ec, reg_cfp, throw_state, throwobj); } +NORETURN(VALUE rb_zjit_throw(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_num_t throw_state, VALUE throwobj)); + // Fallback for ZJIT. Make a longjmp and unwind to the most recent vm_exec(). VALUE rb_zjit_throw(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_num_t throw_state, VALUE throwobj) diff --git a/yjit.h b/yjit.h index 93acf4e60d1dd9..772ccc5b1b5ede 100644 --- a/yjit.h +++ b/yjit.h @@ -24,8 +24,8 @@ #endif // Expose these as declarations since we are building YJIT. -extern uint64_t rb_yjit_call_threshold; -extern uint64_t rb_yjit_cold_threshold; +extern unsigned int rb_yjit_call_threshold; +extern unsigned int rb_yjit_cold_threshold; extern uint64_t rb_yjit_live_iseq_count; extern uint64_t rb_yjit_iseq_alloc_count; extern bool rb_yjit_enabled_p; diff --git a/yjit/src/options.rs b/yjit/src/options.rs index c87a436091279f..0f24bfdddfa29d 100644 --- a/yjit/src/options.rs +++ b/yjit/src/options.rs @@ -3,10 +3,10 @@ use crate::{backend::current::TEMP_REGS, cruby::*, stats::Counter}; use std::os::raw::{c_char, c_int, c_uint}; // Call threshold for small deployments and command-line apps -pub static SMALL_CALL_THRESHOLD: u64 = 30; +pub static SMALL_CALL_THRESHOLD: u32 = 30; // Call threshold for larger deployments and production-sized applications -pub static LARGE_CALL_THRESHOLD: u64 = 120; +pub static LARGE_CALL_THRESHOLD: u32 = 120; // Number of live ISEQs after which we consider an app to be large pub static LARGE_ISEQ_COUNT: u64 = 40_000; @@ -15,13 +15,13 @@ pub static LARGE_ISEQ_COUNT: u64 = 40_000; // Number of method calls after which to start generating code // Threshold==1 means compile on first execution #[no_mangle] -pub static mut rb_yjit_call_threshold: u64 = SMALL_CALL_THRESHOLD; +pub static mut rb_yjit_call_threshold: u32 = SMALL_CALL_THRESHOLD; // This option is exposed to the C side in a global variable for performance, see vm.c // Number of execution requests after which a method is no longer // considered hot. Raising this results in more generated code. #[no_mangle] -pub static mut rb_yjit_cold_threshold: u64 = 200_000; +pub static mut rb_yjit_cold_threshold: u32 = 200_000; // Command-line options #[derive(Debug)] diff --git a/yjit/src/yjit.rs b/yjit/src/yjit.rs index 33e4308b2999af..2a48c94de6f4c1 100644 --- a/yjit/src/yjit.rs +++ b/yjit/src/yjit.rs @@ -238,7 +238,7 @@ pub extern "C" fn rb_yjit_enable(_ec: EcPtr, _ruby_self: VALUE, gen_stats: VALUE if !call_threshold.nil_p() { let threshold = call_threshold.as_isize() >> 1; unsafe { - rb_yjit_call_threshold = threshold as u64; + rb_yjit_call_threshold = threshold as u32; } } diff --git a/zjit.h b/zjit.h index f7d84e2a1157aa..a81011c6b572f0 100644 --- a/zjit.h +++ b/zjit.h @@ -109,8 +109,8 @@ ZJIT_STACK_MAP_BASE_PTR_STACK_SIZE(VALUE entry) extern void *rb_zjit_entry; extern bool rb_zjit_compiling_p; extern const zjit_jit_frame_t rb_zjit_c_frame; -extern uint64_t rb_zjit_call_threshold; -extern uint64_t rb_zjit_profile_threshold; +extern unsigned int rb_zjit_call_threshold; +extern unsigned int rb_zjit_profile_threshold; void rb_zjit_compile_iseq(const rb_iseq_t *iseq, rb_execution_context_t *ec, bool jit_exception); void rb_zjit_profile_insn(uint32_t insn, rb_execution_context_t *ec); void rb_zjit_profile_enable(const rb_iseq_t *iseq); diff --git a/zjit/src/asm/mod.rs b/zjit/src/asm/mod.rs index d45e323253fbff..c0c03ba0ca0685 100644 --- a/zjit/src/asm/mod.rs +++ b/zjit/src/asm/mod.rs @@ -249,12 +249,13 @@ impl CodeBlock { // Keep track of the reference self.label_refs.push(LabelRef { pos: self.write_pos, label, num_bytes, encode: Box::new(encode) }); - // Move past however many bytes the instruction takes up - if self.write_pos + num_bytes < self.mem_size { - self.write_pos += num_bytes; - } else { - self.dropped_bytes = true; // retry emitting the Insn after next_page - } + // Move past however many bytes the instruction takes up. + // Reserve the bytes by writing them rather than by moving the cursor + // over them. Pages are mapped on first write, so a cursor bump alone + // leaves a page that cannot be mapped. + const RESERVED: [u8; 16] = [0; 16]; + assert!(num_bytes <= RESERVED.len(), "label reference wants {num_bytes} bytes"); + self.write_bytes(&RESERVED[..num_bytes]); } // Link internal label references @@ -470,4 +471,39 @@ mod tests assert_eq!(uimm_num_bits((u32::MAX as u64) + 1), 64); assert_eq!(uimm_num_bits(u64::MAX), 64); } + + #[test] + fn test_label_ref_at_an_unmappable_page_sets_dropped_bytes() { + // Two pages of address space, but a memory limit that only lets + // the first page be mapped. + let page_size = unsafe { crate::cruby::rb_jit_get_page_size() } as usize; + let limit = crate::stats::zjit_alloc_bytes() + page_size + page_size / 2; + let virt_mem = VirtualMem::alloc(2 * page_size, Some(limit)); + let mut cb = CodeBlock::new(Rc::new(RefCell::new(virt_mem)), false); + + // Fill the mappable page up to 2 bytes below its end, so that + // a 5-byte jump reservation straddles into the unmappable page. + for _ in 0..(page_size - 2) { + cb.write_byte(0x90); + } + assert!(!cb.has_dropped_bytes(), "the first page must map"); + + // label_ref() must reserve its bytes by writing them, so that the + // unmappable page surfaces as dropped_bytes here, where the caller + // still reads it and turns it into CompileError::OutOfMemory. + let label = cb.new_label("over_the_page_boundary".to_string()); + cb.label_ref(label, 5, |cb, _, _| { + cb.write_bytes(&[0; 5]); + Ok(()) + }); + cb.write_label(label); + + // Mirror what compile_with_regs does after emit. It calls link_labels() + // only if emit did not go OOM. This reproduces an assertion failure in + // link_labels() when dropped_bytes isn't updated properly. + if !cb.has_dropped_bytes() { + cb.link_labels().unwrap(); + } + assert!(cb.has_dropped_bytes(), "the reservation must discover the unmappable page"); + } } diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index afc0e10bc46c2d..44c5da93f37dab 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -8058,7 +8058,7 @@ impl<'a> std::fmt::Display for FunctionPrinter<'a> { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct FrameState { pub iseq: IseqPtr, insn_idx: YarvInsnIdx, @@ -8177,14 +8177,14 @@ impl FrameState { /// Pop a stack operand fn stack_pop(&mut self) -> Result { - self.stack.pop().ok_or_else(|| ParseError::StackUnderflow(self.clone())) + self.stack.pop().ok_or_else(|| ParseError::StackUnderflow(self.insn_idx)) } fn stack_pop_n(&mut self, count: usize) -> Result, ParseError> { // Check if we have enough values on the stack let stack_len = self.stack.len(); if stack_len < count { - return Err(ParseError::StackUnderflow(self.clone())); + return Err(ParseError::StackUnderflow(self.insn_idx)); } Ok(self.stack.split_off(stack_len - count)) @@ -8192,7 +8192,7 @@ impl FrameState { /// Get a stack-top operand fn stack_top(&self) -> Result { - self.stack.last().ok_or_else(|| ParseError::StackUnderflow(self.clone())).copied() + self.stack.last().ok_or_else(|| ParseError::StackUnderflow(self.insn_idx)).copied() } /// Set a stack operand at idx @@ -8204,9 +8204,9 @@ impl FrameState { /// Get a stack operand at idx fn stack_topn(&self, idx: usize) -> Result { let Some(idx) = self.stack.len().checked_sub(idx + 1) else { - return Err(ParseError::StackUnderflow(self.clone())); + return Err(ParseError::StackUnderflow(self.insn_idx)); }; - self.stack.get(idx).ok_or_else(|| ParseError::StackUnderflow(self.clone())).copied() + self.stack.get(idx).ok_or_else(|| ParseError::StackUnderflow(self.insn_idx)).copied() } fn setlocal(&mut self, ep_offset: u32, opnd: InsnId) { @@ -8327,7 +8327,8 @@ pub enum CallType { #[derive(Clone, Debug, PartialEq)] pub enum ParseError { - StackUnderflow(FrameState), + /// Instruction index of the YARV instruction that underflowed the stack. + StackUnderflow(YarvInsnIdx), MalformedIseq(u32), // insn_idx into iseq_encoded Validation(ValidationError), NotAllowed, diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 7d0c35244fdb83..d2adf0478684c6 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -18118,15 +18118,16 @@ mod hir_opt_tests { #[test] fn test_specialize_inlined_megamorphic_receiver() { - set_call_threshold(6); + assert_eq!(crate::profile::DISTRIBUTION_SIZE, 8, "If you change distribution size, update the number of classes used"); + set_call_threshold((crate::profile::DISTRIBUTION_SIZE + 2).try_into().unwrap()); eval(" def klass_eq(klass) = klass == Integer def test = klass_eq(String) - # 5 distinct receiver classes at the == site: one more than the profile's - # 4 buckets, so the distribution is megamorphic. - klass_eq(Integer); klass_eq(Array); klass_eq(Hash); klass_eq(Symbol); klass_eq(Float) + # 9 distinct receiver classes at the == site: one more than the profile's + # 8 buckets, so the distribution is megamorphic. + klass_eq(Integer); klass_eq(Array); klass_eq(Hash); klass_eq(Symbol); klass_eq(Float); klass_eq(NilClass); klass_eq(TrueClass); klass_eq(FalseClass); klass_eq(String) 6.times { test } "); assert_snapshot!(hir_string("test"), @" @@ -18148,11 +18149,11 @@ mod hir_opt_tests { PatchPoint StableConstantNames(0x1068, Integer) v31:ClassSubclass[Integer@0x1070] = Const Value(VALUE(0x1070)) PatchPoint MethodRedefined(Class@0x1078, ==@0x1080, cme:0x1088) - v45:CBool = IsBitEqual v12, v31 - v46:BoolExact = BoxBool v45 + v82:CBool = IsBitEqual v12, v31 + v83:BoolExact = BoxBool v82 CheckInterrupts PopInlineFrame - Return v46 + Return v83 "); } diff --git a/zjit/src/options.rs b/zjit/src/options.rs index 3529bbc8c143e7..27f2fa90762daf 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -25,7 +25,7 @@ pub type NumProfiles = u16; /// Default --zjit-call-threshold. This should be large enough to avoid compiling /// warmup code, but small enough to perform well on micro-benchmarks. pub const DEFAULT_CALL_THRESHOLD: CallThreshold = 30; -pub type CallThreshold = u64; +pub type CallThreshold = u32; /// Default --zjit-inline-threshold /// TODO (nirvdrum 2026-06-25): 30 has proven to work well with ruby-bench, but we should finely diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index 38cb44576aa4c1..f4a69d3afec3a0 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -143,7 +143,7 @@ pub fn num_arguments_on_stack(cd: *const rb_call_data) -> usize { (unsafe { vm_ci_argc(ci) }) as usize + has_blockarg as usize } -const DISTRIBUTION_SIZE: usize = 8; +pub const DISTRIBUTION_SIZE: usize = 8; pub type TypeDistribution = Distribution;