Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ PHP NEWS
. Fixed bug GH-22395 (base_convert() outputs at most 64 characters).
(Weilin Du)

- Zip:
. Fixed bug GH-17787 (ZipArchive stream stops reading early when the archive
is freed while the stream is still open). (Eyüp Can Akman)

02 Jul 2026, PHP 8.4.23

- Core:
Expand Down
2 changes: 1 addition & 1 deletion ext/zip/php_zip.c
Original file line number Diff line number Diff line change
Expand Up @@ -3024,7 +3024,7 @@ static void php_zip_get_stream(INTERNAL_FUNCTION_PARAMETERS, int type, bool acce
PHP_ZIP_STAT_INDEX(intern, index, flags, sb);
}

stream = php_stream_zip_open(intern, &sb, mode, flags STREAMS_CC);
stream = php_stream_zip_open(Z_ZIP_P(self), &sb, mode, flags STREAMS_CC);
if (stream) {
php_stream_to_zval(stream, return_value);
} else {
Expand Down
2 changes: 1 addition & 1 deletion ext/zip/php_zip.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ static inline ze_zip_object *php_zip_fetch_object(zend_object *obj) {
#define Z_ZIP_P(zv) php_zip_fetch_object(Z_OBJ_P((zv)))

php_stream *php_stream_zip_opener(php_stream_wrapper *wrapper, const char *path, const char *mode, int options, zend_string **opened_path, php_stream_context *context STREAMS_DC);
php_stream *php_stream_zip_open(struct zip *arch, struct zip_stat *sb, const char *mode, zip_flags_t flags STREAMS_DC);
php_stream *php_stream_zip_open(ze_zip_object *obj, struct zip_stat *sb, const char *mode, zip_flags_t flags STREAMS_DC);

@devnexen devnexen Aug 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say it is probably possible to avoid this change in a stable branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

php_stream_zip_open has a single caller in php_zip.c, the symbol isn't exported (no PHPAPI, hidden visibility) and php_zip.h isn't installed, so the prototype change isn't visible outside the tree. Can keep the old signature and add a second function that takes the object if you'd rather avoid it on 8.4.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No you re right.
Would it be possible however to update the test as follow ?

diff --git a/ext/zip/tests/gh17787.phpt b/ext/zip/tests/gh17787.phpt
index c05558191cf..f82cfa1145b 100644
--- a/ext/zip/tests/gh17787.phpt
+++ b/ext/zip/tests/gh17787.phpt
@@ -30,11 +30,51 @@

 var_dump(stream_get_contents($stream) === $data);
 fclose($stream);
+
+// Same with getStream()
+$zip = new ZipArchive;
+$zip->open($name, ZipArchive::RDONLY);
+$stream = $zip->getStream('entry.txt');
+$zip = null;
+
+var_dump(stream_get_contents($stream) === $data);
+fclose($stream);
+
+// Pending changes are still committed once the last stream is closed
+$name = __DIR__ . '/gh17787_write.zip';
+
+$zip = new ZipArchive;
+var_dump($zip->open($name, ZipArchive::CREATE | ZipArchive::OVERWRITE));
+$zip->addFromString('first.txt', 'first');
+$zip->close();
+
+$zip = new ZipArchive;
+var_dump($zip->open($name));
+$zip->addFromString('second.txt', 'second');
+$stream = $zip->getStreamName('first.txt', ZipArchive::FL_UNCHANGED);
+$zip = null;
+
+var_dump(stream_get_contents($stream));
+fclose($stream);
+
+$zip = new ZipArchive;
+var_dump($zip->open($name, ZipArchive::RDONLY));
+var_dump($zip->numFiles);
+var_dump($zip->getFromName('second.txt'));
+$zip->close();
 ?>
 --CLEAN--
 <?php
 @unlink(__DIR__ . '/gh17787.zip');
+@unlink(__DIR__ . '/gh17787_write.zip');
 ?>
 --EXPECT--
 bool(true)
 bool(true)
+bool(true)
+bool(true)
+bool(true)
+string(5) "first"
+bool(true)
+int(2)
+string(6) "second"


extern const php_stream_wrapper php_stream_zip_wrapper;

Expand Down
40 changes: 40 additions & 0 deletions ext/zip/tests/gh17787.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
--TEST--
GH-17787 (ZipArchive stream stops reading early when the archive is freed while the stream is open)
--EXTENSIONS--
zip
--FILE--
<?php
$name = __DIR__ . '/gh17787.zip';
$data = str_repeat("The quick brown fox jumps over the lazy dog.\n", 4000);

$zip = new ZipArchive;
$zip->open($name, ZipArchive::CREATE | ZipArchive::OVERWRITE);
$zip->addFromString('entry.txt', $data);
$zip->close();

$zip = new ZipArchive;
$zip->open($name, ZipArchive::RDONLY);
$stream = $zip->getStreamIndex(0, ZipArchive::FL_UNCHANGED);

// Free the archive while the stream is still open
$zip = null;

var_dump(stream_get_contents($stream) === $data);
fclose($stream);

// Same with getStreamName()
$zip = new ZipArchive;
$zip->open($name, ZipArchive::RDONLY);
$stream = $zip->getStreamName('entry.txt', ZipArchive::FL_UNCHANGED);
$zip = null;

var_dump(stream_get_contents($stream) === $data);
fclose($stream);
?>
--CLEAN--
<?php
@unlink(__DIR__ . '/gh17787.zip');
?>
--EXPECT--
bool(true)
bool(true)
14 changes: 13 additions & 1 deletion ext/zip/zip_stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ struct php_zip_stream_data_t {
struct zip_file *zf;
size_t cursor;
php_stream *stream;
ze_zip_object *owner;
};

#define STREAM_DATA_FROM_STREAM() \
Expand Down Expand Up @@ -101,6 +102,12 @@ static int php_zip_ops_close(php_stream *stream, int close_handle)
self->za = NULL;
}
}

