From 08fb5c2153deaee53e481ebe3602fe1d59e256c7 Mon Sep 17 00:00:00 2001 From: usa Date: Fri, 16 Feb 2018 16:21:44 +0000 Subject: merge revision(s) 58471,58493,62436: [Backport #13505] load.c: backtrace of circular require * load.c (load_lock): print backtrace of circular require via `Warning.warn` [ruby-core:80850] [Bug #13505] Send the backtrace of the circular require warning as a single String to Warning.warn * load.c: send as a single string. * error.c: expose the string formatted by rb_warning as rb_warning_string(). * test/ruby/test_exception.rb: update tests. [ruby-core:80850] [Bug #13505] fix regexp literal warning. * test/rubygems/test_gem_server.rb: eliminate duplicated character class warning. [Bug #14481] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_3@62439 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 19 +++ lib/rubygems.rb | 2 +- lib/rubygems/commands/owner_command.rb | 2 +- lib/rubygems/package.rb | 37 ++++- lib/rubygems/package/tar_header.rb | 23 ++-- lib/rubygems/package/tar_writer.rb | 2 + lib/rubygems/server.rb | 14 +- lib/rubygems/specification.rb | 15 ++- test/rubygems/test_gem_commands_owner_command.rb | 25 ++++ test/rubygems/test_gem_package.rb | 74 +++++++++- test/rubygems/test_gem_package_tar_header.rb | 21 +++ test/rubygems/test_gem_server.rb | 165 +++++++++++++++++++++++ test/rubygems/test_gem_specification.rb | 17 ++- version.h | 6 +- 14 files changed, 397 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index 163c685140..b6810a1ce8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,22 @@ +Sat Feb 17 01:21:00 2018 SHIBATA Hiroshi + + load.c: backtrace of circular require + + * load.c (load_lock): print backtrace of circular require via + `Warning.warn` [ruby-core:80850] [Bug #13505] + + Send the backtrace of the circular require warning as a single String to Warning.warn + + * load.c: send as a single string. + * error.c: expose the string formatted by rb_warning as rb_warning_string(). + * test/ruby/test_exception.rb: update tests. + [ruby-core:80850] [Bug #13505] + + fix regexp literal warning. + + * test/rubygems/test_gem_server.rb: eliminate duplicated character class warning. + [Bug #14481] + Thu Feb 1 04:00:53 2018 NARUSE, Yui Remove debug print introduced in r52386 diff --git a/lib/rubygems.rb b/lib/rubygems.rb index fd4075a632..acec819ab6 100644 --- a/lib/rubygems.rb +++ b/lib/rubygems.rb @@ -10,7 +10,7 @@ require 'rbconfig' require 'thread' module Gem - VERSION = '2.5.2.2' + VERSION = '2.5.2.3' end # Must be first since it unloads the prelude from 1.9.2 diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb index e507c988d6..62780f354e 100644 --- a/lib/rubygems/commands/owner_command.rb +++ b/lib/rubygems/commands/owner_command.rb @@ -62,7 +62,7 @@ permission to. end with_response response do |resp| - owners = YAML.load resp.body + owners = Gem::SafeYAML.load resp.body say "Owners for gem: #{name}" owners.each do |owner| diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb index ab49ea2d2c..93054828e0 100644 --- a/lib/rubygems/package.rb +++ b/lib/rubygems/package.rb @@ -376,7 +376,7 @@ EOM File.dirname destination end - FileUtils.mkdir_p mkdir, mkdir_options + mkdir_p_safe mkdir, mkdir_options, destination_dir, entry.full_name open destination, 'wb' do |out| out.write entry.read @@ -414,20 +414,35 @@ EOM raise Gem::Package::PathError.new(filename, destination_dir) if filename.start_with? '/' - destination_dir = File.realpath destination_dir if - File.respond_to? :realpath + destination_dir = realpath destination_dir destination_dir = File.expand_path destination_dir destination = File.join destination_dir, filename destination = File.expand_path destination raise Gem::Package::PathError.new(destination, destination_dir) unless - destination.start_with? destination_dir + destination.start_with? destination_dir + '/' destination.untaint destination end + def mkdir_p_safe mkdir, mkdir_options, destination_dir, file_name + destination_dir = realpath File.expand_path(destination_dir) + parts = mkdir.split(File::SEPARATOR) + parts.reduce do |path, basename| + path = realpath path unless path == "" + path = File.expand_path(path + File::SEPARATOR + basename) + lstat = File.lstat path rescue nil + if !lstat || !lstat.directory? + unless path.start_with? destination_dir and (FileUtils.mkdir path, mkdir_options rescue false) + raise Gem::Package::PathError.new(file_name, destination_dir) + end + end + path + end + end + ## # Loads a Gem::Specification from the TarEntry +entry+ @@ -601,6 +616,10 @@ EOM raise Gem::Package::FormatError.new \ 'package content (data.tar.gz) is missing', @gem end + + if duplicates = @files.group_by {|f| f }.select {|k,v| v.size > 1 }.map(&:first) and duplicates.any? + raise Gem::Security::Exception, "duplicate files in the package: (#{duplicates.map(&:inspect).join(', ')})" + end end ## @@ -614,6 +633,16 @@ EOM raise Gem::Package::FormatError.new(e.message, entry.full_name) end + if File.respond_to? :realpath + def realpath file + File.realpath file + end + else + def realpath file + file + end + end + end require 'rubygems/package/digest_io' diff --git a/lib/rubygems/package/tar_header.rb b/lib/rubygems/package/tar_header.rb index c54bd14d57..d557357114 100644 --- a/lib/rubygems/package/tar_header.rb +++ b/lib/rubygems/package/tar_header.rb @@ -104,25 +104,30 @@ class Gem::Package::TarHeader fields = header.unpack UNPACK_FORMAT new :name => fields.shift, - :mode => fields.shift.oct, - :uid => fields.shift.oct, - :gid => fields.shift.oct, - :size => fields.shift.oct, - :mtime => fields.shift.oct, - :checksum => fields.shift.oct, + :mode => strict_oct(fields.shift), + :uid => strict_oct(fields.shift), + :gid => strict_oct(fields.shift), + :size => strict_oct(fields.shift), + :mtime => strict_oct(fields.shift), + :checksum => strict_oct(fields.shift), :typeflag => fields.shift, :linkname => fields.shift, :magic => fields.shift, - :version => fields.shift.oct, + :version => strict_oct(fields.shift), :uname => fields.shift, :gname => fields.shift, - :devmajor => fields.shift.oct, - :devminor => fields.shift.oct, + :devmajor => strict_oct(fields.shift), + :devminor => strict_oct(fields.shift), :prefix => fields.shift, :empty => empty end + def self.strict_oct(str) + return str.oct if str =~ /\A[0-7]*\z/ + raise ArgumentError, "#{str.inspect} is not an octal string" + end + ## # Creates a new TarHeader using +vals+ diff --git a/lib/rubygems/package/tar_writer.rb b/lib/rubygems/package/tar_writer.rb index ab0313c9f8..2c94c95123 100644 --- a/lib/rubygems/package/tar_writer.rb +++ b/lib/rubygems/package/tar_writer.rb @@ -196,6 +196,8 @@ class Gem::Package::TarWriter digest_name == signer.digest_name end + raise "no #{signer.digest_name} in #{digests.values.compact}" unless signature_digest + if signer.key then signature = signer.sign signature_digest.digest diff --git a/lib/rubygems/server.rb b/lib/rubygems/server.rb index 8d6a96b1c2..db3730bc7d 100644 --- a/lib/rubygems/server.rb +++ b/lib/rubygems/server.rb @@ -626,6 +626,18 @@ div.method-source-code pre { color: #ffdead; overflow: hidden; } executables = nil if executables.empty? executables.last["is_last"] = true if executables + # Pre-process spec homepage for safety reasons + begin + homepage_uri = URI.parse(spec.homepage) + if [URI::HTTP, URI::HTTPS].member? homepage_uri.class + homepage_uri = spec.homepage + else + homepage_uri = "." + end + rescue URI::InvalidURIError + homepage_uri = "." + end + specs << { "authors" => spec.authors.sort.join(", "), "date" => spec.date.to_s, @@ -635,7 +647,7 @@ div.method-source-code pre { color: #ffdead; overflow: hidden; } "only_one_executable" => (executables && executables.size == 1), "full_name" => spec.full_name, "has_deps" => !deps.empty?, - "homepage" => spec.homepage, + "homepage" => homepage_uri, "name" => spec.name, "rdoc_installed" => Gem::RDoc.new(spec).rdoc_installed?, "ri_installed" => Gem::RDoc.new(spec).ri_installed?, diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb index de324d76d9..2d6b1683bd 100644 --- a/lib/rubygems/specification.rb +++ b/lib/rubygems/specification.rb @@ -15,6 +15,7 @@ require 'rubygems/basic_specification' require 'rubygems/stub_specification' require 'rubygems/util/list' require 'stringio' +require 'uri' ## # The Specification class contains the information for a Gem. Typically @@ -2807,10 +2808,16 @@ http://spdx.org/licenses or '#{Gem::Licenses::NONSTANDARD}' for a nonstandard li raise Gem::InvalidSpecificationException, "#{lazy} is not a summary" end - if homepage and not homepage.empty? and - homepage !~ /\A[a-z][a-z\d+.-]*:/i then - raise Gem::InvalidSpecificationException, - "\"#{homepage}\" is not a URI" + # Make sure a homepage is valid HTTP/HTTPS URI + if homepage and not homepage.empty? + begin + homepage_uri = URI.parse(homepage) + unless [URI::HTTP, URI::HTTPS].member? homepage_uri.class + raise Gem::InvalidSpecificationException, "\"#{homepage}\" is not a valid HTTP URI" + end + rescue URI::InvalidURIError + raise Gem::InvalidSpecificationException, "\"#{homepage}\" is not a valid HTTP URI" + end end # Warnings diff --git a/test/rubygems/test_gem_commands_owner_command.rb b/test/rubygems/test_gem_commands_owner_command.rb index 7217f5cff0..d68e8528db 100644 --- a/test/rubygems/test_gem_commands_owner_command.rb +++ b/test/rubygems/test_gem_commands_owner_command.rb @@ -36,6 +36,31 @@ EOF assert_match %r{- user2@example.com}, @ui.output end + def test_show_owners_dont_load_objects + skip "testing a psych-only API" unless defined?(::Psych::DisallowedClass) + + response = <xsshomepagegem 1 + # + # + # [rdoc] + # + # + # + # [www] + # + # Variant #2 - rdoc installed + # + # xsshomepagegem 1 + # + # + # \[rdoc\]<\/a> + # + # + # + # [www] + regex_match = /xsshomepagegem 1<\/b>[\s]+(\[rdoc\]<\/span>|\[rdoc\]<\/a>)[\s]+\[www\]<\/a>/ + assert_match regex_match, @res.body + end + + def test_invalid_homepage + data = StringIO.new "GET / HTTP/1.0\r\n\r\n" + dir = "#{@gemhome}2" + + spec = util_spec 'invalidhomepagegem', 1 + spec.homepage = "notavalidhomepageurl" + + specs_dir = File.join dir, 'specifications' + FileUtils.mkdir_p specs_dir + + open File.join(specs_dir, spec.spec_name), 'w' do |io| + io.write spec.to_ruby + end + + server = Gem::Server.new dir, process_based_port, false + + @req.parse data + + server.root @req, @res + + assert_equal 200, @res.status + assert_match 'invalidhomepagegem 1', @res.body + + # This verifies that the homepage for this spec is not displayed and is set to ".", because it's not a + # valid HTTP/HTTPS URL and could be unsafe in an HTML context. We would prefer to throw an exception here, + # but spec.homepage is currently free form and not currently required to be a URL, this behavior may be + # validated in future versions of Gem::Specification. + # + # There are two variant we're checking here, one where rdoc is not present, and one where rdoc is present in the same regex: + # + # Variant #1 - rdoc not installed + # + # invalidhomepagegem 1 + # + # + # [rdoc] + # + # + # + # [www] + # + # Variant #2 - rdoc installed + # + # invalidhomepagegem 1 + # + # + # \[rdoc\]<\/a> + # + # + # + # [www] + regex_match = /invalidhomepagegem 1<\/b>[\s]+(\[rdoc\]<\/span>|\[rdoc\]<\/a>)[\s]+\[www\]<\/a>/ + assert_match regex_match, @res.body + end + + def test_valid_homepage_http + data = StringIO.new "GET / HTTP/1.0\r\n\r\n" + dir = "#{@gemhome}2" + + spec = util_spec 'validhomepagegemhttp', 1 + spec.homepage = "http://rubygems.org" + + specs_dir = File.join dir, 'specifications' + FileUtils.mkdir_p specs_dir + + open File.join(specs_dir, spec.spec_name), 'w' do |io| + io.write spec.to_ruby + end + + server = Gem::Server.new dir, process_based_port, false + + @req.parse data + + server.root @req, @res + + assert_equal 200, @res.status + assert_match 'validhomepagegemhttp 1', @res.body + + regex_match = /validhomepagegemhttp 1<\/b>[\s]+(\[rdoc\]<\/span>|\[rdoc\]<\/a>)[\s]+\[www\]<\/a>/ + assert_match regex_match, @res.body + end + + def test_valid_homepage_https + data = StringIO.new "GET / HTTP/1.0\r\n\r\n" + dir = "#{@gemhome}2" + + spec = util_spec 'validhomepagegemhttps', 1 + spec.homepage = "https://rubygems.org" + + specs_dir = File.join dir, 'specifications' + FileUtils.mkdir_p specs_dir + + open File.join(specs_dir, spec.spec_name), 'w' do |io| + io.write spec.to_ruby + end + + server = Gem::Server.new dir, process_based_port, false + + @req.parse data + + server.root @req, @res + + assert_equal 200, @res.status + assert_match 'validhomepagegemhttps 1', @res.body + + regex_match = /validhomepagegemhttps 1<\/b>[\s]+(\[rdoc\]<\/span>|\[rdoc\]<\/a>)[\s]+\[www\]<\/a>/ + assert_match regex_match, @res.body + end + def test_specs data = StringIO.new "GET /specs.#{Gem.marshal_version} HTTP/1.0\r\n\r\n" @req.parse data diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb index 9a49bbbf59..33b39fa48c 100644 --- a/test/rubygems/test_gem_specification.rb +++ b/test/rubygems/test_gem_specification.rb @@ -2879,7 +2879,22 @@ duplicate dependency on c (>= 1.2.3, development), (~> 1.2) use: @a1.validate end - assert_equal '"over at my cool site" is not a URI', e.message + assert_equal '"over at my cool site" is not a valid HTTP URI', e.message + + @a1.homepage = 'ftp://rubygems.org' + + e = assert_raises Gem::InvalidSpecificationException do + @a1.validate + end + + assert_equal '"ftp://rubygems.org" is not a valid HTTP URI', e.message + + @a1.homepage = 'http://rubygems.org' + assert_equal true, @a1.validate + + @a1.homepage = 'https://rubygems.org' + assert_equal true, @a1.validate + end end diff --git a/version.h b/version.h index c428cffe49..44aa7ec272 100644 --- a/version.h +++ b/version.h @@ -1,10 +1,10 @@ #define RUBY_VERSION "2.3.7" -#define RUBY_RELEASE_DATE "2018-02-01" -#define RUBY_PATCHLEVEL 407 +#define RUBY_RELEASE_DATE "2018-02-17" +#define RUBY_PATCHLEVEL 408 #define RUBY_RELEASE_YEAR 2018 #define RUBY_RELEASE_MONTH 2 -#define RUBY_RELEASE_DAY 1 +#define RUBY_RELEASE_DAY 17 #include "ruby/version.h" -- cgit v1.2.3