Correctly read extra fields when opening a zip file.

Previously, only the extra fields stored in the central directory were being read in. In reality it is often the case that the extra field in the central directory is just a marker, and the full data is in the local header. So we need to read both in and merge the two into the final correct extra field. This merging infrastructure was already implemented in the extra field code but we never actually read the local extra fields in until now. Reading the central directory headers and local entry headers seems rather fragile, so we can't just read one over the other and hope to end up with a correctly merged set of extra fields because this breaks other things. So we need to specifically read the local extra field data and merge just those bits. This commit also fixes a couple of tests that were 'broken' by us now reading extra fields in correctly!
rubyzip · simonoff · Feb 14, 2021 · Sep 20, 2020 · Sep 20, 2020 · Sep 20, 2020
commit c2b9aa2893c03537e9a985b8410d9bb1eeeba8e2
diff --git a/lib/zip/central_directory.rb b/lib/zip/central_directory.rb
@@ -124,10 +124,37 @@ def read_central_directory_entries(io) #:nodoc:
       end
       @entry_set = EntrySet.new
       @size.times do
-        @entry_set << Entry.read_c_dir_entry(io)
+        entry = Entry.read_c_dir_entry(io)
+        next unless entry
+    <
8000
span class='blob-code-inner blob-code-marker ' data-code-marker="+">
+        offset = if entry.extra['Zip64']
+                   entry.extra['Zip64'].relative_header_offset
+                 else
+                   entry.local_header_offset
+                 end
+
+        unless offset.nil?
+          io_save = io.tell
+          io.seek(offset, IO::SEEK_SET)
+          entry.read_extra_field(read_local_extra_field(io))
+          io.seek(io_save, IO::SEEK_SET)
+        end
+
+        @entry_set << entry
       end
     end
 
+    def read_local_extra_field(io)
+      buf = io.read(::Zip::LOCAL_ENTRY_STATIC_HEADER_LENGTH) || ''
+      return '' unless buf.bytesize == ::Zip::LOCAL_ENTRY_STATIC_HEADER_LENGTH
+
+      head, _, _, _, _, _, _, _, _, _, n_len, e_len = buf.unpack('VCCvvvvVVVvv')
+      return '' unless head == ::Zip::LOCAL_ENTRY_SIGNATURE
+
+      io.seek(n_len, IO::SEEK_CUR) # Skip over the entry name.
+      io.read(e_len)
+    end
+
     def read_from_stream(io) #:nodoc:
       buf = start_buf(io)
       if zip64_file?(buf)

diff --git a/test/filesystem/file_nonmutating_test.rb b/test/filesystem/file_nonmutating_test.rb
@@ -295,8 +295,10 @@ def test_ctime
   end
 
   def test_atime
-    assert_nil(@zip_file.file.atime('file1'))
-    assert_nil(@zip_file.file.stat('file1').atime)
+    assert_equal(::Zip::DOSTime.at(1_027_694_306),
+                 @zip_file.file.atime('file1'))
+    assert_equal(::Zip::DOSTime.at(1_027_694_306),
+                 @zip_file.file.stat('file1').atime)
   end
 
   def test_ntfs_time

diff --git a/test/filesystem/file_stat_test.rb b/test/filesystem/file_stat_test.rb
@@ -19,11 +19,11 @@ def test_ino
   end
 
   def test_uid
-    assert_equal(0, @zip_file.file.stat('file1').uid)
+    assert_equal(500, @zip_file.file.stat('file1').uid)
   end
 
   def test_gid
-    assert_equal(0, @zip_file.file.stat('file1').gid)
+    assert_equal(500, @zip_file.file.stat('file1').gid)
   end
 
   def test_ftype