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

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;
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.\n";
$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";
$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