ZipArchive->getStreamIndex(0) aborts after 790 of 1800 lines

[Wilfried-HD]

Description

The following code:

$z = new ZipArchive();
$res = $z->open(sys_get_temp_dir().'/temp.zip',ZipArchive::RDONLY |ZipArchive::CHECKCONS);
$datei = $z->getStreamIndex(0,ZipArchive::FL_UNCHANGED);

while ($z = fgets($datei)){
...
delivers only 950 Lines of 1800 in the file - aprox. 100 kB.
Ommitting the Flag ZipArchive::FL_UNCHANGED does not make any difference.

51000-0002_de_flat.zip

51000-0002_de_flat.zip

The following code works even with much larger files:
$z = new ZipArchive();
$res = $z->open(sys_get_temp_dir().'/temp.zip',ZipArchive::RDONLY |ZipArchive::CHECKCONS);
$fileName = $z->getNameIndex(0);
$z->close();
$datei = fopen('zip://'.sys_get_temp_dir().'/temp.zip#'.$fileName, 'r');

Tested with
PHP 8.2.26 Development Server in Eclipse

PHP Version

PHP 8.2.26

Operating System

Debian 12

[cmb69]

I can confirm the issue (master branch, libzip 1.11.2, Windows). For debugging, I've applied:

 ext/zip/zip_stream.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ext/zip/zip_stream.c b/ext/zip/zip_stream.c
index a32324347d..be8cbbadf4 100644
--- a/ext/zip/zip_stream.c
+++ b/ext/zip/zip_stream.c
@@ -66,6 +66,7 @@ static ssize_t php_zip_ops_read(php_stream *stream, char *buf, size_t count)
 		/* cast count to signed value to avoid possibly negative n
 		 * being cast to unsigned value */
 		if (n == 0 || n < (ssize_t)count) {
+			int64_t pos = zip_ftell(self->zf);
 			stream->eof = 1;
 		} else {
 			self->cursor += n;

When the new line is hit, pos == 114688, while n == 2550, although count == 8192. So apparently, zip_fread() fails to read further bytes, although there are plenty of these. There might be an issue in libzip.

[Wilfried-HD]

But why does
$datei = fopen('zip://'.sys_get_temp_dir().'/temp.zip#'.$fileName, 'r');
work?
Doesn't it use the libzip-library as well?

Now it seems to me like a buffer-problem: When the program arrives at the end of the buffer, it should try to continue by reading the zip-file, but it returns EOF instead.

[cmb69]

Gosh, that issue drove me mad! Try:

$z = new ZipArchive();
$res = $z->open(sys_get_temp_dir().'/temp.zip',ZipArchive::RDONLY |ZipArchive::CHECKCONS);
$datei = $z->getStreamIndex(0,ZipArchive::FL_UNCHANGED);
while ($line = fgets($datei)){
    // ...
}

You were freeing the ZipArchive when assigning to $z again in the while loop. This went unnoticed, due to

php-src/ext/zip/zip_stream.c

Line 254 in 78d934a

self->za = NULL; /* to keep it open on stream close */

(an unfortunately needed hack)

[Wilfried-HD]

You are right. If I don't reuse the identifier $z, it works as expected.
Sorry and thanks.

Should there be a note in the manual like:
The ZipArchive is closed, when anything else is assigned to its reference.
Strange Example:
$z = new ZipArchive();
$z->open('zipfile.zip');
$zx = $z;
$zx = 'This assignement also closes $z, as it is the same object';

`

[cmb69]

The ZipArchive is closed, when anything else is assigned to its reference.

That is not special for ZipArchive, but rather generally applies to objects and resources. See https://www.php.net/manual/en/features.gc.refcounting-basics.php. Usually, you notice this quickly, e.g.

$z = fopen($filename, "rb");
while ($z = fread($z, 10)) {
    var_dump($z);
}

This throws TypeError: fread(): Argument #1 ($stream) must be of type resource, string given.

Your "strange example" is also not a problem; when the new zipArchive is assigned to $z, its refcount is 1. When assigning $z to $zx, the refcount is increased to 2. When a string is assigned to $zx, the refcount is decreased to 1, so still above 0, so the archive is not closed.

However, in your original example the situation is more complex. since you're getting a stream from the ZipArchive; the hack mentioned above caters to the case where the stream is closed (e.g. fclose($datei)), so that the ZipArchive stays open in this case. The problem is when the stream stays open, but the ZipArchive is closed.

Thinking about that, there is a bug. Even when the ZipArchive is destroyed, we must not close the underlying zip_t object, if there are still open streams using it. Re-opening.