From 14b63f68dbaab3f1a799de87d753cdcffff78e37 Mon Sep 17 00:00:00 2001
From: Robert Haines <rhaines@manchester.ac.uk>
Date: Tue, 30 Nov 2021 22:22:37 +0000
Subject: [PATCH] Ensure `File.open_buffer` doesn't rewrite unchanged data.

---
 .rubocop_todo.yml |  2 +-
 lib/zip/file.rb   |  3 +++
 test/file_test.rb | 26 ++++++++++++++++++++++++--
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index b5466f4..4406c97 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -30,7 +30,7 @@ Metrics/CyclomaticComplexity:
 # Offense count: 47
 # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods.
 Metrics/MethodLength:
-  Max: 32
+  Max: 34
 
 # Offense count: 5
 # Configuration parameters: CountKeywordArgs.
diff --git a/lib/zip/file.rb b/lib/zip/file.rb
index dfe9758..ddb5e73 100644
--- a/lib/zip/file.rb
+++ b/lib/zip/file.rb
@@ -90,6 +90,7 @@ module Zip
         end
       elsif buffer && path_or_io.size > 0
         # This zip is probably a non-empty StringIO.
+        @create = false
         read_from_stream(path_or_io)
       elsif @create
         # This zip is completely new/empty and is to be created.
@@ -311,6 +312,8 @@ module Zip
 
     # Write buffer write changes to buffer and return
     def write_buffer(io = ::StringIO.new)
+      return unless commit_required?
+
       ::Zip::OutputStream.write_buffer(io) do |zos|
         @entry_set.each { |e| e.write_to_zip_output_stream(zos) }
         zos.comment = comment
diff --git a/test/file_test.rb b/test/file_test.rb
index cf3619f..d08778a 100644
--- a/test/file_test.rb
+++ b/test/file_test.rb
@@ -111,17 +111,27 @@ class ZipFileTest < MiniTest::Test
   end
 
   def test_open_buffer_with_string
-    string = File.read('test/data/rubycode.zip', mode: 'rb')
+    data = File.read('test/data/rubycode.zip', mode: 'rb')
+    string = data.dup
+
     ::Zip::File.open_buffer string do |zf|
       assert zf.entries.map(&:name).include?('zippedruby1.rb')
     end
+
+    # Ensure the buffer hasn't changed.
+    assert_equal(data, string)
   end
 
   def test_open_buffer_with_stringio
-    string_io = StringIO.new File.read('test/data/rubycode.zip', mode: 'rb')
+    data = File.read('test/data/rubycode.zip', mode: 'rb')
+    string_io = StringIO.new(data.dup)
+
     ::Zip::File.open_buffer string_io do |zf|
       assert zf.entries.map(&:name).include?('zippedruby1.rb')
     end
+
+    # Ensure the buffer hasn't changed.
+    assert_equal(data, string_io.string)
   end
 
   def test_close_buffer_with_stringio
@@ -181,6 +191,18 @@ class ZipFileTest < MiniTest::Test
     assert zf.entries.map(&:name).include?('zippedruby1.rb')
   end
 
+  def test_open_buffer_without_block_write_buffer_does_nothing
+    data = File.read('test/data/rubycode.zip', mode: 'rb')
+    string_io = StringIO.new(data.dup)
+
+    zf = ::Zip::File.open_buffer(string_io)
+    assert zf.entries.map(&:name).include?('zippedruby1.rb')
+
+    # Ensure the buffer isn't changed.
+    zf.write_buffer(string_io)
+    assert_equal(data, string_io.string)
+  end
+
   def test_open_file_with_max_length_comment
     # Should not raise any errors.
     Zip::File.open('test/data/max_length_file_comment.zip')