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
Add ZipArchive::closeString() 
- Add a $flags parameter to ZipArchive::openString(), by analogy with
  ZipArchive::open(). This allows the string to be opened read/write.
- Have the $data parameter to ZipArchive::openString() default to an
  empty string, for convenience of callers that want to create an empty
  archive. This works on all versions of libzip since the change in
  1.6.0 only applied to files, it's opt-in for generic sources.
- Add ZipArchive::closeString() which closes the archive and returns
  the resulting string. For consistency with openString(), return an
  empty string if the archive is empty.
  • Loading branch information
@tstarling
tstarling committed Apr 15, 2026
commit 248c02c008b6587e07ae6fa301e2f91444f69778
62 changes: 52 additions & 10 deletions ext/zip/php_zip.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ static char * php_zipobj_get_zip_comment(ze_zip_object *obj, int *len) /* {{{ */
/* }}} */

/* Close and free the zip_t */
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.

this should include some kind of documentation about the fact that the string in the object can be moved out into the out_str parameter

static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */
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 +606,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);
}


obj->za = NULL;
obj->from_string = false;
return success;
}
/* }}} */
Expand Down Expand Up @@ -1060,7 +1070,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 +1477,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 +1501,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 +1537,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);
Copy link
Copy Markdown
Member

@DanielEScherzer DanielEScherzer Apr 1, 2026

Choose a reason for hiding this comment

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

please add some tests for the new support for arbitrary flags - e.g.

  • invalid values that are not real flags
  • maybe: ZIP_EXCL/ZIP_CREATE/ZIP_TRUNCATE - opening from a string means that (if I understand correctly) the source will be a string, and so will always exist with the given content
  • ZIP_RDONLY - trying to modify that source will result in errors I assume?

Copy link
Copy Markdown
Contributor Author

@tstarling tstarling Apr 2, 2026

Choose a reason for hiding this comment

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

There's no validation for invalid values that are not real flags. If libzip defines those values in future, the behaviour will change. It's a bit dodgy in my opinion, but there are other things doing this. I considered fixing it, but it's a pre-existing issue in ZipArchive::open() which I'm following by analogy so I decided it's not a topic for this PR.

ZIP_EXCL and ZIP_CREATE are evaluated on open by doing a stat call on the source. The stat always succeeds, so ZIP_CREATE always succeeds and ZIP_EXCL always fails. I added tests.

ZIP_RDONLY is already covered by ZipArchive_openString.phpt.

There is no ZipArchive::TRUNCATE. The behaviour of passing 8 is undefined. I tested it manually and it doesn't crash.

I added a test for ZIP_CHECKCONS.

Copy link
Copy Markdown
Member

@DanielEScherzer DanielEScherzer Apr 2, 2026

Choose a reason for hiding this comment

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

There's no validation for invalid values that are not real flags. If libzip defines those values in future, the behaviour will change. It's a bit dodgy in my opinion, but there are other things doing this. I considered fixing it, but it's a pre-existing issue in ZipArchive::open() which I'm following by analogy so I decided it's not a topic for this PR.

Okay, no need to address it in this PR but we (collectively as PHP developers, not necessarily you and I) should probably add validation

if (!intern) {
ze_obj->err_zip = zip_error_code_zip(&err);
ze_obj->err_sys = zip_error_code_system(&err);
Expand All @@ -1530,6 +1546,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 +1585,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;
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.

Suggested change
zend_string * ret = NULL;
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;
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.

Suggested change
zend_string * out_str;
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.\n";
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.

can you give these a bit more helpful names in the output?

$zip = new ZipArchive();
$zip->openString();
var_dump($zip->closeString());
echo $zip->getStatusString() . "\n";

echo "2.\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.
string(0) ""
No error
2.
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.\n";
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.

can you give these a bit more helpful names in the output?

$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.\n";
$zip = new ZipArchive();
var_dump($zip->openString($input, ZipArchive::CREATE));
var_dump($zip->openString($input, ZipArchive::EXCL));
echo $zip->getStatusString() . "\n";

echo "3.\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.
foo
bar
foobar/
foobar/baz
bool(false)
bool(false)
bool(true)
2.
bool(true)
int(10)
File already exists
3.
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