From d35f4b8cad02d30869429fc1a1c2f5c89efb81a0 Mon Sep 17 00:00:00 2001 From: Said Kaldybaev Date: Wed, 28 Jan 2026 23:08:27 -0800 Subject: [PATCH 1/3] Improve handling of paths with invalid Windows characters Add validation and clear error messages for gems with filenames containing invalid Windows characters (e.g., colons), directing users to report issues to gem authors instead of RubyGems. --- lib/rubygems/package.rb | 53 +++++++++++++++++++++++++++---- test/rubygems/test_gem_package.rb | 40 +++++++++++++++++++++++ 2 files changed, 86 insertions(+), 7 deletions(-) diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb index e6e078dce406..27e2a7344111 100644 --- a/lib/rubygems/package.rb +++ b/lib/rubygems/package.rb @@ -89,6 +89,18 @@ class TooLongFileName < Error; end class TarInvalidError < Error; end + ## + # Raised when a filename contains characters that are invalid on Windows + + class InvalidFileNameError < Error + def initialize(filename, gem_name = nil) + message = "The gem contains a file '#{filename}' with characters in its name that are not allowed on Windows (e.g., colons)." + message += " This is a problem with the '#{gem_name}' gem, not Rubygems." if gem_name + message += " Please report this issue to the gem author." + super message + end + end + attr_accessor :build_time # :nodoc: ## @@ -258,6 +270,10 @@ def add_contents(tar) # :nodoc: def add_files(tar) # :nodoc: @spec.files.each do |file| + if invalid_windows_filename?(file) + alert_warning "filename '#{file}' contains characters that are invalid on Windows (e.g., colons). This gem may fail to install on Windows." + end + stat = File.lstat file if stat.symlink? @@ -427,6 +443,11 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc: destination = install_location full_name, destination_dir + if invalid_windows_filename?(full_name) + gem_name = @spec ? @spec.full_name : "unknown" + raise Gem::Package::InvalidFileNameError.new(full_name, gem_name) + end + if entry.symlink? link_target = entry.header.linkname real_destination = link_target.start_with?("/") ? link_target : File.expand_path(link_target, File.dirname(destination)) @@ -450,13 +471,18 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc: end if entry.file? - 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 + 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 => e + gem_name = @spec ? @spec.full_name : "unknown" + raise Gem::Package::InvalidFileNameError.new(full_name, gem_name), e.message end end @@ -529,6 +555,19 @@ def normalize_path(pathname) # :nodoc: end end + ## + # Checks if a filename contains characters that are invalid on Windows. + # Windows doesn't allow: < > : " | ? * \ and control characters (0x00-0x1F). + # Colons are the most common issue since they're allowed on Unix. + # 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]/) + end + ## # Loads a Gem::Specification from the TarEntry +entry+ diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index 0c214a232b76..feb540c421f5 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -1303,4 +1303,44 @@ def test_contents_from_io assert_equal %w[lib/code.rb], package.contents end + + def test_invalid_windows_filename + package = Gem::Package.new @gem + + if Gem.win_platform? + assert package.invalid_windows_filename?("spec/internal/:memory") + assert package.invalid_windows_filename?("file:name.rb") + assert package.invalid_windows_filename?("file Date: Sun, 8 Feb 2026 20:46:30 -0800 Subject: [PATCH 2/3] Rename InvalidFileNameError to InvalidWindowsFileNameError for clarity --- lib/rubygems/package.rb | 8 ++++---- test/rubygems/test_gem_package.rb | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb index 27e2a7344111..40abe617ab90 100644 --- a/lib/rubygems/package.rb +++ b/lib/rubygems/package.rb @@ -92,7 +92,7 @@ class TarInvalidError < Error; end ## # Raised when a filename contains characters that are invalid on Windows - class InvalidFileNameError < Error + class InvalidWindowsFileNameError < Error def initialize(filename, gem_name = nil) message = "The gem contains a file '#{filename}' with characters in its name that are not allowed on Windows (e.g., colons)." message += " This is a problem with the '#{gem_name}' gem, not Rubygems." if gem_name @@ -445,7 +445,7 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc: if invalid_windows_filename?(full_name) gem_name = @spec ? @spec.full_name : "unknown" - raise Gem::Package::InvalidFileNameError.new(full_name, gem_name) + raise Gem::Package::InvalidWindowsFileNameError.new(full_name, gem_name) end if entry.symlink? @@ -480,9 +480,9 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc: out.flush out.chmod file_mode(entry.header.mode) & ~File.umask end - rescue Errno::EINVAL => e + rescue Errno::EINVAL gem_name = @spec ? @spec.full_name : "unknown" - raise Gem::Package::InvalidFileNameError.new(full_name, gem_name), e.message + raise Gem::Package::InvalidWindowsFileNameError.new(full_name, gem_name) end end diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index feb540c421f5..b76f564da1ef 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -1316,7 +1316,7 @@ def test_invalid_windows_filename end def test_invalid_file_name_error_message - error = Gem::Package::InvalidFileNameError.new("spec/internal/:memory", "crono-2.0.1") + error = Gem::Package::InvalidWindowsFileNameError.new("spec/internal/:memory", "crono-2.0.1") assert_match(%r{The gem contains a file 'spec/internal/:memory'}, error.message) assert_match(/characters in its name that are not allowed on Windows/, error.message) assert_match(/This is a problem with the 'crono-2.0.1' gem, not Rubygems/, error.message) @@ -1335,7 +1335,7 @@ def test_extract_tar_gz_invalid_filename end end - e = assert_raise Gem::Package::InvalidFileNameError do + e = assert_raise Gem::Package::InvalidWindowsFileNameError do package.extract_tar_gz tgz_io, @destination end From e2d08d6014ca6bf67178dd48e3eb2f9b798947e7 Mon Sep 17 00:00:00 2001 From: Said Kaldybaev Date: Thu, 12 Feb 2026 16:14:12 -0800 Subject: [PATCH 3/3] pr reviews --- lib/rubygems/package.rb | 30 +++++++++++------------------- test/rubygems/test_gem_package.rb | 31 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb index 40abe617ab90..0d05bc962e69 100644 --- a/lib/rubygems/package.rb +++ b/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 ## diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index b76f564da1ef..8027f09a315b 100644 --- a/test/rubygems/test_gem_package.rb +++ b/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