pr reviews

Author: Saidbek

lib/rubygems/package.rb

@@ -441,13 +441,13 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:
         full_name = entry.full_name
         next unless File.fnmatch pattern, full_name, File::FNM_DOTMATCH
 
-        destination = install_location full_name, destination_dir
-
-        if invalid_windows_filename?(full_name)
+        if Gem.win_platform? && invalid_windows_filename?(full_name)
           gem_name = @spec ? @spec.full_name : "unknown"
           raise Gem::Package::InvalidWindowsFileNameError.new(full_name, gem_name)
         end
 
+        destination = install_location 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))
@@ -471,18 +471,13 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:
         end
 
         if entry.file?
-          begin
-            File.open(destination, "wb") do |out|
-              copy_stream(tar.io, out, entry.size)
-              # Flush needs to happen before chmod because there could be data
-              # in the IO buffer that needs to be written, and that could be
-              # written after the chmod (on close) which would mess up the perms
-              out.flush
-              out.chmod file_mode(entry.header.mode) & ~File.umask
-            end
-          rescue Errno::EINVAL
-            gem_name = @spec ? @spec.full_name : "unknown"
-            raise Gem::Package::InvalidWindowsFileNameError.new(full_name, gem_name)
+          File.open(destination, "wb") do |out|
+            copy_stream(tar.io, out, entry.size)
+            # Flush needs to happen before chmod because there could be data
+            # in the IO buffer that needs to be written, and that could be
+            # written after the chmod (on close) which would mess up the perms
+            out.flush
+            out.chmod file_mode(entry.header.mode) & ~File.umask
           end
         end
 
@@ -562,10 +557,7 @@ def normalize_path(pathname) # :nodoc:
   # Note: Colons are only valid as drive letter separators (e.g., C:), not in filenames.
 
   def invalid_windows_filename?(filename) # :nodoc:
-    return false unless Gem.win_platform?
-
-    basename = File.basename(filename)
-    basename.match?(/[:<>"|?*\\\x00-\x1f]/)
+    filename.to_s.split("/").any? { |part| part.match?(/[:<>"|?*\\\x00-\x1f]/) }
   end
 
   ##

test/rubygems/test_gem_package.rb

@@ -1343,4 +1343,35 @@ def test_extract_tar_gz_invalid_filename
     assert_match(/characters in its name that are not allowed on Windows/, e.message)
     assert_match(/This is a problem with the 'a-2' gem, not Rubygems/, e.message)
   end
+
+  def test_build_warns_on_invalid_windows_filename
+    pend "Windows filename validation only applies on non-Windows" if Gem.win_platform?
+
+    spec = Gem::Specification.new "test_gem", "1.0"
+    spec.summary = "test"
+    spec.authors = "test"
+    spec.files = ["lib/code.rb", "lib/file:name.rb"]
+
+    FileUtils.mkdir "lib"
+
+    File.open "lib/code.rb", "w" do |io|
+      io.write "# lib/code.rb"
+    end
+
+    File.open "lib/file:name.rb", "w" do |io|
+      io.write "# lib/file:name.rb"
+    end
+
+    package = Gem::Package.new spec.file_name
+    package.spec = spec
+
+    ui = Gem::MockGemUi.new
+    use_ui ui do
+      package.build
+    end
+
+    assert_match(%r{filename 'lib/file:name\.rb' contains characters that are invalid on Windows}, ui.error)
+    assert_match(/This gem may fail to install on Windows/, ui.error)
+    assert_path_exist spec.file_name
+  end
 end