From 1935433f5f6fecc786955dcf80e8f65ada6bcaeb Mon Sep 17 00:00:00 2001 From: Samuel Giddins Date: Thu, 17 Aug 2023 15:38:53 -0700 Subject: [PATCH] [rubygems/rubygems] Ensure that loading multiple gemspecs with legacy YAML class references does not warn Before this, you would get constant redefinition warnings on Psych::DefaultKey Additionally, ensure the retries wont continue infinitely in the case of the ArgumentError not being caused by Marshal trying to load the undefined classes https://github.com/rubygems/rubygems/commit/919e8c2de4 --- lib/rubygems/specification.rb | 15 +++++++++++++-- test/rubygems/test_gem_specification.rb | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb index 3f132b6f2b..005d2ab797 100644 --- a/lib/rubygems/specification.rb +++ b/lib/rubygems/specification.rb @@ -1302,10 +1302,20 @@ class Gem::Specification < Gem::BasicSpecification Gem.load_yaml yaml_set = false + retry_count = 0 array = begin Marshal.load str rescue ArgumentError => e + # Avoid an infinite retry loop when the argument error has nothing to do + # with the classes not being defined. + # 1 retry each allowed in case all 3 of + # - YAML + # - YAML::Syck::DefaultKey + # - YAML::Private type + # need to be defined + raise if retry_count >= 3 + # # Some very old marshaled specs included references to `YAML::PrivateType` # and `YAML::Syck::DefaultKey` constants due to bugs in the old emitter @@ -1323,11 +1333,12 @@ class Gem::Specification < Gem::BasicSpecification if message.include?("YAML::Syck::") YAML.const_set "Syck", YAML unless YAML.const_defined?(:Syck) - YAML::Syck.const_set "DefaultKey", Class.new if message.include?("YAML::Syck::DefaultKey") - elsif message.include?("YAML::PrivateType") + YAML::Syck.const_set "DefaultKey", Class.new if message.include?("YAML::Syck::DefaultKey") && !YAML::Syck.const_defined?(:DefaultKey) + elsif message.include?("YAML::PrivateType") && !YAML.const_defined?(:PrivateType) YAML.const_set "PrivateType", Class.new end + retry_count += 1 retry ensure Object.__send__(:remove_const, "YAML") if yaml_set diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb index 9e68c40b7c..d37e1d1571 100644 --- a/test/rubygems/test_gem_specification.rb +++ b/test/rubygems/test_gem_specification.rb @@ -1094,6 +1094,25 @@ dependencies: [] assert_equal(yaml_defined, Object.const_defined?("YAML")) end + def test_handles_dependencies_with_other_syck_requirements_argument_error + yaml_defined = Object.const_defined?("YAML") + + data = Marshal.dump(Gem::Specification.new do |s| + v = Gem::Version.allocate + v.instance_variable_set :@version, "YAML::Syck::DefaultKey" + s.instance_variable_set :@version, v + end) + + assert_raises(ArgumentError) { Marshal.load(data) } + out, err = capture_output do + assert_raises(ArgumentError) { Marshal.load(data) } + end + assert_empty out + assert_empty err + + assert_equal(yaml_defined, Object.const_defined?("YAML")) + end + def test_initialize spec = Gem::Specification.new do |s| s.name = "blah"