/* the pinned object ref is tied to self, so release it regardless of close_handle */
if (self->owner) {
OBJ_RELEASE(&self->owner->zo);
self->owner = NULL;
}
efree(self);
stream->abstract = NULL;
return EOF;
Expand Down Expand Up @@ -234,8 +241,9 @@ const php_stream_ops php_stream_zipio_ops = {
};

/* {{{ php_stream_zip_open */
php_stream *php_stream_zip_open(struct zip *arch, struct zip_stat *sb, const char *mode, zip_flags_t flags STREAMS_DC)
php_stream *php_stream_zip_open(ze_zip_object *obj, struct zip_stat *sb, const char *mode, zip_flags_t flags STREAMS_DC)
{
struct zip *arch = obj->za;
struct zip_file *zf = NULL;

php_stream *stream = NULL;
Expand All @@ -254,6 +262,9 @@ php_stream *php_stream_zip_open(struct zip *arch, struct zip_stat *sb, const cha
self->zf = zf;
self->stream = NULL;
self->cursor = 0;
/* keep the archive object alive while the stream borrows its zip_t */
self->owner = obj;
GC_ADDREF(&obj->zo);
#if LIBZIP_ATLEAST(1,9,1)
if (zip_file_is_seekable(zf) > 0) {
stream = php_stream_alloc(&php_stream_zipio_seek_ops, self, NULL, mode);
Expand Down Expand Up @@ -339,6 +350,7 @@ php_stream *php_stream_zip_opener(php_stream_wrapper *wrapper,
self->zf = zf;
self->stream = NULL;
self->cursor = 0;
self->owner = NULL;
#if LIBZIP_ATLEAST(1,9,1)
if (zip_file_is_seekable(zf) > 0) {
stream = php_stream_alloc(&php_stream_zipio_seek_ops, self, NULL, mode);
Expand Down
Loading