Merge pull request #4960 from rubygems/speed_up_gem_install

Speed up `gem install`, specially under Windows

lib/rubygems/package.rb

@@ -71,6 +71,13 @@ def initialize(destination, destination_dir)
     end
   end
 
+  class SymlinkError < Error
+    def initialize(name, destination, destination_dir)
+      super "installing symlink '%s' pointing to parent path %s of %s is not allowed" %
+              [name, destination, destination_dir]
+    end
+  end
+
   class NonSeekableIO < Error; end
 
   class TooLongFileName < Error; end
@@ -400,13 +407,21 @@ def extract_files(destination_dir, pattern = "*")
   # extracted.
 
   def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:
-    directories = [] if dir_mode
+    directories = []
     open_tar_gz io do |tar|
       tar.each do |entry|
         next unless File.fnmatch pattern, entry.full_name, File::FNM_DOTMATCH
 
         destination = install_location entry.full_name, destination_dir
 
+        if entry.symlink?
+          link_target = entry.header.linkname
+          real_destination = link_target.start_with?("/") ? link_target : File.expand_path(link_target, File.dirname(destination))
+
+          raise Gem::Package::SymlinkError.new(entry.full_name, real_destination, destination_dir) unless
+            normalize_path(real_destination).start_with? normalize_path(destination_dir + '/')
+        end
+
         FileUtils.rm_rf destination
 
         mkdir_options = {}
@@ -417,9 +432,11 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:
           else
             File.dirname destination
           end
-        directories << mkdir if directories
 
-        mkdir_p_safe mkdir, mkdir_options, destination_dir, entry.full_name
+        unless directories.include?(mkdir)
+          FileUtils.mkdir_p mkdir, **mkdir_options
+          directories << mkdir
+        end
 
         File.open destination, 'wb' do |out|
           out.write entry.read
@@ -432,8 +449,7 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:
       end
     end
 
-    if directories
-      directories.uniq!
+    if dir_mode
       File.chmod(dir_mode, *directories)
     end
   end
@@ -466,21 +482,11 @@ def install_location(filename, destination_dir) # :nodoc:
     raise Gem::Package::PathError.new(filename, destination_dir) if
       filename.start_with? '/'
 
-    destination_dir = File.expand_path(File.realpath(destination_dir))
-    destination = File.expand_path(File.join(destination_dir, filename))
+    destination_dir = File.realpath(destination_dir)
+    destination = File.expand_path(filename, destination_dir)
 
     raise Gem::Package::PathError.new(destination, destination_dir) unless
-      destination.start_with? destination_dir + '/'
-
-    begin
-      real_destination = File.expand_path(File.realpath(destination))
-    rescue
-      # it's fine if the destination doesn't exist, because rm -rf'ing it can't cause any damage
-      nil
-    else
-      raise Gem::Package::PathError.new(real_destination, destination_dir) unless
-        real_destination.start_with? destination_dir + '/'
-    end
+      normalize_path(destination).start_with? normalize_path(destination_dir + '/')
 
     destination.tap(&Gem::UNTAINT)
     destination
@@ -494,22 +500,6 @@ def normalize_path(pathname)
     end
   end
 
-  def mkdir_p_safe(mkdir, mkdir_options, destination_dir, file_name)
-    destination_dir = File.realpath(File.expand_path(destination_dir))
-    parts = mkdir.split(File::SEPARATOR)
-    parts.reduce do |path, basename|
-      path = File.realpath(path) unless path == ""
-      path = File.expand_path(path + File::SEPARATOR + basename)
-      lstat = File.lstat path rescue nil
-      if !lstat || !lstat.directory?
-        unless normalize_path(path).start_with? normalize_path(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+
 

test/rubygems/test_gem_package.rb

@@ -574,18 +574,19 @@ def test_extract_symlink_parent
     destination_subdir = File.join @destination, 'subdir'
     FileUtils.mkdir_p destination_subdir
 
-    e = assert_raise(Gem::Package::PathError, Errno::EACCES) do
+    expected_exceptions = win_platform? ? [Gem::Package::SymlinkError, Errno::EACCES] : [Gem::Package::SymlinkError]
+
+    e = assert_raise(*expected_exceptions) do
       package.extract_tar_gz tgz_io, destination_subdir
     end
 
-    if Gem::Package::PathError === e
-      assert_equal("installing into parent path lib/link/outside.txt of " +
-                  "#{destination_subdir} is not allowed", e.message)
-    elsif win_platform?
-      pend "symlink - must be admin with no UAC on Windows"
-    else
-      raise e
-    end
+    pend "symlink - must be admin with no UAC on Windows" if Errno::EACCES === e
+
+    assert_equal("installing symlink 'lib/link' pointing to parent path #{@destination} of " +
+                "#{destination_subdir} is not allowed", e.message)
+
+    assert_path_not_exist File.join(@destination, "outside.txt")
+    assert_path_not_exist File.join(destination_subdir, "lib/link")
   end
 
   def test_extract_symlink_parent_doesnt_delete_user_dir
@@ -608,20 +609,20 @@ def test_extract_symlink_parent_doesnt_delete_user_dir
       tar.add_symlink 'link/dir', '.', 16877
     end
 
-    e = assert_raise(Gem::Package::PathError, Errno::EACCES) do
+    expected_exceptions = win_platform? ? [Gem::Package::SymlinkError, Errno::EACCES] : [Gem::Package::SymlinkError]
+
+    e = assert_raise(*expected_exceptions) do
       package.extract_tar_gz tgz_io, destination_subdir
     end
 
-    assert_path_exist destination_user_subdir
+    pend "symlink - must be admin with no UAC on Windows" if Errno::EACCES === e
 
-    if Gem::Package::PathError === e
-      assert_equal("installing into parent path #{destination_user_subdir} of " +
-                  "#{destination_subdir} is not allowed", e.message)
-    elsif win_platform?
-      pend "symlink - must be admin with no UAC on Windows"
-    else
-      raise e
-    end
+    assert_equal("installing symlink 'link' pointing to parent path #{destination_user_dir} of " +
+                "#{destination_subdir} is not allowed", e.message)
+
+    assert_path_exist destination_user_subdir
+    assert_path_not_exist File.join(destination_subdir, "link/dir")
+    assert_path_not_exist File.join(destination_subdir, "link")
   end
 
   def test_extract_tar_gz_directory