Fix readpartial without over-reading

ruby · Oct 6, 2023 · 462bbf1 · 462bbf1
1 parent 9811fa8
commit 462bbf1
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 11 deletions.
diff --git a/ext/zlib/zlib.c b/ext/zlib/zlib.c
@@ -2947,10 +2947,7 @@ gzfile_readpartial(struct gzfile *gz, long len, VALUE outbuf)
         }
     }
 
-    // fill stream until the buffer can be filled. while this isn't strictly
-    // necessary, it does help cut down on the number of calls to readpartial
-    // in the typical case.
-    while (!ZSTREAM_IS_FINISHED(&gz->z) && ZSTREAM_BUF_FILLED(&gz->z) < len) {
+    while (!ZSTREAM_IS_FINISHED(&gz->z) && ZSTREAM_BUF_FILLED(&gz->z) == 0) {
 	gzfile_read_more(gz, outbuf);
     }
 
@@ -2960,10 +2957,14 @@ gzfile_readpartial(struct gzfile *gz, long len, VALUE outbuf)
     if (GZFILE_IS_FINISHED(gz)) {
 	if (!(gz->z.flags & GZFILE_FLAG_FOOTER_FINISHED)) {
 	    gzfile_check_footer(gz, dst);
-	}
-    if (RSTRING_LEN(dst) <= 0)
+	} else if (RSTRING_LEN(dst) <= 0) {
+        // allow one read past the end of the stream to return an empty string
+        // in case the stream is being read in a loop `until eof?`,
+        // since gzeof will return false until a read is attempted _past_
+        // the end of the stream.
         rb_raise(rb_eEOFError, "end of file reached");
     }
+    }
 
     return dst;
 }
@@ -2978,10 +2979,10 @@ gzfile_read_all(struct gzfile *gz, VALUE dst)
 	if (!(gz->z.flags & GZFILE_FLAG_FOOTER_FINISHED)) {
 	    gzfile_check_footer(gz, dst);
 	}
-    if (!NIL_P(dst)) {
-        rb_str_resize(dst, 0);
-        return dst;
-    }
+        if (!NIL_P(dst)) {
+            rb_str_resize(dst, 0);
+            return dst;
+        }
 	return rb_str_new(0, 0);
     }
 

diff --git a/test/zlib/test_zlib.rb b/test/zlib/test_zlib.rb
@@ -1005,6 +1005,19 @@ def test_read
           assert_equal "".b, s
           assert_predicate f, :eof?
         end
+
+        File.open(File.expand_path("../fixtures/ruby-progressbar-1.13.0.gem.data.tar.gz", __dir__), "rb") do |f|
+          Zlib::GzipReader.wrap(f) do |gzio|
+            count = 0
+            until gzio.eof?
+              s = gzio.read(16_384)
+              count += s.size
+              assert_predicate gzio, :eof? if s.empty?
+            end
+            assert_equal 59904, count
+          end
+          assert_predicate f, :closed?
+        end
       }
     end
 
@@ -1032,24 +1045,33 @@ def test_readpartial
 
       File.open(File.expand_path("../fixtures/ruby-progressbar-1.13.0.gem.data.tar.gz", __dir__), "rb") do |f|
         Zlib::GzipReader.wrap(f) do |gzio|
-          gzio.readpartial(16_384) until gzio.eof?
+          count = 0
+          until gzio.eof?
+            s = gzio.readpartial(16_384)
+            count += s.size
+            assert_predicate gzio, :eof? if s.empty?
+          end
+          assert_equal 59904, count
         end
         assert_predicate f, :closed?
       end
 
       File.open(File.expand_path("../fixtures/ruby-progressbar-1.13.0.gem.data.tar.gz", __dir__), "rb") do |f|
         Zlib::GzipReader.wrap(f) do |gzio|
+          count = 0
           s = String.new(capacity: 16_384, encoding: Encoding::BINARY)
           loop do
             partial = gzio.readpartial(16_384, s)
             assert_same s, partial
+            count += s.size
             if gzio.eof?
               assert_raise(EOFError) { gzio.readpartial(16_384, s) }
               assert_equal "", s
 
               break
             end
           end
+          assert_equal 59904, count
         end
       end
     end