Commit 605c0756c92 for php.net
commit 605c0756c92f9da020d501e7c5199d045812697c
Author: Tim Starling <tstarling@wikimedia.org>
Date: Mon Apr 29 19:48:00 2024 +0200
ext/zip: add ZipArchive::openString() method
Fixes to GH-14078:
* Rename ZipArchive::openBuffer() to ::openString().
* For consistency with ::open(), return int|bool, don't throw an
exception on error. Provide error information via existing properties
and accessors.
* Fix memory leak when ::openString() is called but ::close() is not
called. Add test.
* Fix memory leak when a call to ::open() is followed by a call to
::openString(). Add test.
* Let libzip own the source, don't call zip_source_keep().
* Share buffer handling with ZipArchive::addFromString().
Elsewhere:
* If there is an error from zip_close() during a call to
ZipArchive::open(), emit a warning but proceed to open the archive,
don't return early. Add test.
* When buffers are saved by ZipArchive::addFromString(), release them
in ZipArchive::close() and ::open(), don't accumulate buffers until
the free_obj handler is called.
* Factor out buffer handling and reuse it in ZipArchive::openString()
Closes GH-21205.
Closes GH-14078.
Co-authored-by: Soner Sayakci <s.sayakci@shopware.com>
Co-authored-by: Ghaith Olabi <24876890+Gaitholabi@users.noreply.github.com>
diff --git a/NEWS b/NEWS
index 43f5358ecc8..4dcac8bb21e 100644
--- a/NEWS
+++ b/NEWS
@@ -151,5 +151,7 @@ PHP NEWS
(ilutov)
. Support minimum version for libzip dependency updated to 1.0.0.
(David Carlier)
+ . Added ZipArchive::openString() method.
+ (Tim Starling, Soner Sayakci, Ghaith Olabi)
<<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>
diff --git a/UPGRADING b/UPGRADING
index 595a49e41e4..113258ebbd3 100644
--- a/UPGRADING
+++ b/UPGRADING
@@ -154,6 +154,9 @@ PHP 8.6 UPGRADE NOTES
bound.
RFC: https://wiki.php.net/rfc/clamp_v2
+- Zip:
+ . Added ZipArchive::openString() method.
+
========================================
7. New Classes and Interfaces
========================================
diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c
index 7198fe99d31..c6f516e3a26 100644
--- a/ext/zip/php_zip.c
+++ b/ext/zip/php_zip.c
@@ -575,6 +575,68 @@ static char * php_zipobj_get_zip_comment(ze_zip_object *obj, int *len) /* {{{ */
}
/* }}} */
+/* Add a string to the list of buffers to be released when the object is destroyed.*/
+static void php_zipobj_add_buffer(ze_zip_object *obj, zend_string *str) /* {{{ */
+{
+ size_t pos = obj->buffers_cnt++;
+ obj->buffers = safe_erealloc(obj->buffers, sizeof(*obj->buffers), obj->buffers_cnt, 0);
+ obj->buffers[pos] = zend_string_copy(str);
+}
+/* }}} */
+
+static void php_zipobj_release_buffers(ze_zip_object *obj) /* {{{ */
+{
+ if (obj->buffers_cnt > 0) {
+ for (size_t i = 0; i < obj->buffers_cnt; i++) {
+ zend_string_release(obj->buffers[i]);
+ }
+ efree(obj->buffers);
+ obj->buffers = NULL;
+ }
+ obj->buffers_cnt = 0;
+}
+/* }}} */
+
+/* Close and free the zip_t */
+static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */
+{
+ struct zip *intern = obj->za;
+ bool success = false;
+
+ if (intern) {
+ int err = zip_close(intern);
+ if (err) {
+ php_error_docref(NULL, E_WARNING, "%s", zip_strerror(intern));
+ /* Save error for property reader */
+ zip_error_t *ziperr = zip_get_error(intern);
+ obj->err_zip = zip_error_code_zip(ziperr);
+ obj->err_sys = zip_error_code_system(ziperr);
+ zip_error_fini(ziperr);
+ zip_discard(intern);
+ } else {
+ obj->err_zip = 0;
+ obj->err_sys = 0;
+ }
+ success = !err;
+ }
+
+ /* if we have a filename, we need to free it */
+ if (obj->filename) {
+ /* clear cache as empty zip are not created but deleted */
+ php_clear_stat_cache(1, obj->filename, obj->filename_len);
+
+ efree(obj->filename);
+ obj->filename = NULL;
+ obj->filename_len = 0;
+ }
+
+ php_zipobj_release_buffers(obj);
+
+ obj->za = NULL;
+ return success;
+}
+/* }}} */
+
int php_zip_glob(zend_string *spattern, zend_long flags, zval *return_value) /* {{{ */
{
int cwd_skip = 0;
@@ -1021,21 +1083,8 @@ static void php_zip_object_dtor(zend_object *object)
static void php_zip_object_free_storage(zend_object *object) /* {{{ */
{
ze_zip_object * intern = php_zip_fetch_object(object);
- int i;
-
- if (intern->za) {
- if (zip_close(intern->za) != 0) {
- php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za));
- zip_discard(intern->za);
- }
- }
- if (intern->buffers_cnt>0) {
- for (i=0; i<intern->buffers_cnt; i++) {
- zend_string_release(intern->buffers[i]);
- }
- efree(intern->buffers);
- }
+ php_zipobj_close(intern);
#ifdef HAVE_PROGRESS_CALLBACK
/* if not properly called by libzip */
@@ -1047,12 +1096,7 @@ static void php_zip_object_free_storage(zend_object *object) /* {{{ */
php_zip_cancel_callback_free(intern);
#endif
- intern->za = NULL;
zend_object_std_dtor(&intern->zo);
-
- if (intern->filename) {
- efree(intern->filename);
- }
}
/* }}} */
@@ -1448,19 +1492,8 @@ PHP_METHOD(ZipArchive, open)
RETURN_FALSE;
}
- if (ze_obj->za) {
- /* we already have an opened zip, free it */
- if (zip_close(ze_obj->za) != 0) {
- php_error_docref(NULL, E_WARNING, "Empty string as source");
- efree(resolved_path);
- RETURN_FALSE;
- }
- ze_obj->za = NULL;
- }
- if (ze_obj->filename) {
- efree(ze_obj->filename);
- ze_obj->filename = NULL;
- }
+ /* If we already have an opened zip, free it */
+ php_zipobj_close(ze_obj);
/* open for write without option to empty the archive */
if ((flags & (ZIP_TRUNCATE | ZIP_RDONLY)) == 0) {
@@ -1488,6 +1521,48 @@ PHP_METHOD(ZipArchive, open)
}
/* }}} */
+/* {{{ Create new read-only zip using given string */
+PHP_METHOD(ZipArchive, openString)
+{
+ zend_string *buffer;
+ zval *self = ZEND_THIS;
+
+ if (zend_parse_parameters(ZEND_NUM_ARGS(), "S", &buffer) == FAILURE) {
+ RETURN_THROWS();
+ }
+
+ ze_zip_object *ze_obj = Z_ZIP_P(self);
+
+ zip_error_t err;
+ zip_error_init(&err);
+
+ zip_source_t * zip_source = zip_source_buffer_create(ZSTR_VAL(buffer), ZSTR_LEN(buffer), 0, &err);
+
+ if (!zip_source) {
+ ze_obj->err_zip = zip_error_code_zip(&err);
+ ze_obj->err_sys = zip_error_code_system(&err);
+ zip_error_fini(&err);
+ RETURN_LONG(ze_obj->err_zip);
+ }
+
+ php_zipobj_close(ze_obj);
+
+ struct zip *intern = zip_open_from_source(zip_source, ZIP_RDONLY, &err);
+ if (!intern) {
+ ze_obj->err_zip = zip_error_code_zip(&err);
+ ze_obj->err_sys = zip_error_code_system(&err);
+ zip_error_fini(&err);
+ zip_source_free(zip_source);
+ RETURN_LONG(ze_obj->err_zip);
+ }
+
+ php_zipobj_add_buffer(ze_obj, buffer);
+ ze_obj->za = intern;
+ zip_error_fini(&err);
+ RETURN_TRUE;
+}
+/* }}} */
+
/* {{{ Set the password for the active archive */
PHP_METHOD(ZipArchive, setPassword)
{
@@ -1515,8 +1590,6 @@ PHP_METHOD(ZipArchive, close)
{
struct zip *intern;
zval *self = ZEND_THIS;
- ze_zip_object *ze_obj;
- int err;
if (zend_parse_parameters_none() == FAILURE) {
RETURN_THROWS();
@@ -1524,33 +1597,7 @@ PHP_METHOD(ZipArchive, close)
ZIP_FROM_OBJECT(intern, self);
- ze_obj = Z_ZIP_P(self);
-
- err = zip_close(intern);
- if (err) {
- php_error_docref(NULL, E_WARNING, "%s", zip_strerror(intern));
- /* Save error for property reader */
- zip_error_t *ziperr;
-
- ziperr = zip_get_error(intern);
- ze_obj->err_zip = zip_error_code_zip(ziperr);
- ze_obj->err_sys = zip_error_code_system(ziperr);
- zip_error_fini(ziperr);
- zip_discard(intern);
- } else {
- ze_obj->err_zip = 0;
- ze_obj->err_sys = 0;
- }
-
- /* clear cache as empty zip are not created but deleted */
- php_clear_stat_cache(1, ze_obj->filename, ze_obj->filename_len);
-
- efree(ze_obj->filename);
- ze_obj->filename = NULL;
- ze_obj->filename_len = 0;
- ze_obj->za = NULL;
-
- RETURN_BOOL(!err);
+ RETURN_BOOL(php_zipobj_close(Z_ZIP_P(self)));
}
/* }}} */
@@ -1864,7 +1911,6 @@ PHP_METHOD(ZipArchive, addFromString)
size_t name_len;
ze_zip_object *ze_obj;
struct zip_source *zs;
- int pos = 0;
zend_long flags = ZIP_FL_OVERWRITE;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sS|l",
@@ -1875,15 +1921,7 @@ PHP_METHOD(ZipArchive, addFromString)
ZIP_FROM_OBJECT(intern, self);
ze_obj = Z_ZIP_P(self);
- if (ze_obj->buffers_cnt) {
- ze_obj->buffers = safe_erealloc(ze_obj->buffers, sizeof(*ze_obj->buffers), (ze_obj->buffers_cnt + 1), 0);
- pos = ze_obj->buffers_cnt++;
- } else {
- ze_obj->buffers = emalloc(sizeof(*ze_obj->buffers));
- ze_obj->buffers_cnt++;
- pos = 0;
- }
- ze_obj->buffers[pos] = zend_string_copy(buffer);
+ php_zipobj_add_buffer(ze_obj, buffer);
zs = zip_source_buffer(intern, ZSTR_VAL(buffer), ZSTR_LEN(buffer), 0);
diff --git a/ext/zip/php_zip.h b/ext/zip/php_zip.h
index a408f2affbe..0b96d6ec3ea 100644
--- a/ext/zip/php_zip.h
+++ b/ext/zip/php_zip.h
@@ -71,8 +71,8 @@ typedef struct _ze_zip_object {
zend_string **buffers;
HashTable *prop_handler;
char *filename;
- int filename_len;
- int buffers_cnt;
+ size_t filename_len;
+ size_t buffers_cnt;
zip_int64_t last_id;
int err_zip;
int err_sys;
diff --git a/ext/zip/php_zip.stub.php b/ext/zip/php_zip.stub.php
index 2f055e83d34..42dfb006fc6 100644
--- a/ext/zip/php_zip.stub.php
+++ b/ext/zip/php_zip.stub.php
@@ -646,6 +646,8 @@ class ZipArchive implements Countable
/** @tentative-return-type */
public function open(string $filename, int $flags = 0): bool|int {}
+ public function openString(string $data): bool|int {}
+
/**
* @tentative-return-type
*/
diff --git a/ext/zip/php_zip_arginfo.h b/ext/zip/php_zip_arginfo.h
index f3d081daf04..89dd33b6f1a 100644
Binary files a/ext/zip/php_zip_arginfo.h and b/ext/zip/php_zip_arginfo.h differ
diff --git a/ext/zip/tests/ZipArchive_openString.phpt b/ext/zip/tests/ZipArchive_openString.phpt
new file mode 100644
index 00000000000..f787b4a8493
--- /dev/null
+++ b/ext/zip/tests/ZipArchive_openString.phpt
@@ -0,0 +1,28 @@
+--TEST--
+ZipArchive::openString() method
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+$zip = new ZipArchive();
+$zip->openString(file_get_contents(__DIR__."/test_procedural.zip"));
+
+for ($i = 0; $i < $zip->numFiles; $i++) {
+ $stat = $zip->statIndex($i);
+ echo $stat['name'] . "\n";
+}
+
+// Zip is read-only, not allowed
+var_dump($zip->addFromString("foobar/baz", "baz"));
+var_dump($zip->addEmptyDir("blub"));
+
+var_dump($zip->close());
+?>
+--EXPECTF--
+foo
+bar
+foobar/
+foobar/baz
+bool(false)
+bool(false)
+bool(true)
diff --git a/ext/zip/tests/ZipArchive_openString_leak.phpt b/ext/zip/tests/ZipArchive_openString_leak.phpt
new file mode 100644
index 00000000000..06a27cd0688
--- /dev/null
+++ b/ext/zip/tests/ZipArchive_openString_leak.phpt
@@ -0,0 +1,26 @@
+--TEST--
+ZipArchive::openString leak tests
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+$zip = new ZipArchive;
+$zip->openString(file_get_contents(__DIR__ . '/test.zip'));
+$zip = null;
+
+$zip = new ZipArchive;
+$zip->openString(file_get_contents(__DIR__ . '/test.zip'));
+$zip->openString(file_get_contents(__DIR__ . '/test.zip'));
+$zip = null;
+
+$zip = new ZipArchive;
+$zip->openString(file_get_contents(__DIR__ . '/test.zip'));
+$zip->open(__DIR__ . '/test.zip');
+$zip = null;
+
+$zip = new ZipArchive;
+$zip->open(__DIR__ . '/test.zip');
+$zip->openString(file_get_contents(__DIR__ . '/test.zip'));
+$zip = null;
+?>
+--EXPECT--
diff --git a/ext/zip/tests/ZipArchive_open_with_close_fail.phpt b/ext/zip/tests/ZipArchive_open_with_close_fail.phpt
new file mode 100644
index 00000000000..53ef907753a
--- /dev/null
+++ b/ext/zip/tests/ZipArchive_open_with_close_fail.phpt
@@ -0,0 +1,23 @@
+--TEST--
+zip_close() failure from ZipArchive::open()
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+// Try to create an archive
+$zip = new ZipArchive();
+var_dump($zip->open(__DIR__ . '/close-fail.zip', ZipArchive::CREATE));
+file_put_contents(__DIR__.'/close-fail-file', '');
+$zip->addFile(__DIR__.'/close-fail-file');
+// Delete the file to trigger an error on close
+@unlink(__DIR__.'/close-fail-file');
+var_dump($zip->open(__DIR__ . '/test.zip', ZipArchive::RDONLY));
+var_dump($zip->getFromName('bar'));
+?>
+--EXPECTF--
+bool(true)
+
+Warning: ZipArchive::open(): Can't open file: No such file or directory in %s on line %d
+bool(true)
+string(4) "bar
+"