Skip to content

Add ZipArchive::closeString() #21497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tstarling wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add ZipArchive::closeString() #21497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 55 additions & 11 deletions ext/zip/php_zip.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,8 +573,10 @@ static char * php_zipobj_get_zip_comment(ze_zip_object *obj, int *len) /* {{{ */
}
/* }}} */

/* Close and free the zip_t */
static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */
/* Close and free the zip_t. If out_str is non-null and the archive was opened
* as a string, the final contents of the archive will be assigned to *out_str
* and that string will afterwards be owned by the caller. */
static bool php_zipobj_close(ze_zip_object *obj, zend_string **out_str) /* {{{ */
{
struct zip *intern = obj->za;
bool success = false;
Expand Down Expand Up @@ -606,7 +608,17 @@ static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */
obj->filename_len = 0;
}

if (obj->out_str) {
if (out_str) {
*out_str = obj->out_str;
} else {
zend_string_release(obj->out_str);
}
obj->out_str = NULL;
}
Copy link
Copy Markdown
Member

@DanielEScherzer DanielEScherzer Apr 19, 2026

Choose a reason for hiding this comment

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

if obj->out_str is not set, we should ensure that callers do not try to use the out_str, suggest adding

Suggested change
}
} else {
ZEND_ASSERT(!out_str);
}

Copy link
Copy Markdown
Contributor Author

@tstarling tstarling May 21, 2026

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown
Contributor Author

@tstarling tstarling May 21, 2026

Choose a reason for hiding this comment

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

I had to revert this, since the assertion fails when closeString() is called but something goes wrong with saving the archive. The caller is already guarding for that, and false is returned to userspace. There was already a test, which failed due to the assertion.


obj->za = NULL;
obj->from_string = false;
return success;
}
/* }}} */
Expand Down Expand Up @@ -1060,7 +1072,7 @@ static void php_zip_object_free_storage(zend_object *object) /* {{{ */
{
ze_zip_object * intern = php_zip_fetch_object(object);

php_zipobj_close(intern);
php_zipobj_close(intern, NULL);

#ifdef HAVE_PROGRESS_CALLBACK
/* if not properly called by libzip */
Expand Down Expand Up @@ -1467,7 +1479,7 @@ PHP_METHOD(ZipArchive, open)
}

/* If we already have an opened zip, free it */
php_zipobj_close(ze_obj);
php_zipobj_close(ze_obj, NULL);

/* open for write without option to empty the archive */
if ((flags & (ZIP_TRUNCATE | ZIP_RDONLY)) == 0) {
Expand All @@ -1491,28 +1503,34 @@ PHP_METHOD(ZipArchive, open)
ze_obj->filename = resolved_path;
ze_obj->filename_len = strlen(resolved_path);
ze_obj->za = intern;
ze_obj->from_string = false;
RETURN_TRUE;
}
/* }}} */

/* {{{ Create new read-only zip using given string */
/* {{{ Create new zip from a string, or a create an empty zip to be saved to a string */
PHP_METHOD(ZipArchive, openString)
{
zend_string *buffer;
zend_string *buffer = NULL;
zend_long flags = 0;
zval *self = ZEND_THIS;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "S", &buffer) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|Sl", &buffer, &flags) == FAILURE) {
Comment thread
DanielEScherzer marked this conversation as resolved.
RETURN_THROWS();
}

if (!buffer) {
buffer = ZSTR_EMPTY_ALLOC();
}

ze_zip_object *ze_obj = Z_ZIP_P(self);

php_zipobj_close(ze_obj);
php_zipobj_close(ze_obj, NULL);

zip_error_t err;
zip_error_init(&err);

zip_source_t * zip_source = php_zip_create_string_source(buffer, NULL, &err);
zip_source_t * zip_source = php_zip_create_string_source(buffer, &ze_obj->out_str, &err);

if (!zip_source) {
ze_obj->err_zip = zip_error_code_zip(&err);
Expand All @@ -1521,7 +1539,7 @@ PHP_METHOD(ZipArchive, openString)
RETURN_LONG(ze_obj->err_zip);
}

