[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

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"