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>

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! >>>

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
 ========================================

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);
 

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;

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
      */

ext/zip/php_zip_arginfo.h

@@ -1,5 +1,5 @@
 /* This is a generated file, edit php_zip.stub.php instead.
- * Stub hash: 030038017c7c76195c0b0dc2392696f458c2471f */
+ * Stub hash: e04b3e90c42074ac364ea25a0e794815bd4271e5 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_zip_open, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0)
@@ -45,6 +45,10 @@ ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_MASK_EX(arginfo_class_ZipArchive_open,
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, flags, IS_LONG, 0, "0")
 ZEND_END_ARG_INFO()
 
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_ZipArchive_openString, 0, 1, MAY_BE_BOOL|MAY_BE_LONG)
+	ZEND_ARG_TYPE_INFO(0, data, IS_STRING, 0)
+ZEND_END_ARG_INFO()
+
 ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_ZipArchive_setPassword, 0, 1, _IS_BOOL, 0)
 	ZEND_ARG_TYPE_INFO(0, password, IS_STRING, 0)
 ZEND_END_ARG_INFO()
@@ -313,6 +317,7 @@ ZEND_FUNCTION(zip_entry_compressedsize);
 ZEND_FUNCTION(zip_entry_filesize);
 ZEND_FUNCTION(zip_entry_compressionmethod);
 ZEND_METHOD(ZipArchive, open);
+ZEND_METHOD(ZipArchive, openString);
 ZEND_METHOD(ZipArchive, setPassword);
 ZEND_METHOD(ZipArchive, close);
 ZEND_METHOD(ZipArchive, count);
@@ -391,6 +396,7 @@ static const zend_function_entry ext_functions[] = {
 
 static const zend_function_entry class_ZipArchive_methods[] = {
 	ZEND_ME(ZipArchive, open, arginfo_class_ZipArchive_open, ZEND_ACC_PUBLIC)
+	ZEND_ME(ZipArchive, openString, arginfo_class_ZipArchive_openString, ZEND_ACC_PUBLIC)
 	ZEND_ME(ZipArchive, setPassword, arginfo_class_ZipArchive_setPassword, ZEND_ACC_PUBLIC)
 	ZEND_ME(ZipArchive, close, arginfo_class_ZipArchive_close, ZEND_ACC_PUBLIC)
 	ZEND_ME(ZipArchive, count, arginfo_class_ZipArchive_count, ZEND_ACC_PUBLIC)

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)

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--

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
+"