Repository: rubyzip/rubyzip (ruby)

Issue:
Many tests failing under Windows.
A lot of failures appear to be due to hardcoded paths into places like `/tmp` which should be easy to fix. Other tests appear to fail due to unexpected line endings - `\r\n` vs `\n` - which I think suggests that we're testing what comes out of a zip file (`\n`) with a text file that has been auto-converted to `\r\n` in the checkout process?


Discussion:

Patch:
diff --git a/lib/zip/entry.rb b/lib/zip/entry.rb
--- a/lib/zip/entry.rb
+++ b/lib/zip/entry.rb
@@ -164,8 +164,9 @@ def name_safe?
       return false unless cleanpath.relative?
 
       root = ::File::SEPARATOR
-      naive_expanded_path = ::File.join(root, cleanpath.to_s)
-      ::File.absolute_path(cleanpath.to_s, root) == naive_expanded_path
+      naive = ::File.join(root, cleanpath.to_s)
+      # Allow for Windows drive mappings at the root.
+      ::File.absolute_path(cleanpath.to_s, root).match?(/([A-Z]:)?#{naive}/i)
     end
 
     def local_entry_offset #:nodoc:all
diff --git a/lib/zip/filesystem.rb b/lib/zip/filesystem.rb
--- a/lib/zip/filesystem.rb
+++ b/lib/zip/filesystem.rb
@@ -620,7 +620,7 @@ def each
       end
 
       def expand_path(path)
-        expanded = ::File.expand_path(path, @pwd)
+        expanded = path.start_with?('/') ? path.dup : ::File.join(@pwd, path)
         expanded.gsub!(/\/\.(\/|$)/, '')
         expanded.gsub!(/[^\/]+\/\.\.(\/|$)/, '')
         expanded.empty? ? '/' : expanded
diff --git a/lib/zip/output_stream.rb b/lib/zip/output_stream.rb
--- a/lib/zip/output_stream.rb
+++ b/lib/zip/output_stream.rb
@@ -30,7 +30,7 @@ def initialize(file_name, stream = false, encrypter = nil)
       super()
       @file_name = file_name
       @output_stream = if stream
-                         iostream = @file_name.dup
+                         iostream = Zip::RUNNING_ON_WINDOWS ? @file_name : @file_name.dup
                          iostream.reopen(@file_name)
                          iostream.rewind
                          iostream


Test patch:
diff --git a/test/data/.gitattributes b/test/data/.gitattributes
new file mode 100644
--- /dev/null
+++ b/test/data/.gitattributes
@@ -0,0 +1,2 @@
+file1.txt eol=lf
+file2.txt eol=lf
diff --git a/test/entry_test.rb b/test/entry_test.rb
--- a/test/entry_test.rb
+++ b/test/entry_test.rb
@@ -151,32 +151,35 @@ def test_entry_name_cannot_start_with_slash
   end
 
   def test_store_file_without_compression
-    File.delete('/tmp/no_compress.zip') if File.exist?('/tmp/no_compress.zip')
-    files = Dir[File.join('test/data/globTest', '**', '**')]
+    Dir.mktmpdir do |tmp|
+      tmp_zip = File.join(tmp, 'no_compress.zip')
 
-    Zip.setup do |z|
-      z.write_zip64_support = false
-    end
+      Zip.setup do |z|
+        z.write_zip64_support = false
+      end
 
-    zipfile = Zip::File.open('/tmp/no_compress.zip', Zip::File::CREATE)
-    mimetype_entry = Zip::Entry.new(
-      zipfile,                # @zipfile
-      'mimetype',             # @name
-      compression_method: Zip::Entry::STORED
-    )
+      zipfile = Zip::File.open(tmp_zip, Zip::File::CREATE)
+
+      mimetype_entry = Zip::Entry.new(
+        zipfile,                # @zipfile
+        'mimetype',             # @name
+        compression_method: Zip::Entry::STORED
+      )
+      zipfile.add(mimetype_entry, 'test/data/mimetype')
 
-    zipfile.add(mimetype_entry, 'test/data/mimetype')
+      files = Dir[File.join('test/data/globTest', '**', '**')]
+      files.each do |file|
+        zipfile.add(file.sub('test/data/globTest/', ''), file)
+      end
 
-    files.each do |file|
-      zipfile.add(file.sub('test/data/globTest/', ''), file)
-    end
-    zipfile.close
+      zipfile.close
 
-    f = File.open('/tmp/no_compress.zip', 'rb')
-    first_100_bytes = f.read(100)
-    f.close
+      f = File.open(tmp_zip, 'rb')
+      first_100_bytes = f.read(100)
+      f.close
 
-    assert_match(/mimetypeapplication\/epub\+zip/, first_100_bytes)
+      assert_match(/mimetypeapplication\/epub\+zip/, first_100_bytes)
+    end
   end
 
   def test_encrypted?
diff --git a/test/file_options_test.rb b/test/file_options_test.rb
--- a/test/file_options_test.rb
+++ b/test/file_options_test.rb
@@ -58,6 +58,10 @@ def test_restore_times_true
       zip.extract(ENTRY_2, EXTPATH_2)
     end
 
+    # this test is disabled on Windows for now, waiting for #486.
+    # please remove this after merging #486.
+    skip if Zip::RUNNING_ON_WINDOWS
+
     assert_time_equal(::File.mtime(TXTPATH), ::File.mtime(EXTPATH_1))
     assert_time_equal(::File.mtime(TXTPATH), ::File.mtime(EXTPATH_2))
   end
diff --git a/test/file_test.rb b/test/file_test.rb
--- a/test/file_test.rb
+++ b/test/file_test.rb
@@ -113,21 +113,21 @@ def test_get_output_stream
   end
 
   def test_open_buffer_with_string
-    string = File.read('test/data/rubycode.zip')
+    string = File.read('test/data/rubycode.zip', mode: 'rb')
     ::Zip::File.open_buffer string do |zf|
       assert zf.entries.map(&:name).include?('zippedruby1.rb')
     end
   end
 
   def test_open_buffer_with_stringio
-    string_io = StringIO.new File.read('test/data/rubycode.zip')
+    string_io = StringIO.new File.read('test/data/rubycode.zip', mode: 'rb')
     ::Zip::File.open_buffer string_io do |zf|
       assert zf.entries.map(&:name).include?('zippedruby1.rb')
     end
   end
 
   def test_close_buffer_with_stringio
-    string_io = StringIO.new File.read('test/data/rubycode.zip')
+    string_io = StringIO.new File.read('test/data/rubycode.zip', mode: 'rb')
     zf = ::Zip::File.open_buffer string_io
     assert_nil zf.close
   end
@@ -178,7 +178,7 @@ def test_open_buffer_with_io_and_block
   end
 
   def test_open_buffer_without_block
-    string_io = StringIO.new File.read('test/data/rubycode.zip')
+    string_io = StringIO.new File.read('test/data/rubycode.zip', mode: 'rb')
     zf = ::Zip::File.open_buffer string_io
     assert zf.entries.map(&:name).include?('zippedruby1.rb')
   end
@@ -285,16 +285,20 @@ def test_add_stored
   end
 
   def test_recover_permissions_after_add_files_to_archive
+    # Windows NT does not support granular permissions
+    skip if Zip::RUNNING_ON_WINDOWS
+
     src_zip = TEST_ZIP.zip_name
-    ::File.chmod(0o664, src_zip)
-    src_file = 'test/data/file2.txt'
-    entry_name = 'newEntryName.rb'
-    assert_equal(::File.stat(src_zip).mode, 0o100664)
     assert(::File.exist?(src_zip))
+
+    ::File.chmod(0o664, src_zip)
+    assert_equal(0o100664, ::File.stat(src_zip).mode)
+
     zf = ::Zip::File.new(src_zip, ::Zip::File::CREATE)
-    zf.add(entry_name, src_file)
+    zf.add('newEntryName.rb', 'test/data/file2.txt')
     zf.close
-    assert_equal(::File.stat(src_zip).mode, 0o100664)
+
+    assert_equal(0o100664, ::File.stat(src_zip).mode)
   end
 
   def test_add_existing_entry_name
diff --git a/test/gentestfiles.rb b/test/gentestfiles.rb
--- a/test/gentestfiles.rb
+++ b/test/gentestfiles.rb
@@ -79,19 +79,19 @@ def self.create_test_zips
         "zip -q #{TEST_ZIP1.zip_name} -d test/data/file2.txt"
       )
 
