mirror of
https://github.com/php/php-src.git
synced 2026-03-24 00:02:20 +01:00
Fix ZipArchive callback being called after executor has shut down
free_obj() for objects referenced in the main symbol table may be called only once the executor has already shut down. php_zip_cancel_callback() may attempt to invoke a user callback, which will terminate the process because user code is not expected to be executed at this point. We solve this by calling the callback in dtor_obj(), which is called earlier in the shutdown sequence. dtor_obj() will actually attempt to call it again if the object was reinitialized in the destructor, so we also avoid calling the callback when the executor has shut down in the first place. This should never matter in practice. Closes GH-19602
This commit is contained in:
4
NEWS
4
NEWS
@@ -29,4 +29,8 @@ PHP NEWS
|
||||
. Fixed bug GH-19926 (reset internal pointer earlier while splicing array
|
||||
while COW violation flag is still set). (alexandre-daubois)
|
||||
|
||||
- Zip:
|
||||
. Fixed ZipArchive callback being called after executor has shut down.
|
||||
(ilutov)
|
||||
|
||||
<<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>
|
||||
|
||||
@@ -1002,6 +1002,21 @@ static void php_zip_cancel_callback_free(void *ptr)
|
||||
}
|
||||
#endif
|
||||
|
||||
static void php_zip_object_dtor(zend_object *object)
|
||||
{
|
||||
zend_objects_destroy_object(object);
|
||||
|
||||
ze_zip_object *intern = php_zip_fetch_object(object);
|
||||
|
||||
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);
|
||||
}
|
||||
intern->za = NULL;
|
||||
}
|
||||
}
|
||||
|
||||
static void php_zip_object_free_storage(zend_object *object) /* {{{ */
|
||||
{
|
||||
ze_zip_object * intern = php_zip_fetch_object(object);
|
||||
@@ -2995,6 +3010,10 @@ PHP_METHOD(ZipArchive, getStream)
|
||||
#ifdef HAVE_PROGRESS_CALLBACK
|
||||
static void php_zip_progress_callback(zip_t *arch, double state, void *ptr)
|
||||
{
|
||||
if (!EG(active)) {
|
||||
return;
|
||||
}
|
||||
|
||||
zval cb_args[1];
|
||||
ze_zip_object *obj = ptr;
|
||||
|
||||
@@ -3041,6 +3060,10 @@ static int php_zip_cancel_callback(zip_t *arch, void *ptr)
|
||||
zval cb_retval;
|
||||
ze_zip_object *obj = ptr;
|
||||
|
||||
if (!EG(active)) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL);
|
||||
if (Z_ISUNDEF(cb_retval)) {
|
||||
/* Cancel if an exception has been thrown */
|
||||
@@ -3128,6 +3151,7 @@ static PHP_MINIT_FUNCTION(zip)
|
||||
memcpy(&zip_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
|
||||
zip_object_handlers.offset = XtOffsetOf(ze_zip_object, zo);
|
||||
zip_object_handlers.free_obj = php_zip_object_free_storage;
|
||||
zip_object_handlers.dtor_obj = php_zip_object_dtor;
|
||||
zip_object_handlers.clone_obj = NULL;
|
||||
zip_object_handlers.get_property_ptr_ptr = php_zip_get_property_ptr_ptr;
|
||||
|
||||
|
||||
28
ext/zip/tests/ZipArchive_bailout.phpt
Normal file
28
ext/zip/tests/ZipArchive_bailout.phpt
Normal file
@@ -0,0 +1,28 @@
|
||||
--TEST--
|
||||
ZipArchive destructor should be called on bailout
|
||||
--EXTENSIONS--
|
||||
zip
|
||||
--FILE--
|
||||
<?php
|
||||
|
||||
function &cb() {}
|
||||
|
||||
$file = __DIR__ . '/gh18907.zip';
|
||||
$zip = new ZipArchive;
|
||||
$zip->open($file, ZIPARCHIVE::CREATE);
|
||||
$zip->registerCancelCallback(cb(...));
|
||||
$zip->addFromString('test', 'test');
|
||||
$fusion = $zip;
|
||||
|
||||
?>
|
||||
--CLEAN--
|
||||
<?php
|
||||
$file = __DIR__ . '/gh18907.zip';
|
||||
@unlink($file);
|
||||
?>
|
||||
--EXPECTF--
|
||||
Notice: Only variable references should be returned by reference in %s on line %d
|
||||
|
||||
Notice: Only variable references should be returned by reference in %s on line %d
|
||||
|
||||
Notice: Only variable references should be returned by reference in %s on line %d
|
||||
28
ext/zip/tests/ZipArchive_destruct.phpt
Normal file
28
ext/zip/tests/ZipArchive_destruct.phpt
Normal file
@@ -0,0 +1,28 @@
|
||||
--TEST--
|
||||
Leaking ZipArchive destructor
|
||||
--EXTENSIONS--
|
||||
zip
|
||||
--FILE--
|
||||
<?php
|
||||
|
||||
class MyZipArchive extends ZipArchive {
|
||||
public function __destruct() {
|
||||
global $leak;
|
||||
$leak = $this;
|
||||
}
|
||||
}
|
||||
|
||||
new MyZipArchive;
|
||||
$file = __DIR__ . '/ZipArchive_destruct.zip';
|
||||
$leak->open($file, ZIPARCHIVE::CREATE);
|
||||
$leak->addFromString('test', 'test');
|
||||
|
||||
?>
|
||||
===DONE===
|
||||
--CLEAN--
|
||||
<?php
|
||||
$file = __DIR__ . '/ZipArchive_destruct.zip';
|
||||
@unlink($file);
|
||||
?>
|
||||
--EXPECT--
|
||||
===DONE===
|
||||
Reference in New Issue
Block a user