-
Notifications
You must be signed in to change notification settings - Fork 8k
ext/zip: add ZipArchive::openString method #21205
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
ab299ba
8991220
c676ae4
1d8a567
5a83207
d684590
4073b1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just return 0? Having a return type of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're prepared to take responsibility for this PR and hit the merge button when it meets your requirements, I'll make it return whatever you want.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For forwards compatibility and consistency with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there is a reason to use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I changed it to |
||
| } | ||
| /* }}} */ | ||
|
|
||
| /* {{{ Set the password for the active archive */ | ||
| PHP_METHOD(ZipArchive, setPassword) | ||
| { | ||
|
|
@@ -1515,42 +1590,14 @@ 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(); | ||
| } | ||
|
|
||
| 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); | ||
|
|
||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
tstarling marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ?> | ||
| --EXPECT-- | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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')); | ||
tstarling marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ?> | ||
| --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 | ||
| " | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use
Z_ZIP_P(ZEND_THIS)instead of storingZEND_THISin a variable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every method in this class uses this pattern. I can clean it up in a separate PR if you like. Maybe it makes sense for performance if ZEND_THIS is accessed more than once, but the original author decided to be consistent about it regardless of how many times it's accessed.