struct zip *intern = zip_open_from_source(zip_source, ZIP_RDONLY, &err);
struct zip *intern = zip_open_from_source(zip_source, flags, &err);
Comment thread
tstarling marked this conversation as resolved.
if (!intern) {
ze_obj->err_zip = zip_error_code_zip(&err);
ze_obj->err_sys = zip_error_code_system(&err);
Expand All @@ -1530,6 +1548,7 @@ PHP_METHOD(ZipArchive, openString)
RETURN_LONG(ze_obj->err_zip);
}

ze_obj->from_string = true;
ze_obj->za = intern;
zip_error_fini(&err);
RETURN_TRUE;
Expand Down Expand Up @@ -1568,7 +1587,32 @@ PHP_METHOD(ZipArchive, close)

ZIP_FROM_OBJECT(intern, self);

RETURN_BOOL(php_zipobj_close(Z_ZIP_P(self)));
RETURN_BOOL(php_zipobj_close(Z_ZIP_P(self), NULL));
}
/* }}} */

/* {{{ close the zip archive and get the result as a string */
PHP_METHOD(ZipArchive, closeString)
{
struct zip *intern;
zval *self = ZEND_THIS;

ZEND_PARSE_PARAMETERS_NONE();

ZIP_FROM_OBJECT(intern, self);

Comment thread
DanielEScherzer marked this conversation as resolved.
if (!Z_ZIP_P(self)->from_string) {
zend_throw_error(NULL, "ZipArchive::closeString can only be called on "
"an archive opened with ZipArchive::openString");
RETURN_THROWS();
}

zend_string *ret = NULL;
bool success = php_zipobj_close(Z_ZIP_P(self), &ret);
if (success) {
RETURN_STR(ret ? ret : ZSTR_EMPTY_ALLOC());
}
RETURN_FALSE;
}
/* }}} */

