diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index a8289a41a304..a5bfa7b5d9f6 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -607,6 +607,12 @@ static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */ } obj->za = NULL; + + if (obj->bailout_callback) { + obj->bailout_callback = false; + zend_bailout(); + } + return success; } /* }}} */ @@ -1049,10 +1055,16 @@ static void php_zip_object_dtor(zend_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)); + if (!intern->bailout_callback) { + php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za)); + } zip_discard(intern->za); } intern->za = NULL; + if (intern->bailout_callback) { + intern->bailout_callback = false; + zend_bailout(); + } } } @@ -2902,15 +2914,21 @@ PHP_METHOD(ZipArchive, getStream) #ifdef HAVE_PROGRESS_CALLBACK static void php_zip_progress_callback(zip_t *arch, double state, void *ptr) { - if (!EG(active)) { + ze_zip_object *obj = ptr; + + if (!EG(active) || obj->bailout_callback) { return; } zval cb_args[1]; - ze_zip_object *obj = ptr; ZVAL_DOUBLE(&cb_args[0], state); - zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL); + + zend_try { + zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL); + } zend_catch { + obj->bailout_callback = true; + } zend_end_try(); } /* {{{ register a progression callback: void callback(double state); */ @@ -2953,11 +2971,18 @@ static int php_zip_cancel_callback(zip_t *arch, void *ptr) zval cb_retval; ze_zip_object *obj = ptr; - if (!EG(active)) { + if (!EG(active) || obj->bailout_callback) { return 0; } - zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL); + zend_try { + zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL); + } zend_catch { + obj->bailout_callback = true; + /* Cancel if a bailout occurs to allow cleanup to happen */ + return -1; + } zend_end_try(); + if (Z_ISUNDEF(cb_retval)) { /* Cancel if an exception has been thrown */ return -1; diff --git a/ext/zip/php_zip.h b/ext/zip/php_zip.h index cdd72ba36367..97f70c0873ad 100644 --- a/ext/zip/php_zip.h +++ b/ext/zip/php_zip.h @@ -72,6 +72,7 @@ typedef struct _ze_zip_object { zip_int64_t last_id; int err_zip; int err_sys; + bool bailout_callback; #ifdef HAVE_PROGRESS_CALLBACK zend_fcall_info_cache progress_callback; #endif diff --git a/ext/zip/tests/gh22176.phpt b/ext/zip/tests/gh22176.phpt new file mode 100644 index 000000000000..90b6327072c9 --- /dev/null +++ b/ext/zip/tests/gh22176.phpt @@ -0,0 +1,33 @@ +--TEST-- +GH-22176 (Memory leak when a ZipArchive cancel callback bails out in the shutdown destructor) +--EXTENSIONS-- +zip +--SKIPIF-- + +--FILE-- +open(__DIR__ . '/gh22176_cancel.zip', ZipArchive::CREATE); +$zip->registerCancelCallback(function () { + throw new \Exception('cancel boom'); +}); +$zip->addFromString('test', 'test'); +echo "done\n"; +// The archive is flushed and the object destroyed during request shutdown; +// the thrown exception bails out through libzip's zip_close() without leaking +// its internal state, and the bailout resumes once libzip has unwound. +?> +--CLEAN-- + +--EXPECTF-- +done + +Fatal error: Uncaught Exception: cancel boom in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}() +#1 {main} + thrown in %s on line %d diff --git a/ext/zip/tests/gh22176_progress.phpt b/ext/zip/tests/gh22176_progress.phpt new file mode 100644 index 000000000000..5507fe24ab98 --- /dev/null +++ b/ext/zip/tests/gh22176_progress.phpt @@ -0,0 +1,33 @@ +--TEST-- +GH-22176 (Memory leak when a ZipArchive progress callback bails out in the shutdown destructor) +--EXTENSIONS-- +zip +--SKIPIF-- + +--FILE-- +open(__DIR__ . '/gh22176_progress.zip', ZipArchive::CREATE); +$zip->registerProgressCallback(0.5, function ($r) { + throw new \Exception('progress boom'); +}); +$zip->addFromString('test', 'test'); +echo "done\n"; +// The archive is flushed and the object destroyed during request shutdown; +// the thrown exception bails out through libzip's zip_close() without leaking +// its internal state, and the bailout resumes once libzip has unwound. +?> +--CLEAN-- + +--EXPECTF-- +done + +Fatal error: Uncaught Exception: progress boom in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}(0.0) +#1 {main} + thrown in %s on line %d