Fix File.open_buffer when no changes are made.

Things are now more carefully set up, and if a buffer is passed in which
represents a file that already exists then this is taken into account. All
initialization is now done in File.new, rather than being split between there
and File.open_buffer.

This has also needed a bit of a re-write of Zip::File.initialize. I've tried to
bring some logic to it as a result, and have added comments to explain what is
now happening.

lib/zip/file.rb

@@ -69,19 +69,33 @@ def initialize(path_or_io, create = false, buffer = false, options = {})
       @name    = path_or_io.respond_to?(:path) ? path_or_io.path : path_or_io
       @comment = ''
       @create  = create ? true : false # allow any truthy value to mean true
-      if !buffer && ::File.size?(@name)
+
+      if ::File.size?(@name.to_s)
+        # There is a file, which exists, that is associated with this zip.
         @create = false
         @file_permissions = ::File.stat(@name).mode
-        ::File.open(@name, 'rb') do |f|
-          read_from_stream(f)
+
+        if buffer
+          read_from_stream(path_or_io)
+        else
+          ::File.open(@name, 'rb') do |f|
+            read_from_stream(f)
+          end
         end
+      elsif buffer && path_or_io.size > 0
+        # This zip is probably a non-empty StringIO.
+        read_from_stream(path_or_io)
       elsif @create
+        # This zip is completely new/empty and is to be created.
         @entry_set = EntrySet.new
       elsif ::File.zero?(@name)
+        # A file exists, but it is empty.
         raise Error, "File #{@name} has zero size. Did you mean to pass the create flag?"
       else
+        # Everything is wrong.
         raise Error, "File #{@name} not found"
       end
+
       @stored_entries      = @entry_set.dup
       @stored_comment      = @comment
       @restore_ownership   = options[:restore_ownership]    || false
@@ -119,16 +133,18 @@ def open_buffer(io, options = {})
         unless IO_METHODS.map { |method| io.respond_to?(method) }.all? || io.is_a?(String)
           raise "Zip::File.open_buffer expects a String or IO-like argument (responds to #{IO_METHODS.join(', ')}). Found: #{io.class}"
         end
+
         if io.is_a?(::String)
           io = ::StringIO.new(io)
         elsif io.respond_to?(:binmode)
           # https://github.com/rubyzip/rubyzip/issues/119
           io.binmode
         end
+
         zf = ::Zip::File.new(io, true, true, options)
-        zf.read_from_stream(io)
         return zf unless block_given?
         yield zf
+
         begin
           zf.write_buffer(io)
         rescue IOError => e

test/file_test.rb

@@ -107,13 +107,14 @@ def test_open_buffer_with_stringio
   def test_close_buffer_with_stringio
     string_io = StringIO.new File.read('test/data/rubycode.zip')
     zf = ::Zip::File.open_buffer string_io
-    assert(zf.close || true) # Poor man's refute_raises
+    assert_nil zf.close
   end
 
   def test_close_buffer_with_io
     f = File.open('test/data/rubycode.zip')
     zf = ::Zip::File.open_buffer f
-    assert zf.close
+    refute zf.commit_required?
+    assert_nil zf.close
     f.close
   end