-    File.open('test/data/generated/empty.txt', 'w') {} # Empty file.
-    File.open('test/data/generated/empty_chmod640.txt', 'w') {} # Empty file.
+    File.open('test/data/generated/empty.txt', 'wb') {} # Empty file.
+    File.open('test/data/generated/empty_chmod640.txt', 'wb') {} # Empty file.
     ::File.chmod(0o640, 'test/data/generated/empty_chmod640.txt')
 
-    File.open('test/data/generated/short.txt', 'w') { |file| file << 'ABCDEF' }
+    File.open('test/data/generated/short.txt', 'wb') { |file| file << 'ABCDEF' }
     test_text = ''
-    File.open('test/data/file2.txt') { |file| test_text = file.read }
-    File.open('test/data/generated/longAscii.txt', 'w') do |file|
+    File.open('test/data/file2.txt', 'rb') { |file| test_text = file.read }
+    File.open('test/data/generated/longAscii.txt', 'wb') do |file|
       file << test_text while file.tell < 1E5
     end
 
     binary_pattern = ''
-    File.open('test/data/generated/empty.zip') do |file|
+    File.open('test/data/generated/empty.zip', 'rb') do |file|
       binary_pattern = file.read
     end
     binary_pattern *= 4
@@ -108,7 +108,7 @@ def self.create_test_zips
     if RUBY_PLATFORM.match?(/mswin|mingw|cygwin/)
       raise "failed to add comment to test zip '#{TEST_ZIP2.zip_name}'" \
         unless system(
-          "echo #{TEST_ZIP2.comment}| zip -zq #{TEST_ZIP2.zip_name}\""
+          "cmd /c \"<NUL set /p = \"#{TEST_ZIP2.comment}\"| zip -zq #{TEST_ZIP2.zip_name}\""
         )
     else
       # without bash system interprets everything after echo as parameters to
