From 88e46cf6b83e2ca701cf122756fef9bc2550e845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Thu, 5 Dec 2019 19:03:20 +0100 Subject: [PATCH] [rubygems/rubygems] Remove all `syck` traces from `rubygems` After reading [this blog post](https://blog.rubygems.org/2011/08/31/shaving-the-yaml-yak.html), published almost 10 years ago already, my understanding is that this problem could come up in two ways: * Rubygems.org serving corrupted gemspecs". As far as I understand this was fixed in rubygems.org a lot time ago, since https://github.com/rubygems/rubygems.org/pull/331. * Clients having a ten years old gemspec cache with some of these bad gemspecs. In this case, there's no easy solution but I think ten years is enough and rebuilding the cache should do the trick. So, I think it's time we remove this. https://github.com/rubygems/rubygems/commit/afcb15d556 --- lib/rubygems.rb | 10 -- lib/rubygems/requirement.rb | 23 +--- lib/rubygems/safe_yaml.rb | 2 - lib/rubygems/specification.rb | 6 - lib/rubygems/syck_hack.rb | 77 ----------- test/rubygems/data/null-type.gemspec.rz | Bin 554 -> 504 bytes test/rubygems/test_gem_specification.rb | 164 ------------------------ 7 files changed, 1 insertion(+), 281 deletions(-) delete mode 100644 lib/rubygems/syck_hack.rb diff --git a/lib/rubygems.rb b/lib/rubygems.rb index 1c5a25fd7f..60a599e380 100644 --- a/lib/rubygems.rb +++ b/lib/rubygems.rb @@ -628,12 +628,6 @@ An Array (#{env.inspect}) was passed in from #{caller[3]} rescue ::LoadError # If we can't load psych, that's fine, go on. else - # If 'yaml' has already been required, then we have to - # be sure to switch it over to the newly loaded psych. - if defined?(YAML::ENGINE) && YAML::ENGINE.yamler != "psych" - YAML::ENGINE.yamler = "psych" - end - require 'rubygems/psych_additions' require 'rubygems/psych_tree' end @@ -641,10 +635,6 @@ An Array (#{env.inspect}) was passed in from #{caller[3]} require 'yaml' require 'rubygems/safe_yaml' - # Now that we're sure some kind of yaml library is loaded, pull - # in our hack to deal with Syck's DefaultKey ugliness. - require 'rubygems/syck_hack' - @yaml_loaded = true end diff --git a/lib/rubygems/requirement.rb b/lib/rubygems/requirement.rb index 6721de4055..0067f6304a 100644 --- a/lib/rubygems/requirement.rb +++ b/lib/rubygems/requirement.rb @@ -194,24 +194,17 @@ class Gem::Requirement end def marshal_dump # :nodoc: - fix_syck_default_key_in_requirements - [@requirements] end def marshal_load(array) # :nodoc: @requirements = array[0] - - fix_syck_default_key_in_requirements end def yaml_initialize(tag, vals) # :nodoc: vals.each do |ivar, val| instance_variable_set "@#{ivar}", val end - - Gem.load_yaml - fix_syck_default_key_in_requirements end def init_with(coder) # :nodoc: @@ -246,8 +239,7 @@ class Gem::Requirement def satisfied_by?(version) raise ArgumentError, "Need a Gem::Version: #{version.inspect}" unless Gem::Version === version - # #28965: syck has a bug with unquoted '=' YAML.loading as YAML::DefaultKey - requirements.all? {|op, rv| (OPS[op] || OPS["="]).call version, rv } + requirements.all? {|op, rv| OPS[op].call version, rv } end alias :=== :satisfied_by? @@ -289,19 +281,6 @@ class Gem::Requirement def _tilde_requirements @_tilde_requirements ||= _sorted_requirements.select {|r| r.first == "~>" } end - - private - - def fix_syck_default_key_in_requirements # :nodoc: - Gem.load_yaml - - # Fixup the Syck DefaultKey bug - @requirements.each do |r| - if r[0].kind_of? Gem::SyckDefaultKey - r[0] = "=" - end - end - end end class Gem::Version diff --git a/lib/rubygems/safe_yaml.rb b/lib/rubygems/safe_yaml.rb index 29312ad5a1..e905052e1c 100644 --- a/lib/rubygems/safe_yaml.rb +++ b/lib/rubygems/safe_yaml.rb @@ -17,8 +17,6 @@ module Gem Gem::Specification Gem::Version Gem::Version::Requirement - YAML::Syck::DefaultKey - Syck::DefaultKey ].freeze PERMITTED_SYMBOLS = %w[ diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb index 14226796f1..ff345bf7a3 100644 --- a/lib/rubygems/specification.rb +++ b/lib/rubygems/specification.rb @@ -1690,12 +1690,6 @@ class Gem::Specification < Gem::BasicSpecification when String then if DateTimeFormat =~ date Time.utc($1.to_i, $2.to_i, $3.to_i) - - # Workaround for where the date format output from psych isn't - # parsed as a Time object by syck and thus comes through as a - # string. - elsif /\A(\d{4})-(\d{2})-(\d{2}) \d{2}:\d{2}:\d{2}\.\d+?Z\z/ =~ date - Time.utc($1.to_i, $2.to_i, $3.to_i) else raise(Gem::InvalidSpecificationException, "invalid date format in specification: #{date.inspect}") diff --git a/lib/rubygems/syck_hack.rb b/lib/rubygems/syck_hack.rb deleted file mode 100644 index 051483eac8..0000000000 --- a/lib/rubygems/syck_hack.rb +++ /dev/null @@ -1,77 +0,0 @@ -# frozen_string_literal: true -# :stopdoc: - -# Hack to handle syck's DefaultKey bug -# -# This file is always loaded AFTER either syck or psych are already -# loaded. It then looks at what constants are available and creates -# a consistent view on all rubys. -# -# All this is so that there is always a YAML::Syck::DefaultKey -# class no matter if the full yaml library has loaded or not. -# - -module YAML # :nodoc: - # In newer 1.9.2, there is a Syck toplevel constant instead of it - # being underneath YAML. If so, reference it back under YAML as - # well. - if defined? ::Syck - # for tests that change YAML::ENGINE - # 1.8 does not support the second argument to const_defined? - remove_const :Syck rescue nil - - Syck = ::Syck - - # JRuby's "Syck" is called "Yecht" - elsif defined? YAML::Yecht - Syck = YAML::Yecht - - # Otherwise, if there is no YAML::Syck, then we've got just psych - # loaded, so lets define a stub for DefaultKey. - elsif !defined? YAML::Syck - module Syck - class DefaultKey # :nodoc: - end - end - end - - # Now that we've got something that is always here, define #to_s - # so when code tries to use this, it at least just shows up like it - # should. - module Syck - class DefaultKey - remove_method :to_s rescue nil - - def to_s - '=' - end - end - end - - SyntaxError = Error unless defined? SyntaxError -end - -# Sometime in the 1.9 dev cycle, the Syck constant was moved from under YAML -# to be a toplevel constant. So gemspecs created under these versions of Syck -# will have references to Syck::DefaultKey. -# -# So we need to be sure that we reference Syck at the toplevel too so that -# we can always load these kind of gemspecs. -# -if !defined?(Syck) - Syck = YAML::Syck -end - -# Now that we've got Syck setup in all the right places, store -# a reference to the DefaultKey class inside Gem. We do this so that -# if later on YAML, etc are redefined, we've still got a consistent -# place to find the DefaultKey class for comparison. - -module Gem - # for tests that change YAML::ENGINE - remove_const :SyckDefaultKey if const_defined? :SyckDefaultKey - - SyckDefaultKey = YAML::Syck::DefaultKey -end - -# :startdoc: diff --git a/test/rubygems/data/null-type.gemspec.rz b/test/rubygems/data/null-type.gemspec.rz index bad99289d1b8a4f4d7f0ebd1b7cb4e8d210f9203..2134fcde29d9e2c44f289ca28be67acf8b26dcab 100644 GIT binary patch literal 504 zcmVNd|JJrbpy84x{n?YK`uB0Pp2Mb43zSW0_m0Ncuejt<=?@D;Sv%Mdg9o(i*}!%je|} zw)AZSe2TllUeF9~^@CP;xW?S65hDK^(!+j<^wvl{;lj4Dm2s}shwJ*y zDLyId-){7CbrBzLBZ{ngB{5tRjj7>r1oFQveb`J***_*Atq2wnr&$s4ik$tln$gd2 z3DfQKoGkNduaInjD#f1B!CK9(FD&mJvox{Bw}(K5HdpQZl;Anr}XQT&?r2ZRQw+av-b4Xzy37< literal 554 zcmV+_0@eL^oQ;#)Zqq;zhApUaa1Nv#TB#Mt+8ZQ9HclHVSc}S)PyqoFY7(R>NY%!k zG)wKZyN9&C`Ovo^WfX!Gy6-)S?Uw7AP zlTBpz@eAy-8rqVH=3x*eI_%Owu?15M=zKyP?HL>T|B_9CiM}Eq4o2}_IJyC z=;;u|f(78H#Gyd|QaW0Ka7rmWCXWiOBRJPgKnz?>D&|yCb^rC)vvTo1euevEuln~z z#YJ_Yr`e>PkcWlJ8%TkNz=O;tTmE8M--#%-<+V&gwvP`!eKZba!C6kusE*VJ$gHJ! z+r>LLZ>vw%{XZ0ZhuA)+F`$N;3pP`vgsJlneV zdw_Qh^Ug5!Z*m19_iE1&M@5!Qo<}a9KA&8ZLS3PxslRnqVmG}7=MSryXs6+P90p$C`UbQwgr zjv~@3CVj#}RL}p>ldD?HtMkjf!9fz6Htr9Ba%Uufh8HwL8wU|wnt_{T7_vA95uxY* sE=##5PBL?7u{;=c=SroL^m?aE&Gj@-&@1!xN@(Oio>;#DxAGAf3YUurv;Y7A diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb index 20509c8f3d..88afa3faa0 100644 --- a/test/rubygems/test_gem_specification.rb +++ b/test/rubygems/test_gem_specification.rb @@ -107,8 +107,6 @@ end end @current_version = Gem::Specification::CURRENT_SPECIFICATION_VERSION - - load 'rubygems/syck_hack.rb' end def test_self_find_active_stub_by_path @@ -747,125 +745,6 @@ end spec.specification_version end - def test_self_from_yaml_syck_date_bug - # This is equivalent to (and totally valid) psych 1.0 output and - # causes parse errors on syck. - yaml = @a1.to_yaml - yaml.sub!(/^date:.*/, "date: 2011-04-26 00:00:00.000000000Z") - - spec = with_syck do - Gem::Specification.from_yaml yaml - end - - assert_kind_of Time, @a1.date - assert_kind_of Time, spec.date - end - - def test_self_from_yaml_syck_default_key_bug - # This is equivalent to (and totally valid) psych 1.0 output and - # causes parse errors on syck. - yaml = <<-YAML ---- !ruby/object:Gem::Specification -name: posix-spawn -version: !ruby/object:Gem::Version - version: 0.3.6 - prerelease: -dependencies: -- !ruby/object:Gem::Dependency - name: rake-compiler - requirement: &70243867725240 !ruby/object:Gem::Requirement - none: false - requirements: - - - = - - !ruby/object:Gem::Version - version: 0.7.6 - type: :development - prerelease: false - version_requirements: *70243867725240 -platform: ruby -files: [] -test_files: [] -bindir: - YAML - - spec = with_syck do - Gem::Specification.from_yaml yaml - end - - op = spec.dependencies.first.requirement.requirements.first.first - refute_kind_of YAML::Syck::DefaultKey, op - - refute_match %r{DefaultKey}, spec.to_ruby - end - - def test_self_from_yaml_cleans_up_defaultkey - yaml = <<-YAML ---- !ruby/object:Gem::Specification -name: posix-spawn -version: !ruby/object:Gem::Version - version: 0.3.6 - prerelease: -dependencies: -- !ruby/object:Gem::Dependency - name: rake-compiler - requirement: &70243867725240 !ruby/object:Gem::Requirement - none: false - requirements: - - - !ruby/object:YAML::Syck::DefaultKey {} - - - !ruby/object:Gem::Version - version: 0.7.6 - type: :development - prerelease: false - version_requirements: *70243867725240 -platform: ruby -files: [] -test_files: [] -bindir: - YAML - - spec = Gem::Specification.from_yaml yaml - - op = spec.dependencies.first.requirement.requirements.first.first - refute_kind_of YAML::Syck::DefaultKey, op - - refute_match %r{DefaultKey}, spec.to_ruby - end - - def test_self_from_yaml_cleans_up_defaultkey_from_newer_192 - yaml = <<-YAML ---- !ruby/object:Gem::Specification -name: posix-spawn -version: !ruby/object:Gem::Version - version: 0.3.6 - prerelease: -dependencies: -- !ruby/object:Gem::Dependency - name: rake-compiler - requirement: &70243867725240 !ruby/object:Gem::Requirement - none: false - requirements: - - - !ruby/object:Syck::DefaultKey {} - - - !ruby/object:Gem::Version - version: 0.7.6 - type: :development - prerelease: false - version_requirements: *70243867725240 -platform: ruby -files: [] -test_files: [] -bindir: - YAML - - spec = Gem::Specification.from_yaml yaml - - op = spec.dependencies.first.requirement.requirements.first.first - refute_kind_of YAML::Syck::DefaultKey, op - - refute_match %r{DefaultKey}, spec.to_ruby - end - def test_self_from_yaml_cleans_up_Date_objects yaml = <<-YAML --- !ruby/object:Gem::Specification @@ -3860,49 +3739,6 @@ end end end - def with_syck - begin - verbose, $VERBOSE = $VERBOSE, nil - require "yaml" - old_engine = YAML::ENGINE.yamler - YAML::ENGINE.yamler = 'syck' - load 'rubygems/syck_hack.rb' - rescue NameError - # probably on 1.8, ignore - ensure - $VERBOSE = verbose - end - - yield - ensure - begin - YAML::ENGINE.yamler = old_engine - load 'rubygems/syck_hack.rb' - rescue NameError - # ignore - end - end - - def with_psych - begin - require "yaml" - old_engine = YAML::ENGINE.yamler - YAML::ENGINE.yamler = 'psych' - load 'rubygems/syck_hack.rb' - rescue NameError - # probably on 1.8, ignore - end - - yield - ensure - begin - YAML::ENGINE.yamler = old_engine - load 'rubygems/syck_hack.rb' - rescue NameError - # ignore - end - end - def silence_warnings old_verbose, $VERBOSE = $VERBOSE, false yield