Expand Down
2 changes: 2 additions & 0 deletions ext/zip/php_zip.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ typedef struct _ze_zip_object {
HashTable *prop_handler;
char *filename;
size_t filename_len;
zend_string *out_str;
bool from_string;
zip_int64_t last_id;
int err_zip;
int err_sys;
Expand Down
4 changes: 3 additions & 1 deletion ext/zip/php_zip.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ class ZipArchive implements Countable
/** @tentative-return-type */
public function open(string $filename, int $flags = 0): bool|int {}

public function openString(string $data): bool|int {}
public function openString(string $data = '', int $flags = 0): bool|int {}

/**
* @tentative-return-type
Expand All @@ -656,6 +656,8 @@ public function setPassword(#[\SensitiveParameter] string $password): bool {}
/** @tentative-return-type */
public function close(): bool {}

public function closeString(): string|false {}

/** @tentative-return-type */
public function count(): int {}

Expand Down
12 changes: 9 additions & 3 deletions ext/zip/php_zip_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions ext/zip/tests/ZipArchive_closeString_basic.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
--TEST--
ZipArchive::closeString() basic
--EXTENSIONS--
zip
--FILE--
<?php
$zip = new ZipArchive();
$zip->openString();
$zip->addFromString('test1', '1');
$zip->addFromString('test2', '2');
$contents = $zip->closeString();
echo $contents ? "OK\n" : "FAILED\n";

$zip = new ZipArchive();
$zip->openString($contents);
var_dump($zip->getFromName('test1'));
var_dump($zip->getFromName('test2'));
var_dump($zip->getFromName('nonexistent'));

?>
--EXPECT--
OK
string(1) "1"
string(1) "2"
bool(false)
47 changes: 47 additions & 0 deletions ext/zip/tests/ZipArchive_closeString_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
--TEST--
ZipArchive::closeString() error cases
--EXTENSIONS--
zip
--FILE--
<?php
echo "1.\n";
$zip = new ZipArchive();
$zip->openString();
var_dump($zip->open(__DIR__ . '/test.zip'));
try {
$zip->closeString();
} catch (Error $e) {
echo $e->getMessage() . "\n";
}

echo "2.\n";
$zip = new ZipArchive();
$zip->openString('...');
echo $zip->getStatusString() . "\n";
try {
$zip->closeString();
} catch (Error $e) {
echo $e->getMessage() . "\n";
}

echo "3.\n";
$zip = new ZipArchive();
$zip->openString(file_get_contents(__DIR__ . '/test.zip'));
echo gettype($zip->closeString()) . "\n";
try {
$zip->closeString();
} catch (Error $e) {
echo $e->getMessage() . "\n";
}

?>
--EXPECT--
1.
bool(true)
ZipArchive::closeString can only be called on an archive opened with ZipArchive::openString
2.
Not a zip archive
Invalid or uninitialized Zip object
3.
string
Invalid or uninitialized Zip object
22 changes: 22 additions & 0 deletions ext/zip/tests/ZipArchive_closeString_false.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
ZipArchive::closeString() false return
--EXTENSIONS--
zip
--FILE--
<?php
$zip = new ZipArchive();
// The "compressed size" fields are wrong, causing an error when reading the contents.
// The error is reported on close when we rewrite the member with setCompressionIndex().
// The error code is ER_DATA_LENGTH in libzip 1.10.0+ or ER_INCONS otherwise.
$input = file_get_contents(__DIR__ . '/wrong-file-size.zip');
var_dump($zip->openString($input));
$zip->setCompressionIndex(0, ZipArchive::CM_DEFLATE);
var_dump($zip->closeString());
echo $zip->getStatusString() . "\n";
?>
--EXPECTREGEX--
bool\(true\)

Warning: ZipArchive::closeString\(\): (Zip archive inconsistent|Unexpected length of data).*
bool\(false\)
(Zip archive inconsistent|Unexpected length of data)
28 changes: 28 additions & 0 deletions ext/zip/tests/ZipArchive_closeString_variation.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
ZipArchive::closeString() variations
Comment thread
DanielEScherzer marked this conversation as resolved.
--EXTENSIONS--
zip
--FILE--
<?php
echo "1. Empty archive creation\n";
$zip = new ZipArchive();
$zip->openString();
var_dump($zip->closeString());
echo $zip->getStatusString() . "\n";

echo "2. Update existing archive\n";
$input = file_get_contents(__DIR__ . '/test.zip');
$zip = new ZipArchive();
$zip->openString($input);
$zip->addFromString('entry1.txt', '');
$result = $zip->closeString();
echo gettype($result) . "\n";
var_dump($input !== $result);
?>
--EXPECT--
1. Empty archive creation
string(0) ""
No error
2. Update existing archive
string
bool(true)
25 changes: 24 additions & 1 deletion ext/zip/tests/ZipArchive_openString.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ ZipArchive::openString() method
zip
--FILE--
<?php
echo "1. Open read-only and read directory\n";
$input = file_get_contents(__DIR__."/test_procedural.zip");
$zip = new ZipArchive();
$zip->openString(file_get_contents(__DIR__."/test_procedural.zip"));
$zip->openString($input, ZipArchive::RDONLY);

for ($i = 0; $i < $zip->numFiles; $i++) {
$stat = $zip->statIndex($i);
Expand All @@ -17,12 +19,33 @@ var_dump($zip->addFromString("foobar/baz", "baz"));
var_dump($zip->addEmptyDir("blub"));

var_dump($zip->close());

echo "2. CREATE and EXCL flags\n";
$zip = new ZipArchive();
var_dump($zip->openString($input, ZipArchive::CREATE));
var_dump($zip->openString($input, ZipArchive::EXCL));
echo $zip->getStatusString() . "\n";

echo "3. CHECKCONS flag\n";
$inconsistent = file_get_contents(__DIR__ . '/checkcons.zip');
$zip = new ZipArchive();
var_dump($zip->openString($inconsistent));
var_dump($zip->openString($inconsistent, ZipArchive::CHECKCONS));

?>
--EXPECTF--
1. Open read-only and read directory
foo
bar
foobar/
foobar/baz
bool(false)
bool(false)
bool(true)
2. CREATE and EXCL flags
bool(true)
int(10)
File already exists
3. CHECKCONS flag
bool(true)
int(%d)
Binary file added ext/zip/tests/checkcons.zip
View file
Open in desktop
Binary file not shown.
Binary file added ext/zip/tests/wrong-file-size.zip
View file
Open in desktop
Binary file not shown.
Loading