diff --git a/test/input_stream_test.rb b/test/input_stream_test.rb
--- a/test/input_stream_test.rb
+++ b/test/input_stream_test.rb
@@ -42,13 +42,13 @@ def test_open_buffer_with_block
   end
 
   def test_open_string_io_without_block
-    string_io = ::StringIO.new(::File.read(TestZipFile::TEST_ZIP2.zip_name))
+    string_io = ::StringIO.new(::File.read(TestZipFile::TEST_ZIP2.zip_name, mode: 'rb'))
     zis = ::Zip::InputStream.open(string_io)
     assert_stream_contents(zis, TestZipFile::TEST_ZIP2)
   end
 
   def test_open_string_io_with_block
-    string_io = ::StringIO.new(::File.read(TestZipFile::TEST_ZIP2.zip_name))
+    string_io = ::StringIO.new(::File.read(TestZipFile::TEST_ZIP2.zip_name, mode: 'rb'))
     ::Zip::InputStream.open(string_io) do |zis|
       assert_stream_contents(zis, TestZipFile::TEST_ZIP2)
       assert_equal(true, zis.eof?)
@@ -106,7 +106,7 @@ def test_incomplete_reads
   end
 
   def test_incomplete_reads_from_string_io
-    string_io = ::StringIO.new(::File.read(TestZipFile::TEST_ZIP2.zip_name))
+    string_io = ::StringIO.new(::File.read(TestZipFile::TEST_ZIP2.zip_name, mode: 'rb'))
     ::Zip::InputStream.open(string_io) do |zis|
       entry = zis.get_next_entry # longAscii.txt
       assert_equal(false, zis.eof?)
diff --git a/test/output_stream_test.rb b/test/output_stream_test.rb
--- a/test/output_stream_test.rb
+++ b/test/output_stream_test.rb
@@ -85,7 +85,7 @@ def test_put_next_entry
       zos << stored_text
     end
 
-    assert(File.read(TEST_ZIP.zip_name)[stored_text])
+    assert(File.read(TEST_ZIP.zip_name, mode: 'rb')[stored_text])
     ::Zip::File.open(TEST_ZIP.zip_name) do |zf|
       assert_equal(stored_text, zf.read(entry_name))
     end
diff --git a/test/pass_thru_decompressor_test.rb b/test/pass_thru_decompressor_test.rb
--- a/test/pass_thru_decompressor_test.rb
+++ b/test/pass_thru_decompressor_test.rb
@@ -6,7 +6,7 @@ class PassThruDecompressorTest < MiniTest::Test
 
   def setup
     super
-    @file = File.new(TEST_FILE)
+    @file = File.new(TEST_FILE, 'rb')
     @decompressor = ::Zip::PassThruDecompressor.new(@file, File.size(TEST_FILE))
   end
 
diff --git a/test/test_helper.rb b/test/test_helper.rb
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -57,7 +57,7 @@ module DecompressorTests
 
   def setup
     @ref_text = ''
-    File.open(TEST_FILE) { |f| @ref_text = f.read }
+    File.open(TEST_FILE, 'rb') { |f| @ref_text = f.read }
     @ref_lines = @ref_text.split($INPUT_RECORD_SEPARATOR)
   end